Skip to content

Commit 8f39eed

Browse files
agjsclaude
andcommitted
fix: address PR #6 review — action pinning, wall render, AABB collision
Scorecard Pinned-Dependencies (github-advanced-security bot): - Every GitHub Action pinned to commit SHA with the corresponding version tag in a trailing comment. Covers all six workflow files. This also bumps the actions to their current majors (checkout v6, setup-node v6, upload-artifact v7, pnpm/action-setup v6, codeql v4), obsoleting the open Dependabot PRs. Gemini high-priority: WallLayerEntity O(N) scene-graph nodes - Replaced per-tile Phaser.GameObjects.Rectangle with a single Phaser.Graphics fillRect batch. Turns 30x17-tile grids from hundreds of scene nodes into one. Gemini high-priority: WallCollisionFeature point-based hit test - Added PLAYER_SIZE_PX / PLAYER_HALF_EXTENT_PX constants. - resolveMove now checks all four corners of the player's AABB, not just the center, eliminating the ghosting that let players sink half their size into a wall. - Added a test that a center-point check would pass but the AABB check correctly rejects. - Feature deps accept an optional playerHalfExtent override for tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 23a04a5 commit 8f39eed

11 files changed

Lines changed: 100 additions & 55 deletions

File tree

.github/workflows/arch-invariants.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ jobs:
1010
name: grep-based banned patterns
1111
runs-on: ubuntu-latest
1212
steps:
13-
- uses: actions/checkout@v5
13+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.0
1414
- name: no phaser imports in domain/
1515
run: |
1616
if grep -rnE "from ['\"]phaser" src/domain 2>/dev/null; then

.github/workflows/ci.yml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ jobs:
1010
name: pnpm check
1111
runs-on: ubuntu-latest
1212
steps:
13-
- uses: actions/checkout@v5
14-
- uses: pnpm/action-setup@v4
13+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.0
14+
- uses: pnpm/action-setup@903f9c1a6ebcba6cf41d87230be49611ac97822e # v6.0.0
1515
with:
1616
version: 10.30.3
17-
- uses: actions/setup-node@v5
17+
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.0.0
1818
with:
1919
node-version: 22.22.0
2020
cache: pnpm
@@ -29,11 +29,11 @@ jobs:
2929
name: playwright smoke
3030
runs-on: ubuntu-latest
3131
steps:
32-
- uses: actions/checkout@v5
33-
- uses: pnpm/action-setup@v4
32+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.0
33+
- uses: pnpm/action-setup@903f9c1a6ebcba6cf41d87230be49611ac97822e # v6.0.0
3434
with:
3535
version: 10.30.3
36-
- uses: actions/setup-node@v5
36+
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.0.0
3737
with:
3838
node-version: 22.22.0
3939
cache: pnpm

.github/workflows/codeql.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ jobs:
2121
matrix:
2222
language: [javascript-typescript]
2323
steps:
24-
- uses: actions/checkout@v5
25-
- uses: github/codeql-action/init@v3
24+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.0
25+
- uses: github/codeql-action/init@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.0.0
2626
with:
2727
languages: ${{ matrix.language }}
2828
queries: security-and-quality
29-
- uses: github/codeql-action/analyze@v3
29+
- uses: github/codeql-action/analyze@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.0.0
3030
with:
3131
category: '/language:${{ matrix.language }}'

.github/workflows/deploy-pages.yml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,22 @@ jobs:
1818
build:
1919
runs-on: ubuntu-latest
2020
steps:
21-
- uses: actions/checkout@v5
22-
- uses: pnpm/action-setup@v4
21+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.0
22+
- uses: pnpm/action-setup@903f9c1a6ebcba6cf41d87230be49611ac97822e # v6.0.0
2323
with:
2424
version: 10.30.3
25-
- uses: actions/setup-node@v5
25+
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.0.0
2626
with:
2727
node-version: 22.22.0
2828
cache: pnpm
29-
- uses: actions/configure-pages@v5
29+
- uses: actions/configure-pages@983d7736d9b0ae728b81ab479565c72886d7745b # v5.0.0
3030
id: pages
3131
- run: pnpm install --frozen-lockfile
3232
- name: build with public base path
3333
env:
3434
VITE_BASE_PATH: ${{ steps.pages.outputs.base_path }}
3535
run: pnpm build
36-
- uses: actions/upload-pages-artifact@v4
36+
- uses: actions/upload-pages-artifact@7b1f4a764d45c48632c6b24a0339c27f5614fb0b # v4.0.0
3737
with:
3838
path: ./dist
3939

@@ -44,5 +44,5 @@ jobs:
4444
name: github-pages
4545
url: ${{ steps.deployment.outputs.page_url }}
4646
steps:
47-
- uses: actions/deploy-pages@v4
47+
- uses: actions/deploy-pages@d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e # v4.0.0
4848
id: deployment

.github/workflows/release-please.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
release-please:
1414
runs-on: ubuntu-latest
1515
steps:
16-
- uses: googleapis/release-please-action@v4
16+
- uses: googleapis/release-please-action@5c625bfb5d1ff62eadeeb3772007f7f66fdcf071 # v4.4.0
1717
with:
1818
config-file: .github/release-please-config.json
1919
manifest-file: .github/.release-please-manifest.json

.github/workflows/security.yml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ jobs:
1212
name: pnpm audit (production)
1313
runs-on: ubuntu-latest
1414
steps:
15-
- uses: actions/checkout@v5
16-
- uses: pnpm/action-setup@v4
15+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.0
16+
- uses: pnpm/action-setup@903f9c1a6ebcba6cf41d87230be49611ac97822e # v6.0.0
1717
with:
1818
version: 10.30.3
19-
- uses: actions/setup-node@v5
19+
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.0.0
2020
with:
2121
node-version: 22.22.0
2222
cache: pnpm
@@ -35,19 +35,19 @@ jobs:
3535
contents: read
3636
actions: read
3737
steps:
38-
- uses: actions/checkout@v5
38+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.0
3939
with:
4040
persist-credentials: false
41-
- uses: ossf/scorecard-action@v2.4.3
41+
- uses: ossf/scorecard-action@4eaacf0543bb3f2c246792bd56e8cdeffafb205a # v2.4.3
4242
with:
4343
results_file: scorecard-results.sarif
4444
results_format: sarif
4545
publish_results: true
46-
- uses: actions/upload-artifact@v5
46+
- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.0
4747
with:
4848
name: scorecard-results
4949
path: scorecard-results.sarif
5050
retention-days: 5
51-
- uses: github/codeql-action/upload-sarif@v3
51+
- uses: github/codeql-action/upload-sarif@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.0.0
5252
with:
5353
sarif_file: scorecard-results.sarif
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
export const PLAYER_SPEED_PX_PER_SEC = 220;
22
export const PLAYER_START_POSITION = { x: 100, y: 100 } as const;
3+
export const PLAYER_SIZE_PX = 24;
4+
export const PLAYER_HALF_EXTENT_PX = PLAYER_SIZE_PX / 2;

src/domain/player/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,9 @@ export type { PlayerState } from './Player.types.js';
22
export { createPlayer } from './Player.model.js';
33
export { applyMoveIntent } from './Player.behavior.js';
44
export type { MoveIntent } from './Player.behavior.js';
5-
export { PLAYER_SPEED_PX_PER_SEC, PLAYER_START_POSITION } from './Player.constants.js';
5+
export {
6+
PLAYER_SPEED_PX_PER_SEC,
7+
PLAYER_START_POSITION,
8+
PLAYER_SIZE_PX,
9+
PLAYER_HALF_EXTENT_PX,
10+
} from './Player.constants.js';

src/features/wall-collision/WallCollisionFeature.test.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,28 +11,28 @@ const lookup = (types: readonly TileTypeDescriptor[]): TileTypeLookup => {
1111
};
1212

1313
const tileTypes = lookup([
14-
{ id: 'empty', solid: false, destructible: false, displayColor: '#000' },
14+
{ id: 'empty', solid: false, destructible: false, displayColor: '#000000' },
1515
{ id: 'brick', solid: true, destructible: true, displayColor: '#b35a1b' },
1616
]);
1717

1818
describe('WallCollisionFeature.resolveMove', () => {
1919
it('passes through a move that does not collide', () => {
2020
const grid = createGrid({ cols: 4, rows: 4, tileSize: 32 });
21-
const feature = createWallCollisionFeature({ grid, tileTypes });
21+
const feature = createWallCollisionFeature({ grid, tileTypes, playerHalfExtent: 12 });
2222
const prev = { ...createPlayer(), position: { x: 16, y: 16 } };
2323
const tent = { ...prev, position: { x: 48, y: 16 } };
2424
expect(feature.resolveMove(prev, tent)).toEqual(tent);
2525
});
2626

2727
it('fully reverts when neither X-only nor Y-only can slide past walls', () => {
28-
// Block the target tile and both adjacent single-axis slide destinations.
28+
// Block the target tile and both single-axis slide destinations.
2929
let grid = createGrid({ cols: 4, rows: 4, tileSize: 32 });
3030
grid = setTile(grid, { col: 2, row: 2 }, 'brick');
3131
grid = setTile(grid, { col: 2, row: 1 }, 'brick'); // blocks X-only slide
3232
grid = setTile(grid, { col: 1, row: 2 }, 'brick'); // blocks Y-only slide
3333

34-
const feature = createWallCollisionFeature({ grid, tileTypes });
35-
const prev = { ...createPlayer(), position: { x: 60, y: 60 } };
34+
const feature = createWallCollisionFeature({ grid, tileTypes, playerHalfExtent: 12 });
35+
const prev = { ...createPlayer(), position: { x: 50, y: 50 } };
3636
const tent = { ...prev, position: { x: 80, y: 80 }, velocity: { x: 100, y: 100 } };
3737
const resolved = feature.resolveMove(prev, tent);
3838
expect(resolved.position).toEqual(prev.position);
@@ -46,13 +46,30 @@ describe('WallCollisionFeature.resolveMove', () => {
4646
{ col: 1, row: 2 },
4747
'brick',
4848
);
49-
const feature = createWallCollisionFeature({ grid, tileTypes });
50-
const prev = { ...createPlayer(), position: { x: 36, y: 48 } }; // in (col 1, row 1)
49+
const feature = createWallCollisionFeature({ grid, tileTypes, playerHalfExtent: 12 });
50+
const prev = { ...createPlayer(), position: { x: 36, y: 48 } };
5151
const tent = { ...prev, position: { x: 48, y: 68 }, velocity: { x: 100, y: 100 } };
5252
const resolved = feature.resolveMove(prev, tent);
53-
// X advanced, Y reverted
5453
expect(resolved.position.x).toBe(48);
5554
expect(resolved.position.y).toBe(48);
5655
expect(resolved.velocity).toEqual({ x: 100, y: 0 });
5756
});
57+
58+
it('AABB catches a corner overlap that a center-point check would miss', () => {
59+
// Wall at (col 2, row 1). world bounds x[64,96), y[32,64).
60+
// Tent position (60, 50) has its center in (col 1, row 1) (empty),
61+
// but its top-right corner (71, 38) is inside the wall.
62+
const grid = setTile(
63+
createGrid({ cols: 4, rows: 4, tileSize: 32 }),
64+
{ col: 2, row: 1 },
65+
'brick',
66+
);
67+
const feature = createWallCollisionFeature({ grid, tileTypes, playerHalfExtent: 12 });
68+
const prev = { ...createPlayer(), position: { x: 40, y: 50 } };
69+
const tent = { ...prev, position: { x: 60, y: 50 }, velocity: { x: 100, y: 0 } };
70+
const resolved = feature.resolveMove(prev, tent);
71+
// X motion blocked because the top-right corner would enter the wall.
72+
// Y motion was zero anyway; the move is fully rejected.
73+
expect(resolved.position).toEqual(prev.position);
74+
});
5875
});

src/features/wall-collision/WallCollisionFeature.ts

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,68 @@
11
import { worldToGrid, type GridState } from '@domain/grid';
2-
import type { PlayerState } from '@domain/player';
2+
import { PLAYER_HALF_EXTENT_PX, type PlayerState } from '@domain/player';
33
import { isBlocked, type TileTypeLookup } from '@domain/wall';
44

55
export interface IWallCollisionFeatureDeps {
66
readonly grid: GridState;
77
readonly tileTypes: TileTypeLookup;
8+
/**
9+
* Half the player's AABB extent on each axis, in world pixels.
10+
* Collision is checked against the four corners of the player's bounding box
11+
* rather than its center — a point-based check would let the player sink
12+
* up to half its size into a wall before registering a hit.
13+
*/
14+
readonly playerHalfExtent?: number;
815
}
916

1017
export interface IWallCollisionFeature {
1118
/**
12-
* Resolves a tentative move. If the tentative position would place the
13-
* player inside a solid tile, the position is reverted axis-by-axis so
14-
* the player can slide along a wall rather than sticking to it.
19+
* Resolves a tentative move. If the tentative AABB intersects a solid tile,
20+
* the position is reverted axis-by-axis so the player slides along a wall
21+
* rather than sticking.
1522
*/
1623
resolveMove: (previous: PlayerState, tentative: PlayerState) => PlayerState;
1724
}
1825

1926
export const createWallCollisionFeature = (
2027
deps: IWallCollisionFeatureDeps,
2128
): IWallCollisionFeature => {
22-
const hitsWall = (p: PlayerState): boolean =>
23-
isBlocked(deps.grid, worldToGrid(deps.grid, p.position), deps.tileTypes);
29+
const halfExtent = deps.playerHalfExtent ?? PLAYER_HALF_EXTENT_PX;
30+
31+
const hitsWall = (p: PlayerState): boolean => {
32+
const { x, y } = p.position;
33+
const corners: ReadonlyArray<readonly [number, number]> = [
34+
[x - halfExtent, y - halfExtent],
35+
[x + halfExtent - 1, y - halfExtent],
36+
[x - halfExtent, y + halfExtent - 1],
37+
[x + halfExtent - 1, y + halfExtent - 1],
38+
];
39+
for (const [cx, cy] of corners) {
40+
if (isBlocked(deps.grid, worldToGrid(deps.grid, { x: cx, y: cy }), deps.tileTypes)) {
41+
return true;
42+
}
43+
}
44+
return false;
45+
};
2446

2547
return {
2648
resolveMove(previous, tentative) {
2749
if (!hitsWall(tentative)) {
2850
return tentative;
2951
}
30-
// Try X-only, then Y-only, so movement slides along walls.
3152
const xOnly: PlayerState = {
3253
...tentative,
3354
position: { x: tentative.position.x, y: previous.position.y },
3455
};
35-
3656
if (!hitsWall(xOnly)) {
3757
return { ...xOnly, velocity: { x: tentative.velocity.x, y: 0 } };
3858
}
39-
4059
const yOnly: PlayerState = {
4160
...tentative,
4261
position: { x: previous.position.x, y: tentative.position.y },
4362
};
44-
4563
if (!hitsWall(yOnly)) {
4664
return { ...yOnly, velocity: { x: 0, y: tentative.velocity.y } };
4765
}
48-
4966
return { ...previous, velocity: { x: 0, y: 0 } };
5067
},
5168
};

0 commit comments

Comments
 (0)