mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-07-06 04:05:12 +00:00
refactor: standardize cardinal-neighbor iteration on neighbors() N,S,W,E order (#4495)
## Summary Follow-up to #4494. That PR added `forEachNeighborNSWE` as a third neighbor iterator because the existing allocation-free helpers (`forEachNeighbor`, `neighbors4`) visit in W,E,N,S order while `neighbors()` visits N,S,W,E — and substituting one for the other changes simulation behavior at order-sensitive call sites. This PR removes that duplication by standardizing on **one order everywhere**: `forEachNeighbor` and `neighbors4` now visit in the same N,S,W,E order as `neighbors()`, and `forEachNeighborNSWE` is deleted. ## ⚠️ Intentional behavior change Callers of the flipped helpers that are order-sensitive now make different (equally valid) decisions: - `AttackExecution.addNeighbors` — PRNG values are drawn per neighbor while building the conquest frontier, so attack expansion patterns differ - `AttackExecution.handleDeadDefender` — a dead defender's tiles go to the *first-visited* adjacent player - `WarshipExecution.bestNeighborToward` — distance ties break by visit order - `PlayerExecution` surrounded-cluster flood fill — set insertion order propagates to conquer order Game outcomes for a given seed differ from previous builds (verified: the 12k-tick reference run ends with 31 players alive vs 24 before). Determinism across clients *within* a build is unaffected — all clients run the same code, so there is no desync risk. Replays/verification pinned to old hashes will not match this build. New reference hashes for the headless perf harness (seed `perf-default`): | Run | Final hash | |---|---| | giantworldmap, 12,000 ticks | `57830793797434300` | | giantworldmap, 2,000 ticks | `55125379638382860` | | world, 1,800 ticks | `32337437717390864` | ## Verification - [x] Full suite green (1,901 tests), including new exact-order contract tests: `forEachNeighbor` and `neighbors4` must match `neighbors()` contents **and order** for every tile - [x] 20-game-minute Giant World Map benchmark: no perf regression (73 ticks/sec, GC 1.2% of wall, allocation profile unchanged) - [x] Order-sensitivity audit of every `forEachNeighbor`/`neighbors4` call site (sensitive ones listed above; the rest are booleans, counts, or min/max accumulations) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1172,12 +1172,6 @@ export class GameView implements GameMap {
|
||||
forEachNeighbor(ref: TileRef, callback: (neighbor: TileRef) => void): void {
|
||||
this._map.forEachNeighbor(ref, callback);
|
||||
}
|
||||
forEachNeighborNSWE(
|
||||
ref: TileRef,
|
||||
callback: (neighbor: TileRef) => void,
|
||||
): void {
|
||||
this._map.forEachNeighborNSWE(ref, callback);
|
||||
}
|
||||
neighbors4(ref: TileRef, out: TileRef[]): number {
|
||||
return this._map.neighbors4(ref, out);
|
||||
}
|
||||
|
||||
@@ -70,7 +70,7 @@ export class AiAttackBehavior {
|
||||
}
|
||||
};
|
||||
for (const t of this.player.borderTiles()) {
|
||||
this.game.forEachNeighborNSWE(t, visit);
|
||||
this.game.forEachNeighbor(t, visit);
|
||||
}
|
||||
const playerNeighbors = this.player.nearby();
|
||||
for (const n of playerNeighbors) {
|
||||
|
||||
@@ -703,15 +703,12 @@ export interface Game extends GameMap {
|
||||
map(): GameMap;
|
||||
miniMap(): GameMap;
|
||||
forEachTile(fn: (tile: TileRef) => void): void;
|
||||
// Zero-allocation neighbor iteration (cardinal only) to avoid creating arrays
|
||||
// Zero-allocation neighbor iteration (cardinal only), in the same N, S, W, E
|
||||
// order as neighbors().
|
||||
forEachNeighbor(tile: TileRef, callback: (neighbor: TileRef) => void): void;
|
||||
// Same, but in neighbors() N, S, W, E order — for order-sensitive code.
|
||||
forEachNeighborNSWE(
|
||||
tile: TileRef,
|
||||
callback: (neighbor: TileRef) => void,
|
||||
): void;
|
||||
// Writes the cardinal neighbors of ref into out (W, E, N, S order) and
|
||||
// returns the count. Reuse out across calls to avoid allocation.
|
||||
// Writes the cardinal neighbors of ref into out (same N, S, W, E order as
|
||||
// neighbors()) and returns the count. Reuse out across calls to avoid
|
||||
// allocation.
|
||||
neighbors4(ref: TileRef, out: TileRef[]): number;
|
||||
// Zero-allocation neighbor iteration for performance-critical cluster calculation
|
||||
// Alternative to neighborsWithDiag() that returns arrays
|
||||
|
||||
@@ -1134,12 +1134,6 @@ export class GameImpl implements Game {
|
||||
forEachNeighbor(tile: TileRef, callback: (neighbor: TileRef) => void): void {
|
||||
this._map.forEachNeighbor(tile, callback);
|
||||
}
|
||||
forEachNeighborNSWE(
|
||||
tile: TileRef,
|
||||
callback: (neighbor: TileRef) => void,
|
||||
): void {
|
||||
this._map.forEachNeighborNSWE(tile, callback);
|
||||
}
|
||||
neighbors4(ref: TileRef, out: TileRef[]): number {
|
||||
return this._map.neighbors4(ref, out);
|
||||
}
|
||||
|
||||
@@ -37,19 +37,13 @@ export interface GameMap {
|
||||
isOnEdgeOfMap(ref: TileRef): boolean;
|
||||
isBorder(ref: TileRef): boolean;
|
||||
neighbors(ref: TileRef): TileRef[];
|
||||
// Zero-allocation neighbor iteration (cardinal only), in W, E, N, S order.
|
||||
// Zero-allocation neighbor iteration (cardinal only), in the same N, S, W, E
|
||||
// order as neighbors(). All cardinal-neighbor helpers share this order so
|
||||
// they are interchangeable even in order-sensitive simulation code.
|
||||
forEachNeighbor(ref: TileRef, callback: (neighbor: TileRef) => void): void;
|
||||
// Zero-allocation neighbor iteration (cardinal only) in the same N, S, W, E
|
||||
// order as neighbors(). Use this in order-sensitive code — anything feeding
|
||||
// sets/arrays whose iteration order affects the simulation — where
|
||||
// forEachNeighbor's W, E, N, S order would change behavior.
|
||||
forEachNeighborNSWE(
|
||||
ref: TileRef,
|
||||
callback: (neighbor: TileRef) => void,
|
||||
): void;
|
||||
// Writes the cardinal neighbors of ref into out (W, E, N, S order) and
|
||||
// returns the count. out must have length >= 4; reuse it across calls to
|
||||
// avoid allocation in hot loops.
|
||||
// Writes the cardinal neighbors of ref into out (same N, S, W, E order as
|
||||
// neighbors()) and returns the count. out must have length >= 4; reuse it
|
||||
// across calls to avoid allocation in hot loops.
|
||||
neighbors4(ref: TileRef, out: TileRef[]): number;
|
||||
// Zero-allocation neighbor iteration including diagonals, in dx-major
|
||||
// order: (-1,-1),(-1,0),(-1,1),(0,-1),(0,1),(1,-1),(1,0),(1,1).
|
||||
@@ -397,19 +391,6 @@ export class GameMapImpl implements GameMap {
|
||||
const w = this.width_;
|
||||
const x = this.refToX[ref];
|
||||
|
||||
if (x !== 0) callback(ref - 1);
|
||||
if (x !== w - 1) callback(ref + 1);
|
||||
if (ref >= w) callback(ref - w);
|
||||
if (ref < (this.height_ - 1) * w) callback(ref + w);
|
||||
}
|
||||
|
||||
forEachNeighborNSWE(
|
||||
ref: TileRef,
|
||||
callback: (neighbor: TileRef) => void,
|
||||
): void {
|
||||
const w = this.width_;
|
||||
const x = this.refToX[ref];
|
||||
|
||||
if (ref >= w) callback(ref - w);
|
||||
if (ref < (this.height_ - 1) * w) callback(ref + w);
|
||||
if (x !== 0) callback(ref - 1);
|
||||
@@ -421,10 +402,10 @@ export class GameMapImpl implements GameMap {
|
||||
const x = this.refToX[ref];
|
||||
let n = 0;
|
||||
|
||||
if (x !== 0) out[n++] = ref - 1;
|
||||
if (x !== w - 1) out[n++] = ref + 1;
|
||||
if (ref >= w) out[n++] = ref - w;
|
||||
if (ref < (this.height_ - 1) * w) out[n++] = ref + w;
|
||||
if (x !== 0) out[n++] = ref - 1;
|
||||
if (x !== w - 1) out[n++] = ref + 1;
|
||||
return n;
|
||||
}
|
||||
|
||||
|
||||
@@ -493,7 +493,7 @@ export class PlayerImpl implements Player {
|
||||
}
|
||||
};
|
||||
for (const border of this.borderTiles()) {
|
||||
map.forEachNeighborNSWE(border, visit);
|
||||
map.forEachNeighbor(border, visit);
|
||||
}
|
||||
for (const n of this.shoreReachableNeighbors()) {
|
||||
ns.add(n);
|
||||
|
||||
Reference in New Issue
Block a user