Restore LoadCurrentMapView in CollisionCheckOnLand (collision regression fix)

Removing it in the previous commit caused a regression: walking toward any
2x2 block of impassable tiles (route 1 bushes, building walls, ledges)
would sporadically let the player pass through.

Root cause: LoadCurrentMapView doesn't just rebuild wSurroundingTiles — it
also re-derives the wTileMap viewport from wSurroundingTiles offset by
W_Y_BLOCK_COORD / W_X_BLOCK_COORD (lines 1114-1135). AdvancePlayerSprite
only calls it on block-boundary crossings, so within a block, YBC/XBC can
change (0→1) while wTileMap keeps the old offset. GetTileInFrontOfPlayer
then reads a stale tile — 1 tile row/col off — which maps to the passable
side of a 2x2 impassable cluster.

The call is correctly placed in CollisionCheckOnLand. A future optimisation
can split out just the viewport-copy portion since wSurroundingTiles is
already current, but correctness is the priority.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Happyarch
2026-06-20 18:22:15 -04:00
parent 445c6a3ad9
commit 77fe892e65
2 changed files with 17 additions and 5 deletions

View File

@@ -660,12 +660,19 @@ worked around this by priming `wPlayerLastStopDirection = PLAYER_DIR_DOWN` in
`.handleDirection`, before the turn-delay check. Removed the `wPlayerLastStopDirection`
prime from `PlayerStepOutFromDoor`.
### Cleanup — `LoadCurrentMapView` removed from `CollisionCheckOnLand`
### `LoadCurrentMapView` in `CollisionCheckOnLand` — why it's required
The previous session added `call LoadCurrentMapView` inside `CollisionCheckOnLand`,
called on every direction press. `wTileMap` is always current at that point (built
by `LoadWarpDestination` on map load, and by `AdvancePlayerSprite` on every
block-boundary crossing), so the call was redundant every idle frame.
`LoadCurrentMapView` rebuilds `wSurroundingTiles` from the block map AND copies a
sub-block-offset viewport into `wTileMap` based on `W_Y_BLOCK_COORD`/`W_X_BLOCK_COORD`.
`AdvancePlayerSprite` only calls it on block-boundary crossings. Between crossings
YBC/XBC can advance 0→1 without triggering a rebuild, leaving `wTileMap` at the
previous sub-block viewport offset. `GetTileInFrontOfPlayer` then reads the wrong tile.
Symptom: walking toward a 2×2 cluster of impassable tiles (route 1 bushes, building
outer walls, ledges) sporadically passes through — at the half-block sub-step the
tile read lands on the adjacent passable tile instead of the correct one. The call is
retained in `CollisionCheckOnLand`. A future optimisation could split out just the
viewport-copy step (lines 11141135) since `wSurroundingTiles` is already current.
### Also in this commit

View File

@@ -1369,6 +1369,11 @@ CollisionCheckOnLand:
push eax
push ecx
push esi
; wTileMap is a sub-block viewport into wSurroundingTiles, offset by W_Y_BLOCK_COORD /
; W_X_BLOCK_COORD. AdvancePlayerSprite only calls LoadCurrentMapView on block-boundary
; crossings, so the viewport can be stale within a block (YBC/XBC changed but wTileMap
; not rebuilt). Rebuild here to apply the current sub-block offset before the tile read.
call LoadCurrentMapView
call GetTileInFrontOfPlayer ; CL = tile in front
call IsTilePassable ; CF = 1 if not passable
pop esi