feat: add proper placeholder modules#76
Merged
Merged
Conversation
…hanced prefab generation and sprite handling
…Janitor work type
📝 WalkthroughWalkthroughReorganizes module prefab and SO metadata, introduces gameplay thrust scaling and test normalizations, refactors Laser and WeaponBase behavior, adds editor preview tooling and updated prefab generation, and modularizes the sprite-generator into callable steps with a pipeline entrypoint. ChangesGameplay Mechanics and Constants
Component Behavior and Runtime Guards
Editor Tools and Asset Reorganization
Sprite Generation Pipeline Refactoring
Sequence Diagram(s)No sequence diagram generated. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
sprite_generator/raw_file_generator.py (2)
14-15: ⚡ Quick winReplace
exit()with exceptions in library functions (affectsraw_file_generator.pyandinputs_to_outputs.py).Both
generate_raw_file()andgenerate_pngs()useexit()to terminate on error, which is inappropriate for library functions and makes them untestable. Replaceexit()calls with appropriate exceptions (FileNotFoundError) that the caller (pipeline orchestrator) can handle or propagate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sprite_generator/raw_file_generator.py` around lines 14 - 15, The code currently calls exit() in library functions which prevents callers from handling errors; in raw_file_generator.py (function generate_raw_file) and inputs_to_outputs.py (function generate_pngs) replace the exit() calls with raising appropriate exceptions (e.g., raise FileNotFoundError with a clear message including INPUT_FILE or the missing path) instead of printing and exiting so the pipeline orchestrator can catch or propagate the error.
6-49: 💤 Low valueConsider returning status or parsed file count.
The function has no return value, making it harder to verify success in tests or provide feedback to callers. Consider returning the count of files generated or a success boolean.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sprite_generator/raw_file_generator.py` around lines 6 - 49, generate_raw_file currently returns nothing which makes testing and callers unable to verify success; modify generate_raw_file to compute and return a meaningful result (e.g., an integer count of files written or a boolean success flag). After the loop that writes files (where you use parsed_files and write to filepath in SAVE_DIR), increment a counter for each file actually saved (skip empty matrices as currently), and return that counter (or return True if >0, False otherwise); ensure the function signature and callers handle the new return value.sprite_generator/pipeline.py (1)
4-6: 💤 Low valueConsider adding error handling and return status.
The pipeline silently propagates exceptions from the generator functions, which is acceptable, but consider wrapping in try-except to provide context or cleanup, and returning a status code for programmatic success detection.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sprite_generator/pipeline.py` around lines 4 - 6, Wrap run_pipeline in a try/except that calls generate_raw_file() and generate_pngs() inside the try, catch Exception as e to log or re-raise with contextual information (include function names generate_raw_file and generate_pngs in the message), perform any necessary cleanup in a finally block if applicable, and return an explicit status indicator (e.g., return 0 on success, non-zero on failure) so callers can detect success programmatically.sprite_generator/requirements.txt (1)
1-2: ⚡ Quick winPin dependency versions for reproducibility.
Unpinned dependencies can introduce breaking changes or inconsistent behavior across environments. For build reproducibility and security auditing, pin to specific tested versions.
♻️ Recommended pinning
-pillow -types-Pillow +pillow==11.0.0 +types-Pillow==11.0.0.20241206Note: Use specific versions tested with your pipeline. Run
pip freezeafter testing to capture working versions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sprite_generator/requirements.txt` around lines 1 - 2, The requirements.txt currently lists unpinned packages (pillow and types-Pillow); update those entries to exact, tested versions by replacing "pillow" and "types-Pillow" with pinned specs (e.g., pillow==<tested-version> and types-Pillow==<tested-version>) that you verified in CI or locally, then run pip freeze to capture the full, reproducible versions and commit the updated requirements.txt; target the lines containing "pillow" and "types-Pillow" in requirements.txt when making the change.Assets/Scripts/Editor/ProjectExtensions/ShipModuleSOEditor.cs (1)
18-18: 💤 Low valueConsider using
IPixelatedSpriteinterface for better abstraction.Line 18 directly references
PixelatedRigidbody, while other parts of the codebase (e.g.,ModulePaletteController.cs:104) use theIPixelatedSpriteinterface to retrieve sprites. Using the interface would make this code more flexible and consistent with existing patterns.♻️ Proposed refactor to use interface
-var sprite = Item.Prefab?.GetComponent<PixelatedRigidbody>()?.GetSprite(); +var sprite = Item.Prefab?.GetComponent<IPixelatedSprite>()?.GetSprite();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Assets/Scripts/Editor/ProjectExtensions/ShipModuleSOEditor.cs` at line 18, The code is tightly coupled to PixelatedRigidbody; replace the concrete type with the IPixelatedSprite abstraction so sprite retrieval uses Item.Prefab?.GetComponent<IPixelatedSprite>()?.GetSprite() (or GetComponentInChildren<IPixelatedSprite>() if other components host the sprite) to match ModulePaletteController.cs usage; update the null-safe check accordingly and ensure ShipModuleSOEditor references the IPixelatedSprite interface instead of PixelatedRigidbody.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Assets/Scripts/Core/Constants/GameplayConstants.cs`:
- Line 14: EngineThrustEfficiencyMultiplier is amplifying serialized
Engine.maxThrust (~1–20) by ~1000x in Engine.MaxThrust (combined with
ShipModuleEfficiency), causing unexpectedly large forces given Rigidbody mass is
PixelCount * MassMultiplier; update the constant handling by documenting the
units and rationale (what units maxThrust represents and why 1000f is required)
in the declaration of EngineThrustEfficiencyMultiplier, and either reduce or
make the multiplier configurable (expose as a tunable constant/serialized
setting) so it can be adjusted without code changes; also verify/annotate that
ShipModuleEfficiency scaling and default MassMultiplier = 1 keep acceleration in
stable ranges (check code paths using ShipModuleEfficiency, PixelCount, and
MassMultiplier) and add a sanity-check test or validation to prevent runaway
thrust values.
In `@Assets/Scripts/Editor/ProjectExtensions/ShipModuleSOEditor.cs`:
- Around line 31-33: Replace the incorrect reflection invocation that passes the
method name string as the target with a null target for the static method:
change the call using MethodInfo variable "method" (the static
"RenderStaticPreview" invocation) to invoke with a null first argument and the
existing argument array (sprite, Color.white, width, height) so
MethodInfo.Invoke(null, new object[] { sprite, Color.white, width, height }) is
used.
In `@Assets/Scripts/Ships/Modules/Laser.cs`:
- Line 155: In the Laser class replace the silent early-return "if
(!_lineRenderer) return;" with a hard failure that surfaces the missing required
component: check _lineRenderer and if null call Debug.LogError with a clear
message and then throw an InvalidOperationException (or use
UnityEngine.Assertions.Assert.IsNotNull) so firing cannot proceed with an
invalid runtime state; reference the _lineRenderer field and the method
containing that check (e.g., Fire or Update) when making the change.
- Line 52: Remove the redundant assignment inside Laser.Awake(): delete the line
that sets Type = ModuleType.Weapon; because Laser already overrides the property
via public override ModuleType Type => ModuleType.Weapon; and the base virtual
auto-property in Module is unused by the override; edit the Laser.Awake() method
to omit that assignment so only the override getter defines the Module type.
In `@sprite_generator/inputs_to_outputs.py`:
- Around line 62-106: The generate_pngs function writes PNGs to SAVE_DIR but
never ensures that directory exists; before saving any files (e.g., at the start
of generate_pngs, before the loop or before calling img.save), create the output
directory using os.makedirs(SAVE_DIR, exist_ok=True) (or equivalent) so
img.save(png_path) cannot raise FileNotFoundError; update generate_pngs to
ensure SAVE_DIR exists and optionally handle potential save errors around
img.save.
- Around line 80-81: The code assumes uniform row widths by using width =
len(matrix[0]) and height = len(matrix); add a validation step after computing
height/width to verify every row in matrix has length == width (e.g., check
any(len(row) != width for row in matrix)) and raise a clear exception
(ValueError) or return a handled error if a mismatch is found; include the
offending row index and its length in the error message so callers can diagnose
malformed input.
---
Nitpick comments:
In `@Assets/Scripts/Editor/ProjectExtensions/ShipModuleSOEditor.cs`:
- Line 18: The code is tightly coupled to PixelatedRigidbody; replace the
concrete type with the IPixelatedSprite abstraction so sprite retrieval uses
Item.Prefab?.GetComponent<IPixelatedSprite>()?.GetSprite() (or
GetComponentInChildren<IPixelatedSprite>() if other components host the sprite)
to match ModulePaletteController.cs usage; update the null-safe check
accordingly and ensure ShipModuleSOEditor references the IPixelatedSprite
interface instead of PixelatedRigidbody.
In `@sprite_generator/pipeline.py`:
- Around line 4-6: Wrap run_pipeline in a try/except that calls
generate_raw_file() and generate_pngs() inside the try, catch Exception as e to
log or re-raise with contextual information (include function names
generate_raw_file and generate_pngs in the message), perform any necessary
cleanup in a finally block if applicable, and return an explicit status
indicator (e.g., return 0 on success, non-zero on failure) so callers can detect
success programmatically.
In `@sprite_generator/raw_file_generator.py`:
- Around line 14-15: The code currently calls exit() in library functions which
prevents callers from handling errors; in raw_file_generator.py (function
generate_raw_file) and inputs_to_outputs.py (function generate_pngs) replace the
exit() calls with raising appropriate exceptions (e.g., raise FileNotFoundError
with a clear message including INPUT_FILE or the missing path) instead of
printing and exiting so the pipeline orchestrator can catch or propagate the
error.
- Around line 6-49: generate_raw_file currently returns nothing which makes
testing and callers unable to verify success; modify generate_raw_file to
compute and return a meaningful result (e.g., an integer count of files written
or a boolean success flag). After the loop that writes files (where you use
parsed_files and write to filepath in SAVE_DIR), increment a counter for each
file actually saved (skip empty matrices as currently), and return that counter
(or return True if >0, False otherwise); ensure the function signature and
callers handle the new return value.
In `@sprite_generator/requirements.txt`:
- Around line 1-2: The requirements.txt currently lists unpinned packages
(pillow and types-Pillow); update those entries to exact, tested versions by
replacing "pillow" and "types-Pillow" with pinned specs (e.g.,
pillow==<tested-version> and types-Pillow==<tested-version>) that you verified
in CI or locally, then run pip freeze to capture the full, reproducible versions
and commit the updated requirements.txt; target the lines containing "pillow"
and "types-Pillow" in requirements.txt when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c6330172-63fd-42c0-b02b-6a822cd1dfe6
⛔ Files ignored due to path filters (78)
Assets/Prefabs/Gameplay/Modules/Command/player_command_medium_24x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Command/player_command_small_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Engine/player_engine_big_40x32.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Engine/player_engine_medium_32x24.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Engine/player_engine_small_1_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Engine/player_engine_small_2_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_cannon_medium_1_32x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_cannon_medium_2_32x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_cannon_small_3_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_command_small_1_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_conn_unarmored_1_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_conn_unarmored_2_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_crew_small_1_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_crew_small_2_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_engine_medium_1_16x32.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_engine_small_1_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_engine_small_2_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_engine_small_3_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_laser_big_1_32x32.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_power_small_1_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Old/player_power_small_2_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Resource/player_battery_small_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Resource/player_crew_medium_24x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Resource/player_crew_small_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Resource/player_power_medium_24x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Resource/player_power_small_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Weapons/player_cannon_large_32x32.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Weapons/player_cannon_medium_24x24.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Weapons/player_cannon_small_1_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Weapons/player_cannon_small_2_16x16.prefabis excluded by!**/*.prefabAssets/Prefabs/Gameplay/Modules/Weapons/player_laser_small_16x16.prefabis excluded by!**/*.prefabAssets/SOs/ModulePrefabLibrary.assetis excluded by!**/*.assetAssets/SOs/Modules/Command/CommandModule 1.assetis excluded by!**/*.assetAssets/SOs/Modules/Command/CommandModule 2.assetis excluded by!**/*.assetAssets/SOs/Modules/Engine/Engine 1.assetis excluded by!**/*.assetAssets/SOs/Modules/Engine/Engine 2.assetis excluded by!**/*.assetAssets/SOs/Modules/Engine/Engine 3.assetis excluded by!**/*.assetAssets/SOs/Modules/Engine/Engine 4.assetis excluded by!**/*.assetAssets/SOs/Modules/Resource/Crew 1.assetis excluded by!**/*.assetAssets/SOs/Modules/Resource/Crew 2.assetis excluded by!**/*.assetAssets/SOs/Modules/Resource/Power 1.assetis excluded by!**/*.assetAssets/SOs/Modules/Resource/Power 2.assetis excluded by!**/*.assetAssets/SOs/Modules/Resource/Power.assetis excluded by!**/*.assetAssets/SOs/Modules/Weapons/CannonBig.assetis excluded by!**/*.assetAssets/SOs/Modules/Weapons/CannonSmall 1.assetis excluded by!**/*.assetAssets/SOs/Modules/Weapons/CannonSmall 2.assetis excluded by!**/*.assetAssets/SOs/Modules/Weapons/CannonSmall 3.assetis excluded by!**/*.assetAssets/SOs/Modules/Weapons/CannonSmall.assetis excluded by!**/*.assetAssets/Scenes/ShipFactory.unityis excluded by!**/*.unityAssets/Sprites/Generated/New.metais excluded by!**/generated/**Assets/Sprites/Generated/player_battery_small_16x16.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_battery_small_16x16.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_battery_small_16x16_armor.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_battery_small_16x16_armor.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_cannon_large_32x32.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_cannon_large_32x32.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_cannon_large_32x32_armor.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_cannon_large_32x32_armor.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_cannon_medium_24x24.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_cannon_medium_24x24.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_cannon_medium_24x24_armor.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_cannon_medium_24x24_armor.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_cannon_small_1_16x16.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_cannon_small_1_16x16.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_cannon_small_1_16x16_armor.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_cannon_small_1_16x16_armor.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_cannon_small_2_16x16.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_cannon_small_2_16x16.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_cannon_small_2_16x16_armor.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_cannon_small_2_16x16_armor.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_engine_big_40x32.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_engine_big_40x32.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_engine_big_40x32_armor.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_engine_big_40x32_armor.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_laser_small_16x16.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_laser_small_16x16.png.metais excluded by!**/generated/**Assets/Sprites/Generated/player_laser_small_16x16_armor.pngis excluded by!**/*.png,!**/generated/**Assets/Sprites/Generated/player_laser_small_16x16_armor.png.metais excluded by!**/generated/**
📒 Files selected for processing (77)
.gitignoreAssets/Prefabs/Gameplay/Modules/Command.metaAssets/Prefabs/Gameplay/Modules/Command/player_command_medium_24x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Command/player_command_small_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Engine.metaAssets/Prefabs/Gameplay/Modules/Engine/player_engine_big_40x32.prefab.metaAssets/Prefabs/Gameplay/Modules/Engine/player_engine_medium_32x24.prefab.metaAssets/Prefabs/Gameplay/Modules/Engine/player_engine_small_1_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Engine/player_engine_small_2_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/New.metaAssets/Prefabs/Gameplay/Modules/Old/player_cannon_small_3_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_command_small_1_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_command_small_2_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_conn_unarmored_1_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_conn_unarmored_2_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_crew_small_1_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_crew_small_2_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_engine_medium_1_16x32.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_engine_small_1_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_engine_small_2_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_engine_small_3_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_laser_big_1_32x32.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_laser_small_1_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_power_small_1_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_power_small_2_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Old/player_power_small_3_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Resource.metaAssets/Prefabs/Gameplay/Modules/Resource/player_battery_small_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Resource/player_crew_medium_24x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Resource/player_crew_small_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Resource/player_power_medium_24x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Resource/player_power_small_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Weapons.metaAssets/Prefabs/Gameplay/Modules/Weapons/player_cannon_large_32x32.prefab.metaAssets/Prefabs/Gameplay/Modules/Weapons/player_cannon_medium_24x24.prefab.metaAssets/Prefabs/Gameplay/Modules/Weapons/player_cannon_small_1_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Weapons/player_cannon_small_2_16x16.prefab.metaAssets/Prefabs/Gameplay/Modules/Weapons/player_laser_small_16x16.prefab.metaAssets/Resources/ShipSnapshots/Enemy.jsonAssets/Resources/ShipSnapshots/Friend.jsonAssets/SOs/Modules/Command.metaAssets/SOs/Modules/Command/CommandModule 1.asset.metaAssets/SOs/Modules/Command/CommandModule 2.asset.metaAssets/SOs/Modules/Engine.metaAssets/SOs/Modules/Engine/Engine 1.asset.metaAssets/SOs/Modules/Engine/Engine 2.asset.metaAssets/SOs/Modules/Engine/Engine 3.asset.metaAssets/SOs/Modules/Engine/Engine 4.asset.metaAssets/SOs/Modules/Resource.metaAssets/SOs/Modules/Resource/Crew 1.asset.metaAssets/SOs/Modules/Resource/Crew 2.asset.metaAssets/SOs/Modules/Resource/Power 1.asset.metaAssets/SOs/Modules/Resource/Power 2.asset.metaAssets/SOs/Modules/Resource/Power.asset.metaAssets/SOs/Modules/Weapons.metaAssets/SOs/Modules/Weapons/CannonBig.asset.metaAssets/SOs/Modules/Weapons/CannonSmall 1.asset.metaAssets/SOs/Modules/Weapons/CannonSmall 2.asset.metaAssets/SOs/Modules/Weapons/CannonSmall 3.asset.metaAssets/SOs/Modules/Weapons/CannonSmall.asset.metaAssets/Scripts/Core/Constants/GameplayConstants.csAssets/Scripts/Core/Ship/CrewSkillType.csAssets/Scripts/Core/Ship/Resources.csAssets/Scripts/Editor/Editor.asmdefAssets/Scripts/Editor/ProjectExtensions.metaAssets/Scripts/Editor/ProjectExtensions/ShipModuleSOEditor.csAssets/Scripts/Editor/ProjectExtensions/ShipModuleSOEditor.cs.metaAssets/Scripts/Editor/Windows/GenerateModulePrefabs.csAssets/Scripts/Editor/Windows/GenerateModulePrefabs.cs.metaAssets/Scripts/Services/EffectsSpawner.csAssets/Scripts/Ships/Modules/Engine.csAssets/Scripts/Ships/Modules/Laser.csAssets/Scripts/Ships/Tests/ModuleJointLifecycleTests.cssprite_generator/inputs_to_outputs.pysprite_generator/pipeline.pysprite_generator/raw_file_generator.pysprite_generator/requirements.txt
💤 Files with no reviewable changes (17)
- Assets/Prefabs/Gameplay/Modules/Old/player_conn_unarmored_1_16x16.prefab.meta
- Assets/Scripts/Ships/Tests/ModuleJointLifecycleTests.cs
- Assets/Prefabs/Gameplay/Modules/Old/player_cannon_small_3_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_crew_small_1_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_command_small_2_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_engine_small_2_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_power_small_1_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_engine_small_1_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_command_small_1_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_power_small_3_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_power_small_2_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_engine_small_3_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_crew_small_2_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_conn_unarmored_2_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_laser_big_1_32x32.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_laser_small_1_16x16.prefab.meta
- Assets/Prefabs/Gameplay/Modules/Old/player_engine_medium_1_16x32.prefab.meta
There was a problem hiding this comment.
♻️ Duplicate comments (1)
sprite_generator/inputs_to_outputs.py (1)
83-84:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd validation for consistent matrix row width.
The code assumes all rows have the same width (line 83 uses
len(matrix[0])), but malformed input could have inconsistent row lengths, causing visual corruption or index errors.🛡️ Proposed validation
width = len(matrix[0]) height = len(matrix) + + if not all(len(row) == width for row in matrix): + print(f"WARNING: Inconsistent row widths in {filename}, skipping.") + continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sprite_generator/inputs_to_outputs.py` around lines 83 - 84, The code computes width = len(matrix[0]) and assumes every row matches that length; add a validation step before that to verify all rows in matrix have identical lengths (e.g., compare len(row) for each row against len(matrix[0])) and if any mismatch is found raise a clear exception (ValueError) with context (e.g., include expected width and the index/length of the bad row) so the calling code can detect malformed input; implement this check just before the existing width/height calculation that references matrix[0].
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@sprite_generator/inputs_to_outputs.py`:
- Around line 83-84: The code computes width = len(matrix[0]) and assumes every
row matches that length; add a validation step before that to verify all rows in
matrix have identical lengths (e.g., compare len(row) for each row against
len(matrix[0])) and if any mismatch is found raise a clear exception
(ValueError) with context (e.g., include expected width and the index/length of
the bad row) so the calling code can detect malformed input; implement this
check just before the existing width/height calculation that references
matrix[0].
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 687d3306-2c77-4346-bff8-e52d5696b5bf
📒 Files selected for processing (4)
Assets/Scripts/Ships/Modules/Cannon.csAssets/Scripts/Ships/Modules/Laser.csAssets/Scripts/Ships/Modules/WeaponBase.cssprite_generator/inputs_to_outputs.py
💤 Files with no reviewable changes (1)
- Assets/Scripts/Ships/Modules/Cannon.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Assets/Scripts/Ships/Modules/Laser.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves #71
Summary by CodeRabbit
New Features
Bug Fixes
Chores