diff --git a/docs/translation_log.md b/docs/translation_log.md index abdab7dc..eaf48650 100644 --- a/docs/translation_log.md +++ b/docs/translation_log.md @@ -2026,3 +2026,51 @@ The original pret visibility range test used 8-bit unsigned byte arithmetic: **Critical:** Lower-bound subtraction must use 32-bit signed registers — `sub al, 3` wraps to `0xFC` when `wYCoord=0`, culling every NPC. Fix: `movzx eax; lea ecx,[eax-N]; cmp ecx,edx; jg .invisible`. --- + +## UpdateNonPlayerSprite / NPC walk state machine + +- **Source:** `engine/overworld/movement.asm:UpdateNPCSprite` and helpers (pret lines 99–370, 556–666, 990–1016) +- **Translated:** `dos_port/src/engine/overworld/movement.asm` +- **Date:** 2026-06-23 +- **H-flag:** not involved +- **Bug tags:** BUG(cosmetic) Yellow south-displacement fix applied (see below) + +### Summary + +Full NPC random-walk state machine: status dispatch, delay countdown, direction selection with UP_DOWN/LEFT_RIGHT/forced-dir constraints, tile passability + collision + displacement bounds check, walk-pixel interpolation, and animation counter. + +### Functions translated + +| Pret label | DOS label | Notes | +|---|---|---| +| `UpdateNPCSprite` | `UpdateNonPlayerSprite` | Status 0→init, 1→ready, 2→delay, 3→walk; BIT_FACE_PLAYER stub | +| `Func_5337` | `Func_5337` | Write FACINGDIRECTION/YSTEPVECTOR/XSTEPVECTOR to sprite slot | +| `Func_5349` | `Func_5349` | Advance MAPY/MAPX to destination at walk START (not end) | +| `TryWalking` | `TryWalking` | Call Func_5337 → CanWalkOntoTile → Func_5349 → STATUS=3 | +| `CanWalkOntoTile` | `CanWalkOntoTile` | IsTilePassable + STAY check + displacement bounds + DetectCollision | +| `UpdateSpriteMovementDelay` | `UpdateSpriteMovementDelay` | Decrement MOVEMENTDELAY; 0 → STATUS=1, fall into NotYetMoving | +| `NotYetMoving` | `NotYetMoving` | Reset ANIMFRAMECOUNTER, UpdateSpriteImage | +| `UpdateSpriteInWalkingAnimation` | `UpdateSpriteInWalkingAnimation` | pixel-interpolation (YPIXELS/XPIXELS += YSTEP/XSTEP), WALKANIMCOUNTER | +| `Random` | `Random` | Thin wrapper: saves/restores EBX, calls `Random_`, returns H_RANDOM_ADD in AL | + +### SPRITESTATEDATA2 constants bug fixed + +`gb_memmap.inc` had MOVEMENTDELAY at offset 0x1 (unused slot) and MOVEMENTBYTE2 at 0x8 (the real MOVEMENTDELAY slot). This caused map_sprites.asm to write direction constraints to slot 0x8 and delays to slot 0x1. Fix: swap them to match pret (MOVEMENTBYTE2=0x1, MOVEMENTDELAY=0x8). Because map_sprites.asm uses symbolic constants, the write offsets corrected automatically. + +### Func_5349 timing — teleport-prevention + +Pret advances MAPY/MAPX to the **destination** at walk **start** (inside `TryWalking`, before the first pixel step). PrepareOAMData's `dos_base_npc` formula therefore subtracts `YSTEP × WALKANIMCOUNTER` and `XSTEP × WALKANIMCOUNTER` to interpolate back to the source position, counting down to 0 at walk end. Without this, NPCs would appear to teleport one metatile and slide back. + +### wMapSpriteData indirection eliminated + +Pret's `UpdateNPCSprite` reads the direction constraint (`wCurSpriteMovement2`) via a separate `wMapSpriteData` pointer array. The DOS port stores the constraint directly in `SPRITESTATEDATA2[MOVEMENTBYTE2]` (offset 0x1), set by `InitMapSprites`. No separate array needed. + +### Yellow south-displacement fix + +Red/Blue had a bug: the south-displacement upper bound used `cmp a, 5; jnc .blocked` — the same condition as the north lower bound — which meant NPCs could only move 4 tiles south of their starting position. Yellow fixed this by removing the south upper bound check. The DOS port follows Yellow behavior (no south or east upper bound). + +### Random_ / IO_DIV + +`random.asm`'s LCG reads `IO_DIV` (at `[EBP + 0xFF04]`). Previously always 0 (emulated but not driven). Fixed by incrementing `IO_DIV` once per frame inside `commit_shadow_regs` (`frame.asm`) so the LCG has changing input. Verified live: NPCs walk with varied directions and delays. + +--- diff --git a/dos_port/Makefile b/dos_port/Makefile index 0d7101dc..fdcab1e0 100644 --- a/dos_port/Makefile +++ b/dos_port/Makefile @@ -92,7 +92,8 @@ GAME_SRCS := \ src/movie/title.asm \ src/engine/overworld/overworld.asm \ src/engine/overworld/movement.asm \ - src/engine/overworld/map_sprites.asm + src/engine/overworld/map_sprites.asm \ + src/engine/math/random.asm # Debug-only sources (linked only when the corresponding flag is set). # Any debug flag implies SKIP_TITLE — debug harnesses boot straight to the overworld. diff --git a/dos_port/include/gb_memmap.inc b/dos_port/include/gb_memmap.inc index c56024e5..8898415d 100644 --- a/dos_port/include/gb_memmap.inc +++ b/dos_port/include/gb_memmap.inc @@ -150,17 +150,38 @@ SPRITESTATEDATA2_XDISPLACEMENT equ 0x3 ; X displacement (init 8) SPRITESTATEDATA2_MAPY equ 0x4 ; map Y position (metatile units, origin+4) SPRITESTATEDATA2_MAPX equ 0x5 ; map X position (metatile units, origin+4) SPRITESTATEDATA2_WALKANIMCOUNTER equ 0x0 ; 8-frame countdown while walking (0=standing) -SPRITESTATEDATA2_MOVEMENTDELAY equ 0x1 ; frames until next random walk attempt +SPRITESTATEDATA2_MOVEMENTBYTE2 equ 0x1 ; dir constraint from map object (pret unnamed 0x01) +SPRITESTATEDATA2_MOVEMENTDELAY equ 0x8 ; frames until next random walk attempt (pret 0x08) SPRITESTATEDATA2_MOVEMENTBYTE1 equ 0x6 ; $ff=STAY, $fe=WALK(random), <$fe=scripted SPRITESTATEDATA2_GRASSPRIORITY equ 0x7 ; $80=under grass (set by CheckSpriteAvailability) -SPRITESTATEDATA2_MOVEMENTBYTE2 equ 0x8 ; dir constraint from map object (0x00/0x01/0x02/0xFF) -SPRITESTATEDATA2_ISTRAINER equ 0x9 ; 1 if trainer NPC, 0 otherwise +SPRITESTATEDATA2_ISTRAINER equ 0x9 ; 1 if trainer NPC, 0 otherwise (pret: ORIGFACINGDIR) SPRITESTATEDATA2_IMAGEBASEOFFSET equ 0xE ; NPC movement-byte sentinel values (constants/map_object_constants.asm) WALK equ 0xFE ; random movement STAY equ 0xFF ; stationary +; NPC direction constraint values (movement byte 2 / pret wMapSpriteData byte 0) +; constants/map_object_constants.asm: ANY_DIR=0x00, UP_DOWN=0x01, LEFT_RIGHT=0x02 +; Forced single-direction: DOWN=0xD0, UP=0xD1, LEFT=0xD2, RIGHT=0xD3, NONE=0xFF +NPC_DIR_ANY equ 0x00 ; any random direction (pret: ANY_DIR) +NPC_DIR_UPDOWN equ 0x01 ; constrained to up/down only (pret: UP_DOWN) +NPC_DIR_LR equ 0x02 ; constrained to left/right only (pret: LEFT_RIGHT) +NPC_DIR_DOWN equ 0xD0 ; forced down (pret: DOWN) +NPC_DIR_UP equ 0xD1 ; forced up +NPC_DIR_LEFT equ 0xD2 ; forced left +NPC_DIR_RIGHT equ 0xD3 ; forced right +NPC_DIR_NONE equ 0xFF ; STAY NPC (no movement constraint) +UP_DOWN equ NPC_DIR_UPDOWN +LEFT_RIGHT equ NPC_DIR_LR + +; NPC random movement direction thresholds (high 2 bits of hRandomAdd) +; constants/sprite_data_constants.asm +NPC_MOVEMENT_DOWN equ 0x00 ; hRandomAdd in [0x00,0x3F] → go down +NPC_MOVEMENT_UP equ 0x40 ; hRandomAdd in [0x40,0x7F] → go up +NPC_MOVEMENT_LEFT equ 0x80 ; hRandomAdd in [0x80,0xBF] → go left +NPC_MOVEMENT_RIGHT equ 0xC0 ; hRandomAdd in [0xC0,0xFF] → go right + ; Named WRAM symbols (subset needed by translated routines). ; Offsets from pokeyellow.sym (bank 00). W_SHADOW_OAM equ 0xC300 ; wShadowOAM — 40 sprites × 4 bytes diff --git a/dos_port/src/engine/overworld/movement.asm b/dos_port/src/engine/overworld/movement.asm index 1d3b4883..07f62965 100644 --- a/dos_port/src/engine/overworld/movement.asm +++ b/dos_port/src/engine/overworld/movement.asm @@ -6,16 +6,20 @@ ; engine/overworld/movement.asm:UpdatePlayerSprite / UpdateNonPlayerSprite / ; InitializeSpriteStatus / InitializeSpriteScreenPosition / Func_5033 / ; CheckSpriteAvailability / GetTileSpriteStandsOn / UpdateSpriteImage / +; UpdateSpriteMovementDelay / UpdateSpriteInWalkingAnimation / NotYetMoving / +; TryWalking / CanWalkOntoTile / Func_5337 / Func_5349 / ; Func_4e32 / Func_5274 ; ; UpdateSprites runs once per overworld-loop iteration. It walks all 16 sprite ; slots: slot 0 → UpdatePlayerSprite (facing + walk animation); slots 1-15 → -; UpdateNonPlayerSprite (static NPCs: screen-position + image index from -; wSpriteStateData1/2, no movement engine yet). PrepareOAMData (sprite_oam.asm) +; UpdateNonPlayerSprite (full NPC walk state machine: delay countdown, 16-frame +; pixel-step animation, direction selection with UP_DOWN/LEFT_RIGHT constraints, +; tile passability + sprite-collision gating). PrepareOAMData (sprite_oam.asm) ; then turns the image indices into shadow-OAM entries each DelayFrame. ; -; NPC scope: static NPCs only (MOVEMENTSTATUS 0→1 init, CheckSpriteAvailability, -; InitializeSpriteScreenPosition). Random/scripted NPC movement is deferred. +; Direction constraint is read from SPRITESTATEDATA2_MOVEMENTBYTE2 (offset 0x1), +; replacing pret's wMapSpriteData/wCurSpriteMovement2 indirection. +; Scripted NPC movement (MOVEMENTBYTE1 < WALK) is a stub — not yet implemented. ; ; Build: nasm -f coff -I include/ -I . -o movement.o src/engine/overworld/movement.asm @@ -26,6 +30,9 @@ bits 32 global UpdateSprites +extern IsTilePassable +extern Random_ + section .text ; --------------------------------------------------------------------------- @@ -73,14 +80,22 @@ _UpdateSprites: ret ; --------------------------------------------------------------------------- -; UpdateNonPlayerSprite — static NPC screen-position + image-index update. -; Pret ref: engine/overworld/movement.asm:UpdateNonPlayerSprite. +; UpdateNonPlayerSprite — full NPC walk state machine. +; Pret ref: engine/overworld/movement.asm:UpdateNPCSprite. ; -; Scope: static NPCs only (MOVEMENTBYTE1 = STAY). Movement engine deferred. +; Status dispatch: +; 0 → InitializeSpriteStatus (first-frame init) +; 1 → CheckSpriteAvailability; if visible and player not walking: direction +; selection → TryWalking (WALK/STAY) or stub (scripted 0. + ; Moving south (DH=0x01): always allowed (Yellow bug-fix: no upper bound). + movzx eax, byte [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_YDISPLACEMENT] + test dh, dh + jz .checkXDisp ; DH=0: not moving vertically + js .moveNorth ; DH=0xFF (bit 7 set): moving north + ; moving south: increment displacement, no upper bound (Yellow fix) + add al, 1 + mov [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_YDISPLACEMENT], al + jmp .checkXDisp +.moveNorth: + sub al, 1 ; YDISPLACEMENT - 1; CF=1 if was 0 → blocked + jc .impassable + mov [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_YDISPLACEMENT], al + +.checkXDisp: + movzx eax, byte [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_XDISPLACEMENT] + test dl, dl + jz .detectCollision + js .moveWest + ; moving east: always allowed + add al, 1 + mov [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_XDISPLACEMENT], al + jmp .detectCollision +.moveWest: + sub al, 1 + jc .impassable + mov [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_XDISPLACEMENT], al + +.detectCollision: + ; Run collision detection to populate COLLISIONDATA for direction check. + ; Must save/restore wUpdateSpritesEnabled (pret pattern). + movzx eax, byte [ebp + W_UPDATE_SPRITES_ENABLED] + push eax + mov byte [ebp + W_UPDATE_SPRITES_ENABLED], 0xFF + call DetectCollisionBetweenSprites ; preserves EBX, ECX, EDX, ESI, EDI + pop eax + mov [ebp + W_UPDATE_SPRITES_ENABLED], al + ; BL = direction bit (preserved through DetectCollisionBetweenSprites) + mov al, [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_COLLISIONDATA] + test al, bl + jnz .impassable + + clc + ret + +.impassable: + ; Set status=2 (delayed), zero step vectors, assign random delay. + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_MOVEMENTSTATUS], 2 + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_YSTEPVECTOR], 0 + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_XSTEPVECTOR], 0 + call Random ; AL = H_RANDOM_ADD (clobbers AL, BL) + and al, 0x7F ; random 0–127 frames + mov [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_MOVEMENTDELAY], al + stc + ret + +; --------------------------------------------------------------------------- +; UpdateSpriteMovementDelay — decrement inter-walk delay, transition to status 1. +; Pret ref: engine/overworld/movement.asm:UpdateSpriteMovementDelay. +; In: ESI = slot byte offset. Clobbers AL. +; Falls through to NotYetMoving when ready. +; --------------------------------------------------------------------------- +UpdateSpriteMovementDelay: + mov al, [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_MOVEMENTBYTE1] + cmp al, WALK + jnc .tickCounter ; WALK or STAY: decrement counter + ; Scripted: clear delay immediately → ready to move + mov byte [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_MOVEMENTDELAY], 0 + jmp .moving + +.tickCounter: + mov al, [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_MOVEMENTDELAY] + dec al + mov [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_MOVEMENTDELAY], al + jnz NotYetMoving ; still waiting: freeze animation + +.moving: + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_MOVEMENTSTATUS], 1 + ; fall through to NotYetMoving + +; --------------------------------------------------------------------------- +; NotYetMoving — reset NPC animation counter and refresh image index. +; Pret ref: engine/overworld/movement.asm:NotYetMoving. +; Called whenever the NPC's visual state needs refreshing but position doesn't change. +; In: ESI = slot byte offset. Clobbers AL. +; --------------------------------------------------------------------------- +NotYetMoving: + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_ANIMFRAMECOUNTER], 0 + call UpdateSpriteImage + ret + +; --------------------------------------------------------------------------- +; UpdateSpriteInWalkingAnimation — advance one frame of NPC pixel-step walk. +; Pret ref: engine/overworld/movement.asm:UpdateSpriteInWalkingAnimation. +; +; Per-frame: advance anim counters (Func_5274); YPIXELS+=YSTEP; XPIXELS+=XSTEP; +; decrement WALKANIMCOUNTER. When counter reaches 0: +; WALK/STAY → random delay (0–127) + status=2; clear step vectors. +; scripted → status=1 (ready for next scripted step). +; +; In: ESI = slot byte offset (H_CURRENT_SPRITE_OFFSET set by outer loop). +; Clobbers AL, DL (from Func_5274). +; --------------------------------------------------------------------------- +UpdateSpriteInWalkingAnimation: + call Func_5274 ; advance intra-anim and anim-frame counters + + ; YPIXELS += YSTEPVECTOR + mov al, [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_YSTEPVECTOR] + add [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_YPIXELS], al + + ; XPIXELS += XSTEPVECTOR + mov al, [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_XSTEPVECTOR] + add [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_XPIXELS], al + + ; Decrement walk animation counter; if still > 0, animation continues + mov al, [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_WALKANIMCOUNTER] + dec al + mov [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_WALKANIMCOUNTER], al + jnz .animRunning + + ; Walk finished: check if random (WALK/STAY) or scripted + mov al, [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_MOVEMENTBYTE1] + cmp al, WALK + jnc .initNextCounter ; WALK or STAY → random inter-walk delay + + ; Scripted: immediately ready for next scripted step + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_MOVEMENTSTATUS], 1 + ret + +.initNextCounter: + call Random ; AL = H_RANDOM_ADD + and al, 0x7F ; random 0–127 frames + mov [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_MOVEMENTDELAY], al + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_MOVEMENTSTATUS], 2 + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_YSTEPVECTOR], 0 + mov byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_XSTEPVECTOR], 0 + ret + +.animRunning: + ret + ; --------------------------------------------------------------------------- ; InitializeSpriteStatus — first-frame NPC init. ; Pret ref: engine/overworld/movement.asm:InitializeSpriteStatus. diff --git a/dos_port/src/engine/overworld/overworld.asm b/dos_port/src/engine/overworld/overworld.asm index 6b8843b5..b052a065 100644 --- a/dos_port/src/engine/overworld/overworld.asm +++ b/dos_port/src/engine/overworld/overworld.asm @@ -74,7 +74,7 @@ global LoadCurrentMapView global CopyMapViewToVRAM global OverworldLoop global AdvancePlayerSprite -global AdvancePlayerSprite +global IsTilePassable global CheckWarpTile global LoadWarpDestination global PlayerStepOutFromDoor diff --git a/dos_port/src/gfx/sprite_oam.asm b/dos_port/src/gfx/sprite_oam.asm index 39e26edb..0d9e7369 100644 --- a/dos_port/src/gfx/sprite_oam.asm +++ b/dos_port/src/gfx/sprite_oam.asm @@ -135,6 +135,18 @@ PrepareOAMData: imul eax, 16 add eax, 96 mov [dos_base_x_tmp], eax + ; NPC walk interpolation: if MOVEMENTSTATUS=3 (walking), Func_5349 already advanced + ; MAPY/MAPX to the destination at walk start. Subtract YSTEP*WALKANIMCOUNTER + ; to interpolate between source and destination over the 16-frame animation. + cmp byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_MOVEMENTSTATUS], 3 + jne .dos_base_done + movzx eax, byte [ebp + esi + W_SPRITE_STATE_DATA_2 + SPRITESTATEDATA2_WALKANIMCOUNTER] + movsx ecx, byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_YSTEPVECTOR] + imul ecx, eax + sub [dos_base_y_tmp], ecx + movsx ecx, byte [ebp + esi + W_SPRITE_STATE_DATA_1 + SPRITESTATEDATA1_XSTEPVECTOR] + imul ecx, eax + sub [dos_base_x_tmp], ecx .dos_base_done: ; Sub-block walk tracking: subtract the player's current walk pixel offset from ; NPC dos_base so NPCs scroll in lockstep with the BG during a walk step. diff --git a/dos_port/src/video/frame.asm b/dos_port/src/video/frame.asm index 293b65d9..3698dc41 100644 --- a/dos_port/src/video/frame.asm +++ b/dos_port/src/video/frame.asm @@ -98,6 +98,7 @@ update_oam: ; --------------------------------------------------------------------------- commit_shadow_regs: push eax + inc byte [ebp + IO_DIV] ; advance emulated DIV counter (~16384 Hz on GB; 1/frame is enough for RNG entropy) mov al, [ebp + H_SCX] mov [ebp + IO_SCX], al mov al, [ebp + H_SCY]