From 95196676a01bd90082eb00df1d504b7cb44e4d3f Mon Sep 17 00:00:00 2001 From: Happyarch Date: Fri, 19 Jun 2026 13:24:33 -0400 Subject: [PATCH] Fix collision bypass: refresh wTileMap before every check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AdvancePlayerSprite calls LoadCurrentMapView only on block-boundary crossings (coord value == 0x02 or 0xFF). Each direction has one non-crossing sub-step (SOUTH YBC 0→1, NORTH YBC 1→0, EAST XBC 0→1, WEST XBC 1→0) that leaves wTileMap stale. At that stale wTileMap, GetTileInFrontOfPlayer's compile-time-constant read offset points to the player's own current tile in wSurroundingTiles, which is always passable — so every other step in every direction bypassed collision entirely, allowing the player to walk through signs, ledges, and bushes. Fix: call LoadCurrentMapView at the top of CollisionCheckOnLand. YBC/XBC and the view pointer are always mutually consistent at W_WALK_COUNTER=0 (updated atomically in AdvancePlayerSprite), so this always produces the correct wTileMap. Cost is ~2700 bytes of memory writes once per 8 frames. Also corrects two stale inline comments that referenced the old SURROUNDING_WIDTH value (24/44 instead of the actual 48), and updates TODO.md with the correct four-direction root-cause analysis. Co-Authored-By: Claude Sonnet 4.6 --- TODO.md | 40 ++++++++++++++++------------ dos_port/src/overworld/overworld.asm | 9 ++++--- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/TODO.md b/TODO.md index c595be88..93376ca1 100644 --- a/TODO.md +++ b/TODO.md @@ -149,23 +149,29 @@ Prioritized task list. Check off items as they complete; add new items with phas constants to `gb_memmap.inc` and updating all five read sites. Also fixed `gen_map_headers.py` NORTH (−4 → −5) and WEST (−6 → −7) view-pointer formulas which had the same off-by-one due to YBC=1/XBC=1 row/col skipping. -- [ ] **BUG — holding a direction lets the player walk ~1 tile into an impassable - block before being stopped.** Root cause: for SOUTH and EAST movement, - `wYBlockCoord`/`wXBlockCoord` goes 0→1 on the first sub-step without - triggering `LoadCurrentMapView` (view only updates on the 1→2 crossing). - The collision check at `W_WALK_COUNTER=0` reads the stale `wTileMap`, which - still reflects the pre-sub-step view — so it checks the last tile of the - player's CURRENT block instead of the first tile of the NEXT block. For - NORTH and WEST the block-coord underflows (0→FF) on the first sub-step, - which IS a crossing and immediately updates the view, so those directions - are unaffected. Fix: in `GetTileInFrontOfPlayer` (overworld.asm ~line 1131), - add the sub-block byte-offset to the wTileMap address: - Down: `row = PLAYER_STANDING_ROW + W_Y_BLOCK_COORD*2 + 2` - Right: `col = PLAYER_STANDING_COL + W_X_BLOCK_COORD*2 + 2` - (Up and Left don't need adjustment — view is always current for those.) - Note: the original GB has the same stale-view window but standard map blocks - are 4 tiles deep so collision is always caught on the second sub-step. Our - port may have thinner collision geometry in some spots causing visible bleed. +- [x] **BUG — holding a direction lets the player walk through impassable tiles + (signs, ledges, bushes) as long as they are ≤4 tiles deep.** Root cause: + `AdvancePlayerSprite` updates the block-coord and calls `LoadCurrentMapView` + only when the new coord value is exactly `0x02` (south/east crossing) or + `0xFF` (north/west crossing). Each direction has ONE non-crossing sub-step: + SOUTH YBC 0→1 | NORTH YBC 1→0 | EAST XBC 0→1 | WEST XBC 1→0 + On these steps `wTileMap` is left stale. At `W_WALK_COUNTER=0` the + subsequent collision check calls `GetTileInFrontOfPlayer`, which reads the + stale `wTileMap` at a compile-time-constant offset — in all four directions + that offset maps to the player's *own* current tile in `wSurroundingTiles`, + which is always passable. Result: every other step in every direction + bypasses collision entirely, allowing penetration up to one full block (4 + tiles). Objects one block deep with passable lower rows (signs, ledges) can + be traversed completely. Prior analysis incorrectly claimed NORTH/WEST were + unaffected because "0→FF is a crossing": it confused which sub-step is the + crossing one — NORTH 0→FF *is* a crossing and *does* call LCV (correct), but + NORTH 1→0 is NOT a crossing and does NOT call LCV (buggy). + **Fixed** (2026-06-19): added `call LoadCurrentMapView` at the top of + `CollisionCheckOnLand` (overworld.asm). YBC/XBC and the view pointer are + always mutually consistent at `W_WALK_COUNTER=0` (updated atomically in + `AdvancePlayerSprite`), so calling LCV here always produces the correct + `wTileMap` regardless of direction. Cost: ~2700 bytes of memory writes once + per 8 frames while a direction is held — negligible on a 386. - [ ] **BUG — rapid direction change can bypass or falsely trigger collision.** Tapping a new direction during the last frames of a walk step can flip `W_SPRITE_PLAYER_FACING_DIR` before the walk counter reaches 0, so the next diff --git a/dos_port/src/overworld/overworld.asm b/dos_port/src/overworld/overworld.asm index 6bc10867..780fb2d7 100644 --- a/dos_port/src/overworld/overworld.asm +++ b/dos_port/src/overworld/overworld.asm @@ -758,8 +758,8 @@ DrawTileBlock: mov al, byte [ebp + edx] mov byte [ebp + esi], al inc edx - ; Advance ESI to start of next tile row: +21 = SURROUNDING_WIDTH - (BLOCK_WIDTH-1) - add esi, SURROUNDING_WIDTH - BLOCK_WIDTH + 1 ; = 24 - 3 = 21 + ; Advance ESI to start of next tile row: +45 = SURROUNDING_WIDTH - (BLOCK_WIDTH-1) + add esi, SURROUNDING_WIDTH - BLOCK_WIDTH + 1 ; = 48 - 4 + 1 = 45 pop ecx dec cl jnz .draw_row @@ -881,8 +881,8 @@ LoadCurrentMapView: dec bl jnz .copy_col_loop - ; Skip SURROUNDING_WIDTH - SCREEN_WIDTH = 4 bytes to reach next row in wSurroundingTiles - add esi, SURROUNDING_WIDTH - SCREEN_WIDTH ; = 44 - 40 = 4 + ; Skip SURROUNDING_WIDTH - SCREEN_WIDTH = 8 bytes to reach next row in wSurroundingTiles + add esi, SURROUNDING_WIDTH - SCREEN_WIDTH ; = 48 - 40 = 8 dec bh jnz .copy_row_loop @@ -1106,6 +1106,7 @@ CollisionCheckOnLand: push eax push ecx push esi + call LoadCurrentMapView ; refresh wTileMap for current YBC/XBC before check call GetTileInFrontOfPlayer ; CL = tile in front call IsTilePassable ; CF = 1 if not passable pop esi