mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 13:20:43 +00:00
perf(AttackExecution): migrate hot loops to forEachNeighbor (#3820)
Resolves [#3815 ](https://github.com/openfrontio/OpenFrontIO/issues/3815) ## Description: Migrates the five `mg.neighbors(tile)` call sites in `src/core/execution/AttackExecution.ts` to the existing zero-allocation `forEachNeighbor(tile, cb)` helper (`src/core/game/GameImpl.ts:1107`). `neighbors()` allocates a fresh `TileRef[]` of length up to 4 on every call; `forEachNeighbor` invokes a callback with no allocation. The helper is already used in `PlayerExecution`, `GameImpl.updateBorders`, and similar hot paths — this PR finishes that rollout in the hottest attack loop. Affected sites in `src/core/execution/AttackExecution.ts`: | Original line | Context | | --- | --- | | 277 | `tick()` border check around `tileToConquer` | | 326 | `addNeighbors()` outer neighbor loop | | 335 | `addNeighbors()` nested `numOwnedByMe` count | | 370 | `handleDeadDefender()` border test | | 375 | `handleDeadDefender()` neighbor scan | Notes: - `handleDeadDefender` previously narrowed `this.target` from `Player | TerraNullius` to `Player` via the early-return check. Inside an arrow callback that narrowing isn't preserved by TypeScript, so the function now captures `target: Player` once after the check. Same pattern as `tick()`'s `targetPlayer` cache. - For loops that previously used `break` or `.some()`, I used a small flag variable to short-circuit the callback body. With at most 4 neighbors per tile the extra callback invocations are negligible compared to the eliminated array allocation. Behavioral guarantees: - Iteration order is identical (`forEachNeighbor` enumerates the same tiles in the same order as `neighbors()`). - No RNG, no float math, no wire-format changes. - Determinism preserved. Verification: - `npm test` — all 994 tests pass across 104 files. The 22 attack-related tests in `Attack.test.ts`, `AttackStats.test.ts`, and `AiAttackBehavior.test.ts` exercise the migrated code paths end-to-end. - `npx tsc --noEmit` — clean for the changed file. - `npx prettier --check` — clean. Briefly flagged in `#development` per the contribution guidelines before opening. ## Please complete the following: - [ ] I have added screenshots for all UI updates - [ ] I process any text displayed to the user through translateText() and I've added it to the en.json file - [ ] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: sxndrexe
This commit is contained in:
@@ -274,12 +274,11 @@ export class AttackExecution implements Execution {
|
||||
this.attack.removeBorderTile(tileToConquer);
|
||||
|
||||
let onBorder = false;
|
||||
for (const n of this.mg.neighbors(tileToConquer)) {
|
||||
if (this.mg.owner(n) === this._owner) {
|
||||
this.mg.forEachNeighbor(tileToConquer, (n) => {
|
||||
if (!onBorder && this.mg.owner(n) === this._owner) {
|
||||
onBorder = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
});
|
||||
if (this.mg.owner(tileToConquer) !== this.target || !onBorder) {
|
||||
continue;
|
||||
}
|
||||
@@ -323,20 +322,20 @@ export class AttackExecution implements Execution {
|
||||
|
||||
const tickNow = this.mg.ticks(); // cache tick
|
||||
|
||||
for (const neighbor of this.mg.neighbors(tile)) {
|
||||
this.mg.forEachNeighbor(tile, (neighbor) => {
|
||||
if (
|
||||
this.mg.isWater(neighbor) ||
|
||||
this.mg.owner(neighbor) !== this.target
|
||||
) {
|
||||
continue;
|
||||
return;
|
||||
}
|
||||
this.attack.addBorderTile(neighbor);
|
||||
this.attack!.addBorderTile(neighbor);
|
||||
let numOwnedByMe = 0;
|
||||
for (const n of this.mg.neighbors(neighbor)) {
|
||||
this.mg.forEachNeighbor(neighbor, (n) => {
|
||||
if (this.mg.owner(n) === this._owner) {
|
||||
numOwnedByMe++;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
let mag: number;
|
||||
switch (this.mg.terrainType(neighbor)) {
|
||||
@@ -359,33 +358,35 @@ export class AttackExecution implements Execution {
|
||||
tickNow;
|
||||
|
||||
this.toConquer.enqueue(neighbor, priority);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
private handleDeadDefender() {
|
||||
if (!(this.target.isPlayer() && this.target.numTilesOwned() < 100)) return;
|
||||
const target: Player = this.target;
|
||||
|
||||
this.mg.conquerPlayer(this._owner, this.target);
|
||||
this.mg.conquerPlayer(this._owner, target);
|
||||
|
||||
for (let i = 0; i < 10; i++) {
|
||||
for (const tile of this.target.tiles()) {
|
||||
const borders = this.mg
|
||||
.neighbors(tile)
|
||||
.some((t) => this.mg.owner(t) === this._owner);
|
||||
for (const tile of target.tiles()) {
|
||||
let borders = false;
|
||||
this.mg.forEachNeighbor(tile, (t) => {
|
||||
if (!borders && this.mg.owner(t) === this._owner) {
|
||||
borders = true;
|
||||
}
|
||||
});
|
||||
if (borders) {
|
||||
this._owner.conquer(tile);
|
||||
} else {
|
||||
for (const neighbor of this.mg.neighbors(tile)) {
|
||||
let captured = false;
|
||||
this.mg.forEachNeighbor(tile, (neighbor) => {
|
||||
if (captured) return;
|
||||
const no = this.mg.owner(neighbor);
|
||||
if (
|
||||
no.isPlayer() &&
|
||||
no !== this.target &&
|
||||
!no.isFriendly(this.target)
|
||||
) {
|
||||
if (no.isPlayer() && no !== target && !no.isFriendly(target)) {
|
||||
this.mg.player(no.id()).conquer(tile);
|
||||
break;
|
||||
captured = true;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user