feat(godot,#1091): brighter, clearer iso hero sprite#1144
Conversation
Replace the dim/murky generic fighter iso bake with a bright, readable armored-knight hero at the SAME scope (sprite-fighter-iso8) — fully additive, no engine/python/scope-wiring change. Root cause of the old murk (in bake_sprites.py): - No view transform set -> Blender's default AgX (Filmic on older builds) desaturated + crushed midtones into a grey blob. Now: Standard transform, look=None, +0.35 exposure. - World ambient was near-black (0.05); shadow sides went dark. Now a bright neutral-cool ambient (0.45..0.52 @ 1.0). - Weak/uneven lights (sun=3 + 2 weak area fills). Now a 4x SUN rig (direction-only so it reads the same across all 8 turntable yaws): warm key 5.0, cool opposite fill 2.6, top wash 1.6, cool back/rim 3.0. Added EEVEE Fast-GI AO (5.x) with legacy GTAO fallback + 64 TAA samples. Art: Meshy meshy-5 PBR text->3D (polished steel plate + royal-blue tabard + gold trim) -> brightened Blender bake (192/192 frames, dimetric 2:1 exact, anchor 64,109) -> pack_sheet @ sprite-fighter-iso8. Validated locally: godot --import clean; boot anims=32; served-finals served-atlas-applied=true resolver_cached=true anims=32; combat-tokens 5/5; smoke-intent frozen-vocab. Same scope key => godot.yml served-finals smoke needs no change (it cp's this committed sheet).
📝 WalkthroughPriority Level: P1Critical IssueOverly Broad Exception Handling in Cross-Version Error Recovery — Required for merge What is wrong: The Why it matters: Silent failure on misspelled attributes (e.g., Location: Recommended fix: Narrow the exception catch to only version/property-existence errors: def _try_set(obj, attr: str, val) -> None:
"""Set obj.attr = val, swallowing AttributeError/TypeError for cross-version safety."""
try:
setattr(obj, attr, val)
except (AttributeError, TypeError):
passRequired before merge: Yes — this allows legitimate property-not-found errors to be handled gracefully across Blender versions while preserving visibility of actual bugs. Implementation Verification — Passing✓ Color Management: View transform "Standard" with look=None and +0.35 exposure correctly prevents AgX/Filmic desaturation. ✓ World Ambient: Bright neutral-cool (0.45, 0.47, 0.52, 1.0) @ 1.0 intensity properly lifts shadow sides off near-black. ✓ Lighting Rig: Four-SUN direction-only configuration maintains consistency across all 8 turntable yaws:
✓ EEVEE Quality: TAA set to 64 samples; Fast-GI AO enabled with legacy GTAO fallback for cross-version compatibility. ✓ Asset Integrity: JSON valid; PNG present (1.1M); attribution updated with final heroic fantasy knight prompt. ✓ No Scope Changes: Modifications fully additive to bake pipeline; same sprite key ( Walkthrough
ChangesSprite Bake Pipeline Improvements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| """Set obj.attr = val, swallowing AttributeError/TypeError for cross-version safety.""" | ||
| try: | ||
| setattr(obj, attr, val) | ||
| except Exception: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@godot/tools/bake_sprites.py`:
- Around line 121-124: The Background node inputs access in lines 123-124 (where
bg.inputs[0] and bg.inputs[1] are assigned) lacks error handling and could fail
with IndexError if the node structure differs across Blender versions. Wrap the
background input assignments (both the default_value assignments for inputs[0]
and inputs[1]) in a try/except block to match the defensive error-handling
pattern used for view_settings configuration earlier in the _reset_scene
function, catching IndexError or KeyError exceptions and logging a warning
instead of crashing when the expected node structure is missing.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b83c1b07-ead0-4df5-b7ed-ce03563e7c5f
⛔ Files ignored due to path filters (1)
godot/assets/characters/fighter/sheet.pngis excluded by!**/*.png
📒 Files selected for processing (2)
godot/assets/characters/fighter/sheet.jsongodot/tools/bake_sprites.py
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: test
- GitHub Check: viewer-tests
- GitHub Check: Analyze (python)
- GitHub Check: viewer-tests
- GitHub Check: test
🧰 Additional context used
🪛 Ruff (0.15.17)
godot/tools/bake_sprites.py
[warning] 112-112: Do not catch blind exception: Exception
(BLE001)
[warning] 113-113: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
[warning] 143-143: Boolean positional value in function call
(FBT003)
[warning] 147-147: Boolean positional value in function call
(FBT003)
[warning] 153-156: Use contextlib.suppress(Exception) instead of try-except-pass
Replace try-except-pass with with contextlib.suppress(Exception): ...
(SIM105)
[error] 155-156: try-except-pass detected, consider logging the exception
(S110)
[warning] 155-155: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (3)
godot/tools/bake_sprites.py (2)
99-156: LGTM!
223-265: LGTM!godot/assets/characters/fighter/sheet.json (1)
55-55: LGTM!
| bg = world.node_tree.nodes.get("Background") | ||
| if bg: | ||
| bg.inputs[0].default_value = (0.05, 0.05, 0.06, 1.0) | ||
| bg.inputs[1].default_value = 0.6 | ||
| bg.inputs[0].default_value = (0.45, 0.47, 0.52, 1.0) # bright neutral-cool ambient | ||
| bg.inputs[1].default_value = 1.0 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Background node inputs access could be wrapped for defensive consistency (90% confidence).
Lines 123-124 access bg.inputs[0] and bg.inputs[1] without error handling, unlike the view_settings configuration (lines 106-113) which wraps property access in try/except. While the freshly created factory-settings world should have a predictable Background node structure, wrapping this access would align with the defensive cross-version pattern established elsewhere in _reset_scene.
Impact: If a future or legacy Blender version creates world nodes with a different structure, the script would crash with IndexError/KeyError during scene setup rather than gracefully degrading.
Root cause: Assumption that factory settings always create a Background node with standard inputs.
Code path: _reset_scene → world creation → Background node access → potential crash if node structure differs.
🛡️ Defensive wrapper to match the view_settings error-handling pattern
bg = world.node_tree.nodes.get("Background")
-if bg:
+if bg and hasattr(bg, "inputs") and len(bg.inputs) >= 2:
bg.inputs[0].default_value = (0.45, 0.47, 0.52, 1.0) # bright neutral-cool ambient
bg.inputs[1].default_value = 1.0Alternatively, wrap in try/except for full consistency:
bg = world.node_tree.nodes.get("Background")
-if bg:
- bg.inputs[0].default_value = (0.45, 0.47, 0.52, 1.0) # bright neutral-cool ambient
- bg.inputs[1].default_value = 1.0
+try:
+ if bg:
+ bg.inputs[0].default_value = (0.45, 0.47, 0.52, 1.0) # bright neutral-cool ambient
+ bg.inputs[1].default_value = 1.0
+except Exception:
+ pass # Factory world will use default background📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bg = world.node_tree.nodes.get("Background") | |
| if bg: | |
| bg.inputs[0].default_value = (0.05, 0.05, 0.06, 1.0) | |
| bg.inputs[1].default_value = 0.6 | |
| bg.inputs[0].default_value = (0.45, 0.47, 0.52, 1.0) # bright neutral-cool ambient | |
| bg.inputs[1].default_value = 1.0 | |
| bg = world.node_tree.nodes.get("Background") | |
| if bg and hasattr(bg, "inputs") and len(bg.inputs) >= 2: | |
| bg.inputs[0].default_value = (0.45, 0.47, 0.52, 1.0) # bright neutral-cool ambient | |
| bg.inputs[1].default_value = 1.0 |
🤖 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 `@godot/tools/bake_sprites.py` around lines 121 - 124, The Background node
inputs access in lines 123-124 (where bg.inputs[0] and bg.inputs[1] are
assigned) lacks error handling and could fail with IndexError if the node
structure differs across Blender versions. Wrap the background input assignments
(both the default_value assignments for inputs[0] and inputs[1]) in a try/except
block to match the defensive error-handling pattern used for view_settings
configuration earlier in the _reset_scene function, catching IndexError or
KeyError exceptions and logging a warning instead of crashing when the expected
node structure is missing.
) (#1146) Adds the --preview-scene Godot entrypoint (additive, non-interactive) that loads a JSON spec, builds a dimetric 2:1 diamond grid overlay via NavOverlay.gd, solves an A* path with AStarGrid2D, captures the viewport to a PNG, and writes a nav.json sidecar — gen→place→render→view in one command. Four additive pieces: 1. Main.gd — _run_preview_scene() + _arg_value() helper; dispatched via --preview-scene 2. RenderProfile.gd — load_inline(profile) so a preview can inject a profile dict 3. WorldView.gd — apply_local_backdrop(path) for zero-HTTP local PNG loading 4. NavOverlay.gd (new Node2D) — _draw(): diamond grid + walkable/blocked tints + zone-anchor rings + solved A* polyline + actor foot-dots + facing arrows Spec format and qa/preview_scene.sh usage documented in both files. Verified locally: proof-A path_found=true (10-cell corridor), proof-B path_found=false (column wall). Existing headless conformance unchanged. Co-authored-by: Eva <arncalso@gmail.com>
1 participant
What
Replaces the dim/murky generic fighter iso bake (merged in #1142) with a bright, readable, detailed armored-knight hero at the same scope (
sprite-fighter-iso8). Fully additive — no engine/python/scope-wiring change.Why the old sheet was a dim grey blob — and the fix
The biggest lever was the bake lighting + color management in
godot/tools/bake_sprites.py:view_transform="Standard",look="None", exposure +0.35(0.05)→ shadow sides went dark(0.45..0.52) @ 1.0Art source: Meshy
meshy-5PBR text→3D (polished steel plate + royal-blue tabard + gold trim) → brightened Blender bake →pack_sheet.py@sprite-fighter-iso8.Validation (local, mirrors
godot.yml)godot --headless --path godot --import→ clean (exit 0)anims=32, WorldView location presentserved-atlas-applied=true resolver_cached=true anims=32ok=true count=5/5 all_zoned=true team_tinted=trueSame scope key ⇒
godot.ymlserved-finals smoke needs no change — itcps this committed sheet. Avoids the multi-point scope-rename trap that broke #1142's first run.Honest verdict (ceiling)
Brightness / readability / detail vs the old blob: decisively better — bar met. Style-match with the painterly tavern backdrop is ~80% — this is a clean 3D PBR render (the Meshy→bake ceiling), readable and grounded on the floor, but not pixel-identical to the hand-painted oil look. The head reads as a pointed helm/hood (A-pose derived from the T-pose source); fine at 128px token scale. A true painterly match would need a 2D path: Scenario character art + manual 8-facing iso framing + a paint post-process.
Diff
sheet.png+sheet.json(attribution prompt only) +bake_sprites.py(lighting). No engine/python/scope change.