diff --git a/docs/adr/0005-playthrough-settles-attempts.md b/docs/adr/0005-playthrough-settles-attempts.md new file mode 100644 index 0000000..6ae6657 --- /dev/null +++ b/docs/adr/0005-playthrough-settles-attempts.md @@ -0,0 +1,28 @@ +# Playthrough has no LoseLife; it settles Attempts + +Playthrough and Attempt are separate aggregate roots (ADR-0002), but "one death costs exactly one life" requires +ending the Attempt and decrementing lives *together*. Rather than expose bare mutators and rely on the caller to +sequence them, Playthrough exposes only what happened — `Commit(attempt)`, `SettleDeath(attempt)`, +`SettleReplay(attempt)` — each of which calls the Attempt's own `TryEnd…` internally and does nothing when it +returns false. Every operation is therefore atomic and idempotent, and there is no `LoseLife` to call from +somewhere else. + +The alternative shape, `if (attempt.TryEndInDeath()) playthrough.LoseLife();`, puts the idempotency in the domain +but leaves the sequencing to the caller. A public `LoseLife` is an invitation: the code being replaced has two +paths to exactly that operation — `LivesStateHandler.RemoveLife` and `ConsoleManager` writing `Player.Lives` +directly, unclamped — because the operation existed to be called. + +## Considered options + +Collapsing the two into one aggregate with Attempt as an inner entity, making the atomicity structural. Rejected: +`Commit(Attempt)` already gave the shape we needed, and passing one root into another's method is a transient +reference rather than a stored association, so ADR-0002 stands unchanged. + +## Consequences + +- Overlapping hazards in a kill zone, or a hazard re-firing during the two seconds before `DeathScreen` reloads, + cost one life rather than two. +- `ExitDoorComponent` and `ConsoleManager.GoToNextLevelCommand` can both request completion; only the first banks. +- The application layer holds the active Attempt and passes it in. The Attempt carries its own `LevelId`, so + `Commit` decides First Clear versus Replay from the Attempt itself rather than trusting a level supplied + alongside it — finishing a Replay stops being a way to corrupt campaign position. diff --git a/docs/plans/phase-a-progression-domain.md b/docs/plans/phase-a-progression-domain.md index 0230e7e..d453510 100644 --- a/docs/plans/phase-a-progression-domain.md +++ b/docs/plans/phase-a-progression-domain.md @@ -73,12 +73,19 @@ Invariants to drive out with tests: The ledger from ADR-0002: `CoinsFound`, `CoinsSpent`, `SkillsBought`, plus an outcome. -- `RecordCoinsFound(Coins)` -- `RecordPurchase(SkillId, Coins cost)` +- `Attempt(LevelId level)` and `LevelId Level` — the Attempt knows which Level it is an attempt at, so + `Commit` decides First Clear versus Replay from the Attempt rather than from a level the caller passes alongside - `Outcome` — `InProgress | Died | Cleared` +- `void RecordCoinsFound(Coins)` — silently ignores late coins once ended +- `bool TryRecordPurchase(SkillId, SkillLevel resulting, Coins cost)` — records where the skill ended up, since + one Attempt can both unlock and upgrade the same skill - `bool TryEndInDeath()` / `bool TryEndInCompletion()` — return false if the Attempt already ended - No `Discard` method: discarding is throwing the object away. +The recording methods are deliberately asymmetric. A coin pickup firing after death is a real event — the player +node survives for two seconds before `DeathScreen` reloads — and its caller has nothing useful to do with a +`false`, so it returns nothing. A purchase's caller does care. + `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. @@ -111,8 +118,12 @@ incidental and `Health` is a public `[Export]` setter that bypasses it. Do not r Aggregate root. `PlaythroughId`, `Lives`, `Bank`, `OwnedSkills : SkillId → SkillLevel`, `CurrentLevel : LevelId`, `ClearedLevels`. -Operations: `LoseLife`, `Commit(Attempt)`, `CanAfford(Attempt, Coins)`, `HasCleared(LevelId)`, -`ReachableLevels(order)`, `IsRunOver`. +Operations: `Commit(Attempt)`, `SettleDeath(Attempt)`, `SettleReplay(Attempt)`, `CanAfford(Attempt, Coins)`, +`HasCleared(LevelId)`, `ReachableLevels(order)`, `IsRunOver`. + +**There is no `LoseLife`** — see ADR-0005. Each settle operation calls the Attempt's `TryEnd…` internally and does +nothing when it returns false, so it is atomic and idempotent, and no bare mutator exists to be called from a +second place. Raises: `LifeLost(remaining)`, `RunEnded`, `BankChanged(total)`, `SkillSetChanged`. diff --git a/progression.tests/AttemptTests.cs b/progression.tests/AttemptTests.cs new file mode 100644 index 0000000..17a8cd0 --- /dev/null +++ b/progression.tests/AttemptTests.cs @@ -0,0 +1,98 @@ +namespace Mr.BrickAdventures.Progression.Tests; + +public class AttemptTests +{ + [Fact] + public void An_attempt_cannot_die_twice() + { + // Overlapping hazards in a kill zone, or a hazard re-firing during the two seconds before + // DeathScreen reloads the scene, must not cost two lives. The Attempt refuses the second + // death, so no caller has to remember to guard against it. + var attempt = new Attempt(LevelId.Of("village_1")); + + Assert.True(attempt.TryEndInDeath()); + Assert.False(attempt.TryEndInDeath()); + Assert.Equal(AttemptOutcome.Died, attempt.Outcome); + } + + [Fact] + public void An_attempt_that_died_cannot_then_be_cleared() + { + var attempt = new Attempt(LevelId.Of("village_1")); + attempt.TryEndInDeath(); + + Assert.False(attempt.TryEndInCompletion()); + Assert.Equal(AttemptOutcome.Died, attempt.Outcome); + } + + [Fact] + public void An_attempt_that_was_cleared_cannot_then_die() + { + // The reverse of the above. Together these say a terminal outcome is final, which is what + // lets both ExitDoorComponent and the console request completion without banking twice. + var attempt = new Attempt(LevelId.Of("village_1")); + attempt.TryEndInCompletion(); + + Assert.False(attempt.TryEndInDeath()); + Assert.Equal(AttemptOutcome.Cleared, attempt.Outcome); + } + + [Fact] + public void Coins_found_after_the_attempt_ended_do_not_count() + { + // The player node survives for two seconds after death while DeathScreen counts down, so a + // coin's Area2D can still fire. That is a real event rather than an error, and it must not + // enlarge a ledger that is already settled. + var attempt = new Attempt(LevelId.Of("village_1")); + attempt.RecordCoinsFound(Coins.Of(3)); + attempt.RecordCoinsFound(Coins.Of(2)); + attempt.TryEndInDeath(); + + attempt.RecordCoinsFound(Coins.Of(100)); + + Assert.Equal(Coins.Of(5), attempt.CoinsFound); + } + + [Fact] + public void A_purchase_after_the_attempt_ended_is_refused() + { + var attempt = new Attempt(LevelId.Of("village_1")); + attempt.TryEndInDeath(); + + var recorded = attempt.TryRecordPurchase(SkillId.Of("brick_throw"), SkillLevel.Of(1), Coins.Of(10)); + + Assert.False(recorded); + Assert.Equal(Coins.Zero, attempt.CoinsSpent); + Assert.Empty(attempt.SkillsBought); + } + + [Fact] + public void Spending_more_than_was_found_this_attempt_is_recorded_not_rejected() + { + // The shortfall is drawn from the Bank when the Attempt is committed. Rejecting it here + // would make the ledger enforce a rule it cannot see — affordability spans both scopes. + var attempt = new Attempt(LevelId.Of("village_1")); + attempt.RecordCoinsFound(Coins.Of(10)); + + var recorded = attempt.TryRecordPurchase(SkillId.Of("magnetic"), SkillLevel.Of(1), Coins.Of(30)); + + Assert.True(recorded); + Assert.Equal(Coins.Of(10), attempt.CoinsFound); + Assert.Equal(Coins.Of(30), attempt.CoinsSpent); + } + + [Fact] + public void Unlocking_then_upgrading_one_skill_records_the_level_it_ended_at() + { + // Both happen in the marketplace mid-Attempt. Commit applies one level per skill, so the + // ledger keeps where it ended up, while the costs accumulate. + var brickThrow = SkillId.Of("brick_throw"); + var attempt = new Attempt(LevelId.Of("village_1")); + + attempt.TryRecordPurchase(brickThrow, SkillLevel.Of(1), Coins.Of(10)); + attempt.TryRecordPurchase(brickThrow, SkillLevel.Of(2), Coins.Of(25)); + + Assert.Equal(SkillLevel.Of(2), attempt.SkillsBought[brickThrow]); + Assert.Equal(Coins.Of(35), attempt.CoinsSpent); + } +} diff --git a/progression/Attempt.cs b/progression/Attempt.cs new file mode 100644 index 0000000..56b8a19 --- /dev/null +++ b/progression/Attempt.cs @@ -0,0 +1,75 @@ +namespace Mr.BrickAdventures.Progression; + +/// +/// One try at a Level: a ledger of everything that happened during the try, plus how it ended. +/// Never persisted — a discarded Attempt is one that is simply thrown away. +/// +public sealed class Attempt +{ + public Attempt(LevelId level) => Level = level; + + /// + /// Which Level this is an attempt at. Settlement reads this rather than trusting a level + /// supplied by the caller, so finishing a Replay cannot advance campaign position. + /// + public LevelId Level { get; } + + public AttemptOutcome Outcome { get; private set; } = AttemptOutcome.InProgress; + + public Coins CoinsFound { get; private set; } = Coins.Zero; + + /// + /// Records coins picked up during this Attempt. Silently ignored once the Attempt has ended: + /// a pickup firing after death is a real event, not an error, and it does not count. + /// + public void RecordCoinsFound(Coins amount) + { + if (Outcome != AttemptOutcome.InProgress) return; + + CoinsFound = CoinsFound.Add(amount); + } + + public Coins CoinsSpent { get; private set; } = Coins.Zero; + + private readonly Dictionary _skillsBought = new(); + + /// + /// Records a Skill bought or upgraded during this Attempt, at the level it ended up at. + /// Returns false once the Attempt has ended. + /// + /// + /// Affordability is not checked here — it spans the Attempt and the Playthrough's Bank, so it + /// belongs to the caller that can see both. This records what was agreed, not whether it was + /// allowed. + /// + public bool TryRecordPurchase(SkillId skill, SkillLevel resulting, Coins cost) + { + if (Outcome != AttemptOutcome.InProgress) return false; + + _skillsBought[skill] = resulting; + CoinsSpent = CoinsSpent.Add(cost); + return true; + } + + public IReadOnlyDictionary SkillsBought => _skillsBought; + + /// + /// Ends this Attempt in death. Returns false if it has already ended, in which case nothing + /// changes — a second death event costs no further lives. + /// + public bool TryEndInDeath() => TryEnd(AttemptOutcome.Died); + + /// + /// Ends this Attempt in completion. Returns false if it has already ended, in which case + /// nothing changes — a second completion event banks nothing further. + /// + public bool TryEndInCompletion() => TryEnd(AttemptOutcome.Cleared); + + private bool TryEnd(AttemptOutcome outcome) + { + if (Outcome != AttemptOutcome.InProgress) return false; + + Outcome = outcome; + return true; + } +} diff --git a/progression/AttemptOutcome.cs b/progression/AttemptOutcome.cs new file mode 100644 index 0000000..765d027 --- /dev/null +++ b/progression/AttemptOutcome.cs @@ -0,0 +1,12 @@ +namespace Mr.BrickAdventures.Progression; + +/// +/// How an Attempt finished, or that it has not finished yet. An Attempt reaches a terminal outcome +/// once and refuses to change afterwards — that refusal is what guarantees one death costs one life. +/// +public enum AttemptOutcome +{ + InProgress, + Died, + Cleared, +} diff --git a/progression/Coins.cs b/progression/Coins.cs index 64e7b6c..b26c593 100644 --- a/progression/Coins.cs +++ b/progression/Coins.cs @@ -10,10 +10,14 @@ public readonly struct Coins : IEquatable private Coins(int value) => Value = value; + public static readonly Coins Zero = new(0); + public static Coins Of(int value) => value < 0 ? throw new ArgumentOutOfRangeException(nameof(value), value, "Coins cannot be negative.") : new Coins(value); + public Coins Add(Coins other) => new(Value + other.Value); + /// /// Subtracts . Returns false when the amount exceeds what is held, /// leaving untouched rather than clamping to zero.