feat(progression): Coins, LevelTime, SkillLevel core invariants (slice 1)

three bug-encoding invariants, one red/green cycle each:
- Coins.TrySubtract fails instead of clamping
- LevelTime rejects zero (continued-run zero best-times)
- SkillLevel has no zero (unowned is absence, not Level=1)

plan: drop signed NetCoins; add Attempt outcome as idempotency guard
This commit is contained in:
2026-07-28 18:22:46 +02:00
parent c3bbf3282a
commit 313a5315b1
7 changed files with 142 additions and 7 deletions

View File

@@ -71,18 +71,38 @@ Invariants to drive out with tests:
## Slice 2 — Attempt · TDD
The ledger from ADR-0002: `CoinsFound`, `CoinsSpent`, `SkillsBought`.
The ledger from ADR-0002: `CoinsFound`, `CoinsSpent`, `SkillsBought`, plus an outcome.
- `RecordCoinsFound(Coins)`
- `RecordPurchase(SkillId, Coins cost)`
- `NetCoins` — may be negative, when purchases drew against the Bank
- `Outcome` — `InProgress | Died | Cleared`
- `bool TryEndInDeath()` / `bool TryEndInCompletion()` — return false if the Attempt already ended
- No `Discard` method: discarding is throwing the object away.
`CoinsFound` and `CoinsSpent` are both non-negative. There is no signed net value — settlement is
`bank.Add(found).TrySubtract(spent)` in `Playthrough.Commit`, which in that order cannot go negative given
affordability was checked at purchase time.
> Corrects an earlier draft of this plan, which had `NetCoins` "may be negative". That would have forced `Coins`
> to permit negatives and defeated the point of the type.
**The outcome is the idempotency guard, and it is the reason this slice matters.** An Attempt is the thing that
dies or is cleared, and it can only do either once. `Lives` flooring at zero does not solve this: losing two lives
from a single death takes you from 3 to 1, which no clamp catches.
Tests worth writing:
- Spending more than found is representable — the shortfall is settled at Commit, not at purchase time.
- Net coins after found 10, spent 30 is −20.
- Nothing on Attempt persists or mutates anything else.
- **An Attempt cannot end twice.** Second `TryEndInDeath` returns false and changes nothing.
- **An Attempt that died cannot then be cleared**, and vice versa.
- Spending more than found is representable — the shortfall is settled at Commit, not rejected here.
- Recording anything after the Attempt ended is refused.
Two real call sites motivate this. Death: overlapping hazards in a kill zone, or a hazard re-triggering during the
two seconds before `DeathScreen` reloads the scene — the player node still exists with zero health. Completion:
`ExitDoorComponent:46` and `ConsoleManager.GoToNextLevelCommand:99` both call `OnLevelComplete()`.
`HealthComponent.ApplyHealthChange` currently absorbs repeat deaths via its `delta == 0` early return, but that is
incidental and `Health` is a public `[Export]` setter that bypasses it. Do not rely on the engine side.
---
@@ -101,9 +121,12 @@ This is where the report's bugs become tests. Write these first, watch them fail
- **Losing the last life sets lives to zero *and* raises `RunEnded` in one operation.** This is the last-life
soft-lock — today two screens read `Lives` before the handler writes it. Make it one atomic operation and the
race cannot be expressed.
- **Two deaths in one Attempt cost one life.** Pairs with slice 2's outcome guard; assert it here at the
Playthrough level too, because this is the invariant a player would actually notice.
- **`Commit` on an already-cleared level does not advance and does not bank** (ADR-0003).
- **`Commit` applies `NetCoins`**, so an Attempt that overspent reduces the Bank correctly, and a discarded
Attempt leaves the Bank untouched — the "buy mid-level, die, lose coins *and* skill" bug.
- **`Commit` settles as `bank.Add(found).TrySubtract(spent)`**, in that order, so an Attempt that overspent
reduces the Bank correctly and can never drive it negative. A discarded Attempt leaves the Bank untouched —
the "buy mid-level, die, lose coins *and* skill" bug.
- **`ReachableLevels` is computed** from `ClearedLevels` plus level order, never stored. `UnlockedLevels` and
`IsLevelUnlocked` are deleted, not ported.
- **`Bank` cannot go negative** even through Commit.