mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 08:00:43 +00:00
Evan
661d96ba28
## Problem
The ghost/build-menu price of a structure can show the wrong (inflated)
cost. Concretely: a player who owns a **captured** city and then starts
building their **first** city sees the 3rd-city price (**500k**) for
that build instead of the 2nd-city price (**250k**).
## Root cause
Structure cost scales as `2^(units built) × base` (city: 125k / 250k /
500k …), counted via:
```ts
Math.min(player.unitsOwned(type), player.unitsConstructed(type))
```
The `Math.min` is deliberate — it caps the count at how many you've
actually **built**, so **captured** units (owned but not built) don't
inflate the price.
`unitsConstructed()` defeated that by double-counting in-progress
builds:
```ts
const built = this.numUnitsConstructed[type] ?? 0; // already includes the building unit
let constructing = 0;
for (const unit of this._units) {
if (unit.type() !== type) continue;
if (!unit.isUnderConstruction()) continue;
constructing++; // counts the SAME unit again
}
return constructing + built; // doubled
```
`recordUnitConstructed()` is called in `buildUnit()` the moment the unit
is created — while it is still under construction — so
`numUnitsConstructed` already accounts for in-progress builds. The extra
loop counted them a second time.
With one captured city + one city under construction: `unitsOwned = 2`,
double-counted `unitsConstructed = 2`, so `Math.min(2, 2) = 2` → 500k.
Without the double-count it's `Math.min(2, 1) = 1` → 250k. ✅
The redundant loop is a leftover from #2378, which removed the separate
`UnitType.Construction` unit. Back then in-progress builds were a
distinct unit type **not** recorded in `numUnitsConstructed`, so the
loop was needed; afterward it became a pure double-count. This is a
long-standing latent bug — present identically on `v31` — not a recent
regression.
## Fix
`unitsConstructed()` now just returns `numUnitsConstructed[type]`, which
already includes under-construction builds.
## Tests
`tests/economy/ConstructionCost.test.ts` covers both:
- pure case (first city under construction) → still 250k
- captured city + first city under construction → was 500k, now 250k
(fails without the fix with `expected 2 to be 1`)
All related suites (economy, PlayerImpl, nation structure behavior,
upgrades, MIRV pricing, stats) — 144 tests — pass.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
79 lines
2.8 KiB
TypeScript
79 lines
2.8 KiB
TypeScript
import { ConstructionExecution } from "../../src/core/execution/ConstructionExecution";
|
|
import {
|
|
Game,
|
|
Player,
|
|
PlayerInfo,
|
|
PlayerType,
|
|
UnitType,
|
|
} from "../../src/core/game/Game";
|
|
import { setup } from "../util/Setup";
|
|
|
|
// Regression test: the ghost/build-menu price of a structure must not double-count
|
|
// a player's first structure while it is still under construction.
|
|
//
|
|
// The cost scales as 2^(units built) * base: 1st city 125k, 2nd 250k, 3rd 500k.
|
|
// Cost uses Math.min(unitsOwned, unitsConstructed) so CAPTURED units (owned but
|
|
// not built) don't inflate the price. unitsConstructed used to also loop over
|
|
// under-construction units, double-counting them and defeating that Math.min —
|
|
// a captured city plus a first city under construction showed 500k (3rd-city
|
|
// price) instead of 250k.
|
|
describe("Structure cost while under construction", () => {
|
|
let game: Game;
|
|
let player: Player;
|
|
let other: Player;
|
|
|
|
const builderInfo = new PlayerInfo(
|
|
"builder",
|
|
PlayerType.Human,
|
|
null,
|
|
"builder_id",
|
|
);
|
|
const otherInfo = new PlayerInfo("other", PlayerType.Human, null, "other_id");
|
|
|
|
beforeEach(async () => {
|
|
game = await setup(
|
|
"plains",
|
|
{ infiniteGold: false, instantBuild: false, infiniteTroops: true },
|
|
[builderInfo, otherInfo],
|
|
);
|
|
player = game.player(builderInfo.id);
|
|
other = game.player(otherInfo.id);
|
|
player.conquer(game.ref(0, 10));
|
|
other.conquer(game.ref(15, 15));
|
|
player.addGold(100_000_000n);
|
|
other.addGold(100_000_000n);
|
|
});
|
|
|
|
function buildFirstCityUnderConstruction() {
|
|
game.addExecution(
|
|
new ConstructionExecution(player, UnitType.City, game.ref(0, 10)),
|
|
);
|
|
game.executeNextTick(); // init
|
|
game.executeNextTick(); // build unit + setUnderConstruction(true)
|
|
const built = player
|
|
.units(UnitType.City)
|
|
.find((u) => u.tile() === game.ref(0, 10));
|
|
expect(built?.isUnderConstruction()).toBe(true);
|
|
}
|
|
|
|
test("first city under construction does not double-count itself", () => {
|
|
buildFirstCityUnderConstruction();
|
|
// One built city (under construction) → next city is the 2nd → 250k.
|
|
expect(player.unitsConstructed(UnitType.City)).toBe(1);
|
|
expect(game.unitInfo(UnitType.City).cost(game, player)).toBe(250_000n);
|
|
});
|
|
|
|
test("captured city does not inflate the price of a city under construction", () => {
|
|
// 'other' builds a city; 'player' captures it (owns it without building it).
|
|
const captured = other.buildUnit(UnitType.City, game.ref(15, 15), {});
|
|
player.captureUnit(captured);
|
|
|
|
buildFirstCityUnderConstruction();
|
|
|
|
// Player has BUILT exactly one city (still under construction). The captured
|
|
// city must not count toward build cost, so the next city is still 250k.
|
|
expect(player.unitsConstructed(UnitType.City)).toBe(1);
|
|
expect(game.unitInfo(UnitType.City).cost(game, player)).toBe(250_000n);
|
|
});
|
|
});
|