Add grid-based movement and wall glyph rendering#13
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR transforms the movement system from an analytic/continuous movement model to a grid-aligned, turn-based movement system suitable for a roguelike game. The changes support diagonal movement with traditional roguelike keybindings and add visual rendering of wall tiles.
Key Changes:
- Replaced smooth analytic movement with discrete grid-step movement (±1 grid unit per move)
- Added diagonal movement support with numpad and vi-style keybindings (y/u/b/n keys)
- Implemented wall tile visualization to enhance dungeon rendering
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/rules/systems/movementSystem.js | Refactored from analytic to grid-aligned movement with helper functions for grid snapping, step clamping, and direction normalization |
| src/rules/systems/aiChaseSystem.js | Updated AI to use discrete directional steps (±1) instead of continuous vector movement |
| src/main.js | Added wall tile detection and rendering system to visualize dungeon walls adjacent to floor tiles |
| src/display/input/InputManager.js | Added diagonal movement keybindings including numpad (1,3,7,9) and vi-style keys (y,u,b,n), plus Numpad5 for wait action |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const vec = normalizeDir(dirx, diry); | ||
| if (Math.abs(vec.x) < DIR_EPS && Math.abs(vec.y) < DIR_EPS) { | ||
| return; | ||
| } | ||
| const fx = world.get(actor, Facing); | ||
| if (fx) { | ||
| world.set(actor, Facing, { x: vec.x, y: vec.y }); | ||
| } else { | ||
| try { world.add(actor, Facing, { x: vec.x, y: vec.y }); } catch { } |
There was a problem hiding this comment.
The updateFacing function calls normalizeDir on arguments that are already passed in, but then normalizes them again. This is redundant since normalizeDir is called both at the call site (line 151) and inside the function. Consider either:
- Removing the
normalizeDircall insideupdateFacingand accepting already-normalized direction vectors, or - Accepting raw direction values and only normalizing inside the function.
Currently, the direction is normalized twice which is inefficient.
| const vec = normalizeDir(dirx, diry); | |
| if (Math.abs(vec.x) < DIR_EPS && Math.abs(vec.y) < DIR_EPS) { | |
| return; | |
| } | |
| const fx = world.get(actor, Facing); | |
| if (fx) { | |
| world.set(actor, Facing, { x: vec.x, y: vec.y }); | |
| } else { | |
| try { world.add(actor, Facing, { x: vec.x, y: vec.y }); } catch { } | |
| if (Math.abs(dirx) < DIR_EPS && Math.abs(diry) < DIR_EPS) { | |
| return; | |
| } | |
| const fx = world.get(actor, Facing); | |
| if (fx) { | |
| world.set(actor, Facing, { x: dirx, y: diry }); | |
| } else { | |
| try { world.add(actor, Facing, { x: dirx, y: diry }); } catch { } |
| @@ -48,108 +89,77 @@ export function movementSystem(world) { | |||
| if (!pos) { world.remove(actor, MoveIntent); continue; } | |||
|
|
|||
| const anatomy = world.get(actor, Anatomy); | |||
There was a problem hiding this comment.
The anatomy variable is retrieved but never used in the simplified grid-based movement system. This variable was used in the old implementation to get strideDistance, but is now only used later for reachDistance (line 159). Consider moving the anatomy retrieval to where it's actually needed (before line 159) to improve code clarity and avoid unnecessary lookups.
| for (let i = 0; i < floorTiles.length; i++) { | ||
| const tile = floorTiles[i]; | ||
| for (let j = 0; j < neighborOffsets.length; j++) { | ||
| const [dx, dy] = neighborOffsets[j]; | ||
| const nx = tile.x + dx; | ||
| const ny = tile.y + dy; | ||
| const key = `${nx},${ny}`; | ||
| if (floorKeys.has(key) || wallKeys.has(key)) continue; | ||
| const px = nx + 0.5; | ||
| const py = ny + 0.5; | ||
| if (kernel.distanceMove(px, py) <= 0.25) { | ||
| state.wallTiles.push({ x: px, y: py }); | ||
| wallKeys.add(key); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[nitpick] The wall tile detection algorithm could have performance issues for large dungeons. For each floor tile, it checks 8 neighbors, and for each neighbor it calls kernel.distanceMove(). This results in O(n × 8) distance queries where n is the number of floor tiles.
For large dungeons, consider:
- Caching distance query results
- Using the existing geometry primitives more directly
- Computing wall tiles only once and invalidating on geometry changes
The nested loops at lines 440-454 could be optimized by batching distance queries or using spatial indexing.
Summary
Testing
Codex Task