Fix structure cost double-counting units under construction (#4320)

## 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>
This commit is contained in:
Evan
2026-06-17 10:19:04 -07:00
committed by GitHub
parent 305534cc65
commit 661d96ba28
2 changed files with 83 additions and 10 deletions
+5 -10
View File
@@ -416,17 +416,12 @@ export class PlayerImpl implements Player {
}
}
// Count of units built by the player, including construction
// Count of units built by the player, including those still under
// construction. recordUnitConstructed() is called in buildUnit() the moment a
// unit is created (while still under construction), so numUnitsConstructed
// already accounts for in-progress builds — don't re-count them.
unitsConstructed(type: UnitType): number {
const built = this.numUnitsConstructed[type] ?? 0;
let constructing = 0;
for (const unit of this._units) {
if (unit.type() !== type) continue;
if (!unit.isUnderConstruction()) continue;
constructing++;
}
const total = constructing + built;
return total;
return this.numUnitsConstructed[type] ?? 0;
}
// Count of units owned by the player, not including construction
+78
View File
@@ -0,0 +1,78 @@
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);
});
});