mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 09:50:43 +00:00
Perf/Fix: spawn and other functions that need closest by unit (#3243)
## Description: Performance improvements. - **PlayerImpl**: for _nukeSpawn_, cache config to const. - **Other files**: for nukeSpawn and other functions doing the same, introduce findClosestBy function. - for **TradeShipExecution**, with the move from _distSortUnit_ to _findClosestBy_, also add check if port isActive, !_isMarkedForDeletion_ and !_isUnderConstruction_. These checks should have been there already, so now do it in one go to make use of the predicate isCandidate in findClosestBy. - for **TradeShipExectution.test.ts**, add mock functions for _isMarkedForDeletion_ and _isUnderConstruction_ because of the above. Also, set Unit tiles and Pathfinding node to actual valid TileRefs for the testing map. This prevents NaN as return value from manhattanDist. This problem was already present with the use of distSortUnit, but that function just did NaN - NaN, returned the first and only port unit in the array and called it a day. For findClosestBy we have to make sure the predicate manhattanDist actually returns a number instead of NaN so we need actually valid tiles. We now have a working test instead of a test that actually silently failed like before. - **PlayerImpl**: _warshipSpawn_ and _nukeSpawn_: Make use of the isCandidate predicate of findClosestBy to have warshipSpawn not return ports under construction or (smaller change) inactive. This fixes a bug i have seen right away (where Warship spawns from under construction Port). Same for _nukeSpawn_ silos, don't return inactive silo just to be sure now that we can easily add it to isCandidate predicate anyway. This costs no performance in the _nukeSpawn_ benchmarks actually. This should as a by-effecft fix an edge case bug i have seen, where a nuke is sent from a phantom silo. Some of this goes along with PR #3220 since playerImpl buildableUnits makes use of the underlying spawn functions via canBuild. Just like ConstructionExecution does. But i didn't want to add more to PR 3220 since there's already a lot in there. The new function _findClosestBy_ could also be applied to some other parts of code to benefit of it being faster, so i did that. _findClosestBy_ uses _findMinimumBy_, which is a little more generic in name. I think _findMinimumBy_ could be used by other parts of code, while _findClosestBy_ is more clear naming for what it does now. But we could ditch _findMinimumBy_ and just leave findClosestBy? Examples of synthetic benchmarks (not included in this PR): **BEFORE CHANGES (before Scamiv's PR #3241)** <img width="705" height="91" alt="image" src="https://github.com/user-attachments/assets/d6d91c08-39f1-4387-9ccc-e51951caa539" /> <img width="751" height="101" alt="image" src="https://github.com/user-attachments/assets/80d400ac-3408-4107-aa58-6d2a847311e9" /> **AFTER CHANGES (before Scamiv's PR #3241)**   **BEFORE CHANGES (after Scamiv's PR #3241)**   **AFTER CHANGES (after Scamiv's PR #3241)** <img width="717" height="96" alt="image" src="https://github.com/user-attachments/assets/5b106843-bf6e-4448-a8e8-94448fb30ced" /> <img width="767" height="92" alt="image" src="https://github.com/user-attachments/assets/e6714c7b-26c1-455b-adae-f0060f1cbc7b" /> _Also see more **BEFORE** and **AFTER** in this comment:_ https://github.com/openfrontio/OpenFrontIO/pull/3243#issuecomment-3949060395 _And here a comparison in the flame charts:_ - based on the same replay and tried to get the performance recording going at the same speed and length but always end up with small differences - because of a bug in replays currently, it puts you in with the same clientID/persistantID currently. This means we can also record part of what is normally only recordable with live human input (the playerActions/playerBuildables). **BEFORE** flame chart with nukeSpawn (human player) and maybeSendNuke (Nation players, uses nukeSpawn via canBuild):    **AFTER** flame chart with nukeSpawn (human player) and maybeSendNuke (Nation players, uses nukeSpawn via canBuild):      ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] 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: tryout33
This commit is contained in:
VariableVince
@@ -10,7 +10,7 @@ import {
|
||||
PlayerRecord,
|
||||
ServerMessage,
|
||||
} from "../core/Schemas";
|
||||
import { createPartialGameRecord, replacer } from "../core/Util";
|
||||
import { createPartialGameRecord, findClosestBy, replacer } from "../core/Util";
|
||||
import { ServerConfig } from "../core/configuration/Config";
|
||||
import { getConfig } from "../core/configuration/ConfigLoader";
|
||||
import { BuildableUnit, Structures, UnitType } from "../core/game/Game";
|
||||
@@ -633,15 +633,15 @@ export class ClientGameRunner {
|
||||
}
|
||||
|
||||
if (upgradeUnits.length > 0) {
|
||||
upgradeUnits.sort((a, b) => a.distance - b.distance);
|
||||
const bestUpgrade = upgradeUnits[0];
|
||||
|
||||
this.eventBus.emit(
|
||||
new SendUpgradeStructureIntentEvent(
|
||||
bestUpgrade.unitId,
|
||||
bestUpgrade.unitType,
|
||||
),
|
||||
);
|
||||
const bestUpgrade = findClosestBy(upgradeUnits, (u) => u.distance);
|
||||
if (bestUpgrade) {
|
||||
this.eventBus.emit(
|
||||
new SendUpgradeStructureIntentEvent(
|
||||
bestUpgrade.unitId,
|
||||
bestUpgrade.unitType,
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
} from "../../../core/game/Game";
|
||||
import { TileRef } from "../../../core/game/GameMap";
|
||||
import { GameView, PlayerView } from "../../../core/game/GameView";
|
||||
import { Emoji, flattenedEmojiTable } from "../../../core/Util";
|
||||
import { Emoji, findClosestBy, flattenedEmojiTable } from "../../../core/Util";
|
||||
import { renderNumber, translateText } from "../../Utils";
|
||||
import { UIState } from "../UIState";
|
||||
import { BuildItemDisplay, BuildMenu, flattenedBuildTable } from "./BuildMenu";
|
||||
@@ -555,14 +555,11 @@ export const deleteUnitElement: MenuElement = {
|
||||
DELETE_SELECTION_RADIUS,
|
||||
);
|
||||
|
||||
if (myUnits.length > 0) {
|
||||
myUnits.sort(
|
||||
(a, b) =>
|
||||
params.game.manhattanDist(a.tile(), params.tile) -
|
||||
params.game.manhattanDist(b.tile(), params.tile),
|
||||
);
|
||||
|
||||
params.playerActionHandler.handleDeleteUnit(myUnits[0].id());
|
||||
const closestUnit = findClosestBy(myUnits, (unit) =>
|
||||
params.game.manhattanDist(unit.tile(), params.tile),
|
||||
);
|
||||
if (closestUnit) {
|
||||
params.playerActionHandler.handleDeleteUnit(closestUnit.id());
|
||||
}
|
||||
|
||||
params.closeMenu();
|
||||
|
||||
@@ -61,6 +61,60 @@ export function distSortUnit(
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Finds minimum, by score, with single pass search
|
||||
* Faster than array.reduce()
|
||||
*/
|
||||
export function findMinimumBy<T>(
|
||||
values: readonly T[],
|
||||
score: (value: T) => number,
|
||||
isCandidate?: (value: T) => boolean,
|
||||
): T | null {
|
||||
let best: T | null = null;
|
||||
let bestScore = Infinity;
|
||||
|
||||
if (isCandidate === undefined) {
|
||||
for (let i = 0, len = values.length; i < len; i++) {
|
||||
const value = values[i];
|
||||
const currentScore = score(value);
|
||||
if (currentScore < bestScore) {
|
||||
bestScore = currentScore;
|
||||
best = value;
|
||||
}
|
||||
}
|
||||
return best;
|
||||
}
|
||||
|
||||
for (let i = 0, len = values.length; i < len; i++) {
|
||||
const value = values[i];
|
||||
if (!isCandidate(value)) continue;
|
||||
|
||||
const currentScore = score(value);
|
||||
if (currentScore < bestScore) {
|
||||
bestScore = currentScore;
|
||||
best = value;
|
||||
}
|
||||
}
|
||||
|
||||
return best;
|
||||
}
|
||||
|
||||
/**
|
||||
* Finds closest by fast. Example usage:
|
||||
* findClosestBy(
|
||||
* this.units(UnitType.MissileSilo),
|
||||
* (silo) => mg.manhattanDist(silo.tile(), tile),
|
||||
* (silo) => !silo.isInCooldown() && !silo.isUnderConstruction(),
|
||||
* )
|
||||
*/
|
||||
export function findClosestBy<T>(
|
||||
values: readonly T[],
|
||||
distance: (value: T) => number,
|
||||
isCandidate?: (value: T) => boolean,
|
||||
): T | null {
|
||||
return findMinimumBy(values, distance, isCandidate);
|
||||
}
|
||||
|
||||
export function simpleHash(str: string): number {
|
||||
let hash = 0;
|
||||
for (let i = 0; i < str.length; i++) {
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
import { TileRef } from "../game/GameMap";
|
||||
import { PathFinding } from "../pathfinding/PathFinder";
|
||||
import { PathStatus, SteppingPathFinder } from "../pathfinding/types";
|
||||
import { distSortUnit } from "../Util";
|
||||
import { findClosestBy } from "../Util";
|
||||
|
||||
export class TradeShipExecution implements Execution {
|
||||
private active = true;
|
||||
@@ -80,27 +80,32 @@ export class TradeShipExecution implements Execution {
|
||||
return;
|
||||
}
|
||||
|
||||
const curTile = this.tradeShip.tile();
|
||||
|
||||
if (
|
||||
this.wasCaptured &&
|
||||
(tradeShipOwner !== dstPortOwner || !this._dstPort.isActive())
|
||||
) {
|
||||
const ports = this.tradeShip
|
||||
.owner()
|
||||
.units(UnitType.Port)
|
||||
.sort(distSortUnit(this.mg, this.tradeShip));
|
||||
if (ports.length === 0) {
|
||||
const nearestPort = findClosestBy(
|
||||
tradeShipOwner.units(UnitType.Port),
|
||||
(port) => this.mg.manhattanDist(port.tile(), curTile),
|
||||
(port) =>
|
||||
port.isActive() &&
|
||||
!port.isMarkedForDeletion() &&
|
||||
!port.isUnderConstruction(),
|
||||
);
|
||||
if (nearestPort === null) {
|
||||
this.tradeShip.delete(false);
|
||||
this.active = false;
|
||||
return;
|
||||
} else {
|
||||
this._dstPort = ports[0];
|
||||
this._dstPort = nearestPort;
|
||||
this.tradeShip.setTargetUnit(this._dstPort);
|
||||
// Plan-driven units don't emit per-tick unit updates, so force a sync for the new target.
|
||||
this.tradeShip.touch();
|
||||
}
|
||||
}
|
||||
|
||||
const curTile = this.tradeShip.tile();
|
||||
if (curTile === this.dstPort()) {
|
||||
this.complete();
|
||||
return;
|
||||
|
||||
+35
-31
@@ -3,7 +3,7 @@ import { PseudoRandom } from "../PseudoRandom";
|
||||
import { ClientID } from "../Schemas";
|
||||
import {
|
||||
assertNever,
|
||||
distSortUnit,
|
||||
findClosestBy,
|
||||
minInt,
|
||||
simpleHash,
|
||||
toInt,
|
||||
@@ -994,14 +994,18 @@ export class PlayerImpl implements Player {
|
||||
type: UnitType,
|
||||
targetTile: TileRef,
|
||||
): Unit | false {
|
||||
const range = this.mg.config().structureMinDist();
|
||||
const existing = this.mg
|
||||
.nearbyUnits(targetTile, range, type, undefined, true)
|
||||
.sort((a, b) => a.distSquared - b.distSquared);
|
||||
if (existing.length === 0) {
|
||||
return false;
|
||||
}
|
||||
return existing[0].unit;
|
||||
const closest = findClosestBy(
|
||||
this.mg.nearbyUnits(
|
||||
targetTile,
|
||||
this.mg.config().structureMinDist(),
|
||||
type,
|
||||
undefined,
|
||||
true,
|
||||
),
|
||||
(entry) => entry.distSquared,
|
||||
);
|
||||
|
||||
return closest?.unit ?? false;
|
||||
}
|
||||
|
||||
private canBuildUnitType(
|
||||
@@ -1167,27 +1171,29 @@ export class PlayerImpl implements Player {
|
||||
}
|
||||
|
||||
nukeSpawn(tile: TileRef, nukeType: UnitType): TileRef | false {
|
||||
if (this.mg.isSpawnImmunityActive()) {
|
||||
const mg = this.mg;
|
||||
if (mg.isSpawnImmunityActive()) {
|
||||
return false;
|
||||
}
|
||||
const owner = this.mg.owner(tile);
|
||||
// Allow nuking teammates after the game is over (aftergame fun)
|
||||
const gameOver = this.mg.getWinner() !== null;
|
||||
const gameOver = mg.getWinner() !== null;
|
||||
if (owner.isPlayer()) {
|
||||
if (this.isOnSameTeam(owner) && !gameOver) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
const config = mg.config();
|
||||
|
||||
// Prevent launching nukes that would hit teammate structures (only in team games).
|
||||
// Disabled after game-over so players can nuke teammates in the aftergame.
|
||||
if (
|
||||
this.mg.config().gameConfig().gameMode === GameMode.Team &&
|
||||
config.gameConfig().gameMode === GameMode.Team &&
|
||||
nukeType !== UnitType.MIRV &&
|
||||
!gameOver
|
||||
) {
|
||||
const magnitude = this.mg.config().nukeMagnitudes(nukeType);
|
||||
const wouldHitTeammate = this.mg.anyUnitNearby(
|
||||
const magnitude = config.nukeMagnitudes(nukeType);
|
||||
const wouldHitTeammate = mg.anyUnitNearby(
|
||||
tile,
|
||||
magnitude.outer,
|
||||
Structures.types,
|
||||
@@ -1199,15 +1205,14 @@ export class PlayerImpl implements Player {
|
||||
}
|
||||
|
||||
// only get missilesilos that are not on cooldown and not under construction
|
||||
const spawns = this.units(UnitType.MissileSilo)
|
||||
.filter((silo) => {
|
||||
return !silo.isInCooldown() && !silo.isUnderConstruction();
|
||||
})
|
||||
.sort(distSortUnit(this.mg, tile));
|
||||
if (spawns.length === 0) {
|
||||
return false;
|
||||
}
|
||||
return spawns[0].tile();
|
||||
const bestSilo = findClosestBy(
|
||||
this.units(UnitType.MissileSilo),
|
||||
(silo) => mg.manhattanDist(silo.tile(), tile),
|
||||
(silo) =>
|
||||
silo.isActive() && !silo.isInCooldown() && !silo.isUnderConstruction(),
|
||||
);
|
||||
|
||||
return bestSilo?.tile() ?? false;
|
||||
}
|
||||
|
||||
portSpawn(tile: TileRef, validTiles: TileRef[] | null): TileRef | false {
|
||||
@@ -1237,15 +1242,14 @@ export class PlayerImpl implements Player {
|
||||
if (!this.mg.isOcean(tile)) {
|
||||
return false;
|
||||
}
|
||||
const spawns = this.units(UnitType.Port).sort(
|
||||
(a, b) =>
|
||||
this.mg.manhattanDist(a.tile(), tile) -
|
||||
this.mg.manhattanDist(b.tile(), tile),
|
||||
|
||||
const bestPort = findClosestBy(
|
||||
this.units(UnitType.Port),
|
||||
(port) => this.mg.manhattanDist(port.tile(), tile),
|
||||
(port) => port.isActive() && !port.isUnderConstruction(),
|
||||
);
|
||||
if (spawns.length === 0) {
|
||||
return false;
|
||||
}
|
||||
return spawns[0].tile();
|
||||
|
||||
return bestPort?.tile() ?? false;
|
||||
}
|
||||
|
||||
landBasedUnitSpawn(tile: TileRef): TileRef | false {
|
||||
|
||||
@@ -10,6 +10,7 @@ describe("TradeShipExecution", () => {
|
||||
let pirate: Player;
|
||||
let srcPort: Unit;
|
||||
let piratePort: Unit;
|
||||
let piratePort2: Unit;
|
||||
let tradeShip: Unit;
|
||||
let dstPort: Unit;
|
||||
let tradeShipExecution: TradeShipExecution;
|
||||
@@ -48,27 +49,41 @@ describe("TradeShipExecution", () => {
|
||||
id: vi.fn(() => 3),
|
||||
addGold: vi.fn(),
|
||||
displayName: vi.fn(() => "Destination"),
|
||||
units: vi.fn(() => [piratePort]),
|
||||
unitCount: vi.fn(() => 1),
|
||||
units: vi.fn(() => [piratePort, piratePort2]),
|
||||
unitCount: vi.fn(() => 2),
|
||||
canTrade: vi.fn(() => true),
|
||||
} as any;
|
||||
|
||||
piratePort = {
|
||||
tile: vi.fn(() => 40011),
|
||||
tile: vi.fn(() => 56),
|
||||
owner: vi.fn(() => pirate),
|
||||
isActive: vi.fn(() => true),
|
||||
isUnderConstruction: vi.fn(() => false),
|
||||
isMarkedForDeletion: vi.fn(() => false),
|
||||
} as any;
|
||||
|
||||
piratePort2 = {
|
||||
tile: vi.fn(() => 75),
|
||||
owner: vi.fn(() => pirate),
|
||||
isActive: vi.fn(() => true),
|
||||
isUnderConstruction: vi.fn(() => false),
|
||||
isMarkedForDeletion: vi.fn(() => false),
|
||||
} as any;
|
||||
|
||||
srcPort = {
|
||||
tile: vi.fn(() => 20011),
|
||||
tile: vi.fn(() => 10),
|
||||
owner: vi.fn(() => origOwner),
|
||||
isActive: vi.fn(() => true),
|
||||
isUnderConstruction: vi.fn(() => false),
|
||||
isMarkedForDeletion: vi.fn(() => false),
|
||||
} as any;
|
||||
|
||||
dstPort = {
|
||||
tile: vi.fn(() => 30015), // 15x15
|
||||
tile: vi.fn(() => 100),
|
||||
owner: vi.fn(() => dstOwner),
|
||||
isActive: vi.fn(() => true),
|
||||
isUnderConstruction: vi.fn(() => false),
|
||||
isMarkedForDeletion: vi.fn(() => false),
|
||||
} as any;
|
||||
|
||||
tradeShip = {
|
||||
@@ -80,13 +95,13 @@ describe("TradeShipExecution", () => {
|
||||
setSafeFromPirates: vi.fn(),
|
||||
touch: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
tile: vi.fn(() => 2001),
|
||||
tile: vi.fn(() => 32),
|
||||
} as any;
|
||||
|
||||
tradeShipExecution = new TradeShipExecution(origOwner, srcPort, dstPort);
|
||||
tradeShipExecution.init(game, 0);
|
||||
tradeShipExecution["pathFinder"] = {
|
||||
next: vi.fn(() => ({ status: PathStatus.NEXT, node: 2001 })),
|
||||
next: vi.fn(() => ({ status: PathStatus.NEXT, node: 32 })),
|
||||
findPath: vi.fn((from: number) => [from]),
|
||||
} as any;
|
||||
tradeShipExecution["tradeShip"] = tradeShip;
|
||||
@@ -118,7 +133,7 @@ describe("TradeShipExecution", () => {
|
||||
|
||||
it("should complete trade and award gold", () => {
|
||||
tradeShipExecution["pathFinder"] = {
|
||||
next: vi.fn(() => ({ status: PathStatus.COMPLETE, node: 2001 })),
|
||||
next: vi.fn(() => ({ status: PathStatus.COMPLETE, node: 32 })),
|
||||
findPath: vi.fn((from: number) => [from]),
|
||||
} as any;
|
||||
tradeShipExecution.tick(1);
|
||||
|
||||
Reference in New Issue
Block a user