fix: residents step aside instead of trading pixels with the hero (#9) by blippip69 · Pull Request #43 · Bitcoindefi/runa

blippip69 · 2026-08-25T14:22:07Z

fix: residents step aside instead of trading pixels with the hero (#9)

Summary by Gitar

Rendering improvements:
- Prevent hero sprite from painting through solid walls by clipping colliding cells (#16)
- Add comprehensive unit tests covering hero wall-clipping and NPC-hero sprite separation (#9, #16)

_{This will update automatically on new commits.}

gitar-bot · 2026-08-25T14:24:06Z

+  let heroGeom = null
+  {
+    const lines_ = Array.isArray(hero.sprite) && hero.sprite.length ? hero.sprite : null
+    if (lines_) {
+      const hw = lines_.reduce((widest, line) => Math.max(widest, ascii(line).length), 1)
+      const hLeft = hx - clamp(HERO_ANCHOR_X, 0, hw - 1)
+      const heroAnchorY = Number.isFinite(Number(hero.anchorY))
+        ? Math.round(Number(hero.anchorY))
+        : lines_.length - 1
+      const hTop = hy - clamp(heroAnchorY, 0, lines_.length - 1)
+      heroGeom = { left: hLeft, width: hw }
+      const solidView = { rows: tiles, width: mapW, height: mapH }
+      for (let sy = 0; sy < lines_.length; sy++) {
+        const line = ascii(lines_[sy]).padEnd(hw)
+        const first = line.search(/\S/)


⚠️ Bug: heroGeom null deref crashes glyph-hero + overlapping actor

When the hero has no sprite (glyph fallback), the reservation block leaves heroGeom = null but still reserves the hero cell hy,hx. If an actor's drawn footprint covers that cell, collidesHero(left) returns true and the code dereferences heroGeom.left/heroGeom.width (lines 831-832), throwing TypeError: Cannot read properties of null and crashing the render. Set a fallback geometry in the else branch (e.g. heroGeom = { left: hx, width: 1 }) so the separation maths works for glyph heroes too.

Give the glyph-hero fallback a valid geometry so the actor step-aside pass never dereferences null.:

} else { heroReserved.add(hy + ',' + hx) heroGeom = { left: hx, width: 1 } }

Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

gitar-bot · 2026-08-25T14:24:07Z

+  // Reservation pass: the exact cells the hero is about to paint, computed
+  // with the same maths as the hero pass below (including the #16 solid clip).
+  const heroReserved = new Set()
+  const actorOccupied = new Set()


💡 Quality: actorOccupied set is populated but never read

actorOccupied is declared at line 759 and written to at line 859 but never read anywhere in the function or elsewhere in the file. It is dead code that adds per-cell overhead each frame. Remove the declaration and the .add() call unless a follow-up consumer is intended.

_{Was this helpful? React with 👍 / 👎}

gitar-bot · 2026-08-25T14:24:08Z


-    const actorW = actorSprite.reduce((widest, line) => Math.max(widest, ascii(line).length), 1)
-    const left = ax - Math.floor(actorW / 2)
+    const actorW = actorSprite.reduce((widest, line) => Math.max(widest, line.length), 1)


💡 Bug: actorW uses raw length, diverging from ascii-normalized paint

The reservation/paint loops normalize actor sprite rows with ascii(...).padEnd(actorW), but actorW is now computed from raw line.length (line 804) after this PR dropped the previous ascii(line).length. For sprites containing astral (surrogate-pair) characters the two widths disagree, so padEnd(actorW) and the collision scan can misalign. NPC art is currently ASCII so impact is latent; restore ascii(line).length to keep width computation consistent with rendering.

Compute actorW from the ascii-normalized width, matching the loops that render the sprite.:

const actorW = actorSprite.reduce((widest, line) => Math.max(widest, ascii(line).length), 1)

Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

The overlay loop painted every hero cell unconditionally, so the rows of the body that hang above the feet landed on solid tiles and off-map cells and read as holes in the wall, most visibly on every dungeon arrival. Hero cells are now skipped when the tile beneath them is solid, checked against the canonical isSolid table instead of render-local guesses.

…tcoindefi#9) A resident sprite is seven columns wide but blocks one logical cell, and talk distance reaches one or two tiles, so the hero body and the resident body shared screen cells and overwrote each other exactly when e hablar mattered. mapPane now reserves the cells the hero is about to paint and slides a colliding resident drawing sideways - the same presentation-only separation fieldPane applies to an active foe - clamped to the camera, falling back to skipping just the contested cells. Logical positions, collision and interaction range stay untouched. Stacked on the Bitcoindefi#16 clip, whose solid-cell reservation it reuses.

gitar-bot · 2026-08-25T16:23:43Z

Code Review ⚠️ Changes requested 0 resolved / 3 findings

Updates resident and hero sprite collision logic to prevent wall painting and overlap, but the heroGeom null deref crashes glyph-hero + overlapping actor.

⚠️

Bug: heroGeom null deref crashes glyph-hero + overlapping actor

📄 lib/render.js:760-774 📄 lib/render.js:828-834

When the hero has no sprite (glyph fallback), the reservation block leaves heroGeom = null but still reserves the hero cell hy,hx. If an actor's drawn footprint covers that cell, collidesHero(left) returns true and the code dereferences heroGeom.left/heroGeom.width (lines 831-832), throwing TypeError: Cannot read properties of null and crashing the render. Set a fallback geometry in the else branch (e.g. heroGeom = { left: hx, width: 1 }) so the separation maths works for glyph heroes too.

Give the glyph-hero fallback a valid geometry so the actor step-aside pass never dereferences null.

} else {
  heroReserved.add(hy + ',' + hx)
  heroGeom = { left: hx, width: 1 }
}

💡 Quality: actorOccupied set is populated but never read

📄 lib/render.js:759 📄 lib/render.js:859

actorOccupied is declared at line 759 and written to at line 859 but never read anywhere in the function or elsewhere in the file. It is dead code that adds per-cell overhead each frame. Remove the declaration and the .add() call unless a follow-up consumer is intended.

💡 Bug: actorW uses raw length, diverging from ascii-normalized paint

📄 lib/render.js:804 📄 lib/render.js:820

The reservation/paint loops normalize actor sprite rows with ascii(...).padEnd(actorW), but actorW is now computed from raw line.length (line 804) after this PR dropped the previous ascii(line).length. For sprites containing astral (surrogate-pair) characters the two widths disagree, so padEnd(actorW) and the collision scan can misalign. NPC art is currently ASCII so impact is latent; restore ascii(line).length to keep width computation consistent with rendering.

Compute actorW from the ascii-normalized width, matching the loops that render the sprite.

const actorW = actorSprite.reduce((widest, line) => Math.max(widest, ascii(line).length), 1)

🤖 Prompt for agents

Code Review: Updates resident and hero sprite collision logic to prevent wall painting and overlap, but the heroGeom null deref crashes glyph-hero + overlapping actor.

1. ⚠️ Bug: heroGeom null deref crashes glyph-hero + overlapping actor
   Files: lib/render.js:760-774, lib/render.js:828-834

   When the hero has no sprite (glyph fallback), the reservation block leaves `heroGeom = null` but still reserves the hero cell `hy,hx`. If an actor's drawn footprint covers that cell, `collidesHero(left)` returns true and the code dereferences `heroGeom.left`/`heroGeom.width` (lines 831-832), throwing `TypeError: Cannot read properties of null` and crashing the render. Set a fallback geometry in the else branch (e.g. `heroGeom = { left: hx, width: 1 }`) so the separation maths works for glyph heroes too.

   Fix (Give the glyph-hero fallback a valid geometry so the actor step-aside pass never dereferences null.):
   } else {
     heroReserved.add(hy + ',' + hx)
     heroGeom = { left: hx, width: 1 }
   }

2. 💡 Quality: actorOccupied set is populated but never read
   Files: lib/render.js:759, lib/render.js:859

   `actorOccupied` is declared at line 759 and written to at line 859 but never read anywhere in the function or elsewhere in the file. It is dead code that adds per-cell overhead each frame. Remove the declaration and the `.add()` call unless a follow-up consumer is intended.

3. 💡 Bug: actorW uses raw length, diverging from ascii-normalized paint
   Files: lib/render.js:804, lib/render.js:820

   The reservation/paint loops normalize actor sprite rows with `ascii(...).padEnd(actorW)`, but `actorW` is now computed from raw `line.length` (line 804) after this PR dropped the previous `ascii(line).length`. For sprites containing astral (surrogate-pair) characters the two widths disagree, so `padEnd(actorW)` and the collision scan can misalign. NPC art is currently ASCII so impact is latent; restore `ascii(line).length` to keep width computation consistent with rendering.

   Fix (Compute actorW from the ascii-normalized width, matching the loops that render the sprite.):
   const actorW = actorSprite.reduce((widest, line) => Math.max(widest, ascii(line).length), 1)

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

Important

Your trial ends in 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}

leocagli · 2026-08-25T17:24:11Z

Merged onto current main, the tests pass. npm run lint does not:

Checking formatting...
[warn] <the new test file>
[warn] Code style issues found in the above file. Run Prettier with --write to fix.

The repository runs prettier . --check as part of npm run lint, and CI runs
lint as its own job, so this alone turns the build red.

npm run format fixes it in one pass. Nothing else needs to change; the code itself
is fine.

blippip69 mentioned this pull request Aug 25, 2026

el heroe le come el dibujo al NPC justo a la distancia en la que se puede hablar #9

Closed

gitar-bot Bot reviewed Aug 25, 2026

View reviewed changes

byblik added 2 commits August 25, 2026 19:19

blippip69 force-pushed the fix/npc-hero-sprite-separation branch from bfca610 to 98f69ee Compare August 25, 2026 16:20

Navigation Menu

Uh oh!

fix: residents step aside instead of trading pixels with the hero (#9) - #43

fix: residents step aside instead of trading pixels with the hero (#9)#43
blippip69 wants to merge 2 commits into
Bitcoindefi:mainfrom
blippip69:fix/npc-hero-sprite-separation

blippip69 commented Aug 25, 2026 •

edited by gitar-bot Bot

Loading

Uh oh!

gitar-bot Bot Aug 25, 2026

Uh oh!

gitar-bot Bot Aug 25, 2026

Uh oh!

gitar-bot Bot Aug 25, 2026

Uh oh!

gitar-bot Bot commented Aug 25, 2026

Uh oh!

leocagli commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

blippip69 commented Aug 25, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Gitar

Uh oh!

gitar-bot Bot Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot Bot commented Aug 25, 2026

Uh oh!

leocagli commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

blippip69 commented Aug 25, 2026 •

edited by gitar-bot Bot

Loading