Files
OpenFrontIO/src/core/PseudoRandom.ts
T
FloPinguin 0801cad0b5 Fix NaN coordinates in Warship patrol logic 🚢 (#3697)
## Description:

This PR fixes the `Invalid coordinates: NaN,NaN` crash during Warship
patrol execution.

### Root cause

`WarshipExecution.randomTile` picks a patrol destination inside
`warshipPatrolRange / 2` of the current patrol tile. When a search fails
to find a valid tile, the range expands by 50% per retry (`100 → 150 →
225 → 337`) and becomes odd. Once odd, `warshipPatrolRange / 2` is a
float (e.g. `112.5`), which is handed straight to
`PseudoRandom.nextInt`:

```ts
Math.floor(this.rng() * (max - min)) + min;
```

With a float `min`, this returns `integer + float` - a float. Despite
its name, `nextInt` was silently returning a non-integer. From there:

- `x = mg.x(patrolTile) + floatOffset` → float
- `mg.isValidCoord(floatX, floatY)` → `true` (only bounds were checked)
- `mg.ref(floatX, floatY)` → `yToRef[floatY] + floatX` → `undefined +
float` → `NaN`
- `hasWaterComponent(NaN, …)` → `miniMap.ref(NaN, NaN)` → **throw**

### Why this only started crashing recently

The float‑leaking `nextInt` bug has been latent since at least the
pathfinding refactor (#2866, January), which introduced the
`hasWaterComponent` check. It was invisible because the guard directly
above it short‑circuited on `NaN`:

```ts
if (!this.mg.isOcean(tile) || (!allowShoreline && this.mg.isShoreline(tile))) continue;
```

For a `NaN` tile ref, `terrain[NaN]` is `undefined`, so:

- `isOcean(NaN)` → `Boolean(undefined & OCEAN_BIT)` → **`false`**
- `isLand(NaN)` → **`false`**
- `isWater(NaN)` → `!isLand(NaN)` → **`true`**

Before: `!isOcean(NaN)` was `true`, execution hit `continue`, and the
poisoned ref never reached `hasWaterComponent`. The "Trading in lakes"
PR (#3653) relaxed that single line to allow patrol on lakes:

```diff
- if (!this.mg.isOcean(tile) || ...) continue;
+ if (!this.mg.isWater(tile) || ...) continue;
```

Because `isWater(NaN)` is `true`, `!isWater(NaN)` is now `false` -
execution falls through to `hasWaterComponent(NaN, …)` and crashes.
#3653 didn't introduce the bug; it just happened to remove the
accidental NaN filter that was hiding it.

### Changes

- **`PseudoRandom.nextInt`** - root‑cause fix. Floors both `min` and
`max` so `nextInt` always returns an integer regardless of what callers
pass. Future callers can't re‑trip this trap.
- **`WarshipExecution.randomTile`** - replaced the unsafe
`this.warship.patrolTile()!` non‑null assertion with a proper
`undefined` guard that returns early.
- **`GameMap.isValidCoord`** - defense in depth: also requires
`Number.isInteger(x)` and `Number.isInteger(y)`. Non‑integer coords can
still be produced outside `nextInt` (trig, arithmetic); this makes
`ref()` fail loudly at the boundary instead of silently producing `NaN`
refs.

### Original stacktrace

Please paste the following in your bug report in Discord:
Game crashed!
game id: gGicMpDh
client id: wXE5SpT2
Error: Invalid coordinates: NaN,NaN
Message: Error: Invalid coordinates: NaN,NaN
at at.ref
(https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:64853)
at r_.hasWaterComponent
(https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:243326)
at l_.hasWaterComponent
(https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:260740)
at b1.randomTile
(https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:92634)
at b1.patrol
(https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:91728)
at b1.tick
(https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:89996)
at
https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:251463
    at Array.forEach (<anonymous>)
at l_.executeNextTick
(https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:251383)
at p_.executeNextTick
(https://nightly.openfront.dev/assets/Worker.worker-DL_guV2P.js:31:271256)

Discord:
https://discord.com/channels/1284581928254701718/1494336024740888667

## 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:

FloPinguin
2026-04-17 17:34:12 -07:00

79 lines
2.0 KiB
TypeScript

import seedrandom from "seedrandom";
export class PseudoRandom {
private rng: seedrandom.PRNG;
private static readonly POW36_8 = Math.pow(36, 8); // Pre-compute 36^8
constructor(seed: number) {
this.rng = seedrandom(String(seed));
}
// Generates the next pseudorandom number between 0 and 1.
next(): number {
return this.rng();
}
// Generates a random integer between min (inclusive) and max (exclusive).
nextInt(min: number, max: number): number {
const lo = Math.floor(min);
const hi = Math.floor(max);
return Math.floor(this.rng() * (hi - lo)) + lo;
}
// Generates a random float between min (inclusive) and max (exclusive).
nextFloat(min: number, max: number): number {
return this.rng() * (max - min) + min;
}
// Generates a random ID (8 characters, alphanumeric).
nextID(): string {
return Math.floor(this.rng() * PseudoRandom.POW36_8)
.toString(36)
.padStart(8, "0");
}
// Selects a random element from an array.
randElement<T>(arr: T[]): T {
if (arr.length === 0) {
throw new Error("array must not be empty");
}
return arr[this.nextInt(0, arr.length)];
}
// Selects a random element from a set.
randFromSet<T>(set: Set<T>): T {
const size = set.size;
if (size === 0) {
throw new Error("set must not be empty");
}
const index = this.nextInt(0, size);
let i = 0;
for (const item of set) {
if (i === index) {
return item;
}
i++;
}
// This should never happen
throw new Error("Unexpected error selecting element from set");
}
// Returns true with probability 1/odds.
chance(odds: number): boolean {
return this.nextInt(0, odds) === 0;
}
// Returns a shuffled copy of the array using Fisher-Yates algorithm.
shuffleArray<T>(array: T[]): T[] {
const result = [...array];
for (let i = result.length - 1; i > 0; i--) {
const j = this.nextInt(0, i + 1);
[result[i], result[j]] = [result[j], result[i]];
}
return result;
}
}