Conversation
There was a problem hiding this comment.
Pull request overview
This PR centralizes “safe tile” selection for pet spawning/teleporting by introducing a shared helper that finds a nearby walkable, unoccupied tile (or returns null), then updates pet placement logic to use it and adds unit tests around the new behavior.
Changes:
- Added
findNearestValidTileAroundhelper insrc/rules/utils/queries.jsto locate the nearest valid tile around a source location. - Updated kitty spawn logic in
src/main.jsand pet teleport logic insrc/rules/systems/petFollowSystem.jsto use the helper and avoid forced invalid placement. - Added
tests/petPlacement.test.mjsto cover wall-adjacent spawning and teleport fallback when surrounded.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/petPlacement.test.mjs |
Adds coverage for pet spawn selection near walls and teleport no-op when adjacent tiles are blocked. |
src/rules/utils/queries.js |
Introduces findNearestValidTileAround for reusable nearby-tile validation. |
src/rules/systems/petFollowSystem.js |
Uses the helper to only teleport pets when a valid adjacent tile exists. |
src/main.js |
Uses the helper to place the initial pet spawn on a safe adjacent tile (or fallback). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const x = source.x + dx; | ||
| const y = source.y + dy; | ||
| const dist = Math.abs(dx) + Math.abs(dy); |
There was a problem hiding this comment.
maxDistance is treated as a coordinate offset (square area), but the code computes Manhattan dist and will still consider/return tiles where |dx| + |dy| > maxDistance (e.g. dx=2,dy=2 when maxDistance=2). If maxDistance is intended to be a Manhattan radius, filter candidates to dist <= maxDistance (or iterate by distance rings) so the function never returns a tile beyond the requested distance.
| const x = source.x + dx; | |
| const y = source.y + dy; | |
| const dist = Math.abs(dx) + Math.abs(dy); | |
| const dist = Math.abs(dx) + Math.abs(dy); | |
| if (dist > maxDistance) continue; | |
| const x = source.x + dx; | |
| const y = source.y + dy; |
| for (let dy = -maxDistance; dy <= maxDistance; dy++) { | ||
| for (let dx = -maxDistance; dx <= maxDistance; dx++) { | ||
| const x = source.x + dx; | ||
| const y = source.y + dy; | ||
| const dist = Math.abs(dx) + Math.abs(dy); | ||
| candidates.push({ x, y, dist, axisBias: (dx === 0 || dy === 0) ? 0 : 1 }); | ||
| } |
There was a problem hiding this comment.
The candidate generation includes the source tile (dx=0, dy=0), so callers that forget to pass exclude: [source] can get back the same tile even though the docstring says "around a source point". Consider skipping dx=0/dy=0 by default (or clarifying in the JSDoc that the source tile is included unless excluded).
| Deno.test('findNearestValidTileAround finds valid spawn near walls', () => { | ||
| clearAll(); | ||
| const tiles = fillChunk(TILE_WALL); | ||
| tiles[1 * CHUNK_SIZE + 1] = TILE_FLOOR; // source tile | ||
| tiles[2 * CHUNK_SIZE + 1] = TILE_FLOOR; // only valid adjacent tile (1,2) | ||
| loadChunk(0, 0, tiles); | ||
|
|
||
| const world = new World({ seed: 1 }); | ||
| const tile = findNearestValidTileAround(world, { x: 1, y: 1 }, { | ||
| maxDistance: 1, | ||
| exclude: [{ x: 1, y: 1 }], | ||
| }); | ||
|
|
||
| assert(tile, 'expected a valid adjacent tile'); | ||
| assertEquals(tile, { x: 1, y: 2 }); | ||
| clearAll(); | ||
| }); | ||
|
|
||
| Deno.test('pet teleport keeps current position when nearby tiles are blocked', () => { | ||
| clearAll(); |
There was a problem hiding this comment.
Repository tests consistently use double quotes for Deno.test("...") names; this file uses single quotes. For consistency with the rest of the test suite, switch these test names to double quotes.
…and-tests Add findNearestValidTileAround helper and use it for pet spawn/teleport; add tests
Motivation
nullwhen no valid adjacent tile exists.Description
findNearestValidTileAroundinsrc/rules/utils/queries.jswhich searches nearby tiles (configurablemaxDistance) and validates candidates usingisWalkable, solidCollidercomponents, living occupants viaVitality.hp, and an optionalexcludelist, returning the nearest valid tile ornull.src/main.jsfor the kitty spawn to choose a safe adjacent tile (excluding the player tile) and fall back to the player position when no valid adjacent tile is found.src/rules/systems/petFollowSystem.jsteleport branch to only teleport pets when a valid nearby tile exists, otherwise keep the pet at its current position.tests/petPlacement.test.mjscovering spawn selection near walls and teleport fallback when surrounding tiles are blocked.Testing
tests/petPlacement.test.mjswhich includes tests for spawn selection near walls and teleport fallback, but the tests were not executed in this environment.deno fmt/ test runner) but the environment does not havedenoinstalled, so formatting/testing could not be performed here.Codex Task