przygody-pana-cegly/docs/plans/phase-a-progression-domain.md

Phase A — Progression domain

Replace the current state layer with the model in CONTEXT.md, governed by ADR-0001..0004. No new player-facing features. Save files are wiped once; the game is unreleased so this costs nothing (ADR context).

Phase B — slot picker UI, level select, Replay surfacing, best times out of the ghost files — is out of scope here. Phase A does put the persistence format in place for slots, so Phase B is UI work rather than another model change.

Two kinds of slice

	verified by	/tdd
Slices 1–6 — pure domain, no Godot	dotnet test	yes — red/green/refactor throughout
Slices 7–13 — adapters and wiring	compiler + playing the game	no — write the test where one is possible, otherwise verify by hand

Don't try to TDD slices 7–13. You can't red-green an autoload swap or a .tscn connection deletion, and forcing it produces tests that assert the mock rather than the behaviour. The value of slices 1–6 is that they absorb all the logic worth testing, so the untestable slices become mechanical.

Each slice ends with the build green and the game runnable. Don't start the next one until it is.

---

Slice 0 — scaffolding

No tests. Pure setup.

• progression/Mr.BrickAdventures.Progression.csproj — plain net8.0 library, no Godot reference, root namespace Mr.BrickAdventures.Progression.
• progression.tests/Mr.BrickAdventures.Progression.Tests.csproj — xunit (NUnit is equally fine), references the domain library only.
• Add both to Mr. Brick Adventures.sln. Add a ProjectReference from the game project to the domain library.
• Set <EnableDynamicLoading>true</EnableDynamicLoading> in the domain library too, not just the game project — Godot needs it on referenced assemblies for hot reload, and omitting it produces confusing load failures.

Done when: dotnet test runs and reports zero tests; Godot still builds; the game still launches and plays.

Guard rail for the whole phase:

grep -rn "using Godot" progression progression.tests --include=*.cs   # must print nothing

That single check is what ADR-0004 buys you. Scope it to --include=*.cs — an unscoped grep -r also matches prose in the .csproj comments and reports a false violation.

---

Slice 1 — value objects · TDD

Coins, Lives, LevelId, SkillId, SkillLevel, StatName, LevelTime, PlaythroughId.

Invariants to drive out with tests:

• Coins cannot be negative. Subtraction that would go negative is a failure the caller must handle, not a silent clamp — the current Mathf.Max(0, ...) clamp in one path and absence of it in another is exactly the bug this removes.
• Lives floors at zero, and reaching zero is observable.
• LevelId is a non-empty stable string. Not an int, not an index.
• SkillLevel starts at 1 and has no zero value — "unowned" is represented by absence, never by level 0. See the SkillData.Level = 1 ambiguity this replaces.
• LevelTime is positive; comparison is what "best" means.
• StatName is a closed set (the five in StatNames today), so a typo cannot invent a statistic.

Done when: no value object can hold invalid state, and every clamp/guard currently scattered across GameStateStore and ConsoleManager exists in exactly one place.

---

Slice 2 — Attempt · TDD

The ledger from ADR-0002: CoinsFound, CoinsSpent, SkillsBought, plus an outcome.

• 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.

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:

• 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.

---

Slice 3 — Playthrough · TDD

Aggregate root. PlaythroughId, Lives, Bank, OwnedSkills : SkillId → SkillLevel, CurrentLevel : LevelId, ClearedLevels.

Operations: SettleDeath(Attempt), SettleCompletion(Attempt, Campaign), CanAfford(Attempt, Coins), HasCleared(LevelId), ReachableLevels(Campaign), 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.

Two settle operations, not three. An earlier draft had Commit and SettleReplay as separate calls, which made the caller decide whether a completion was a First Clear or a Replay — precisely the ADR-0003 regression. SettleCompletion branches on HasCleared(attempt.Level) itself, so it cannot be selected wrongly.

Campaign is a domain value object holding the authored ordering of LevelIds (First, Next, Contains). ILevelCatalogue stays on the adapter side resolving a LevelId to a scene handle and display name; the ordering is pure data handed in as a Campaign, so the aggregate never touches a port.

Commit does not validate skill levels against the upgrade cap. Max level comes from SkillData.Upgrades.Count, which is authored data the domain cannot see. The cap is a purchase-time rule enforced in slice 6's UpgradeSkill, which is also where TryUpgradeSkill's current Upgrades[skill.Level] overrun gets a test. Commit applies what the ledger recorded.

Events leave via IReadOnlyList<ProgressionEvent> DrainEvents(). The composition root drains once after every use case and republishes (ADR-0004), so operations stay void and the "forgot to drain" risk collapses to one line in one place.

Raises: LifeLost(remaining), RunEnded, BankChanged(total), SkillSetChanged.

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 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.

---

Slice 4 — Profile · TDD

Aggregate root, one per installation. Achievements, BestTimes : LevelId → LevelTime, Statistics.

• RecordTime(LevelId, LevelTime) — only replaces a slower time. Recording a worse time is a no-op, not an error.
• UnlockAchievement(id) — idempotent, raises AchievementUnlocked only on the transition.
• Record(StatName, int).
• RelockAchievement(id) exists for the console only, and is documented as not reaching Steam — that asymmetry is unavoidable (Steam achievements cannot be un-earned) so make it explicit rather than surprising.

---

Slice 5 — ports and in-memory adapters · TDD

Interfaces in the domain: IPlaythroughStore, IProfileStore, ILevelCatalogue, IGhostStore.

In-memory implementations live in the test project. That's what makes the seam real rather than hypothetical — two adapters, one of which ships and one of which doesn't.

• Round-trip: save a Playthrough, load it, assert equality. Same for Profile.
• Loading an absent Playthrough and loading a corrupt one are different outcomes. Today both surface as false from LoadGame, which is why a version bump silently overwrites saves.
• ILevelCatalogue returns ordered LevelIds and resolves one to a scene reference — as an opaque handle, since the domain can't name PackedScene.

---

Slice 6 — use cases · TDD

StartPlaythrough, LoadPlaythrough, EnterLevel, FinishLevel, HandlePlayerDeath, PurchaseSkill, UpgradeSkill.

Four of these carry the orchestration that is currently spread across GameManager, the Events/ handlers and the UI. Each of the remaining report bugs gets a test here:

• EnterLevel always announces LevelStarted. Today only TryToGoToNextLevel emits it, so ghost recording never starts on the first level or after Continue, and a continued run records completion times of zero.
• FinishLevel branches First Clear vs Replay and a Replay never advances CurrentLevel (ADR-0003 flags this as the obvious regression).
• HandlePlayerDeath loses a life and discards the Attempt as one operation.
• PurchaseSkill writes only the Attempt, checks affordability across Attempt + Bank, and rejects rather than throwing when short.
• UpgradeSkill reads the level from the Playthrough, and cannot index past the upgrade table.

Done when: every bug listed in the architecture review has a test that failed before the fix and passes after. That is the deliverable of Phase A — the point where "hidden bugs" stops being a category.

---

Everything above is pure C# with no Godot. Everything below touches the engine. Switch verification mode here.

---

Slice 7 — level catalogue adapter

resources/levels/ already holds six LevelResource files (village_1..5, forest_1) but there are eight level scenes and only two scenes reference a resource at all. Complete and adopt them.

• Add LevelId and explicit order to LevelResource; create the two missing entries (tutorial_1, village_6).
• A catalogue resource lists them in order. Adapter implements ILevelCatalogue.
• Delete GameManager.LevelScenes and all array-index addressing.
• LevelResource.ScenePath is currently never read — either it becomes how the catalogue resolves scenes, or it goes. Don't leave it.

Verify: game boots into the first level; order matches the catalogue; DeathScreen's level-name label still populates.

Slice 8 — JSON store adapters

• user://profile.json and user://playthrough_{id}.json. The slot format lands now even though the UI only ever creates one; Phase B then adds a picker and nothing about the format changes.
• Write to a temp file and rename, so a crash mid-save can't corrupt a slot. With N slots this stops being theoretical.
• Keep a version field, but replace the equality check with a migration hook that can return "unmigratable" — distinct from "absent".

Verify: play, quit, continue. Then hand-corrupt a file and confirm you get an error rather than a silent new game.

Slice 9 — composition root and republisher

• New autoload owns the Profile and the active Playthrough. It is the only Node in this area.
• Republisher subscribes to domain events and emits the existing EventBus signals (ADR-0004), so Hud, DeathScreen and the rest keep working unchanged.
• Delete GameStateStore. Register the new autoload in project.godot in its place.

Verify: HUD shows coins and lives; a coin pickup updates them.

Slice 10 — swap the call sites

Mechanical and compiler-driven — sealing the aggregates turns every old write into a compile error, which is the cheapest way to find them all.

• Events/ handlers become thin adapters that call use cases. CoinStateHandler already has this shape.
• ConsoleManager calls use cases instead of writing fields and hand-re-emitting signals.
• AchievementManager reads and writes the Profile instead of reaching into PlayerState.
• SkillManager reconciles ActiveComponents from SkillSetChanged — ApplyUnlockedSkills becomes the only path, so the four two-call pairs collapse to one call each and a skill can no longer end up owned but inert.
• Delete SaveSystem, PlayerState, SessionState, SaveDataDto.

Verify: compiler first, then play one level start to finish including a purchase.

Slice 11 — skill level off the asset

• ISkill.Initialize(Node owner, SkillData data, int level). Update BrickThrowComponent:90, MagneticSkillComponent:97, BrickArmorSkillComponent:25 to use the passed level rather than Data.Level. BrickArmorSkillComponent indexes Upgrades[Level - 1] unguarded — fix while you're there.
• Delete SkillData.Level. SkillData becomes immutable authored data (CONTEXT.md).
• MarketplaceButton and Marketplace read the level from the Playthrough.

Verify: buy a skill, upgrade it, quit, continue — level persists. Game over, New Game — level is gone. Both are currently wrong.

Landmine to note: five skills (brick_armor, brick_shield, double_jump, ground_pound_skill, wall_jump) have no upgrades/ entries, so Upgrades.Count == 0. TryUnlockSkill would throw on Upgrades[0].Cost; it's currently unreachable only because MarketplaceButton hides zero-upgrade skills. Decide whether those skills are purchasable at all and make the domain say so.

Slice 12 — one death path

• Delete the 18 Death → OnPlayerDeath connections across objects/level/base_level.tscn and the eight level scenes.
• DeathScreen and GameOverScreen subscribe to republished LivesChanged / RunEnded.
• DeathScreen currently reads Player.Lives directly and subscribes to LivesChanged — it keeps only the subscription.

Verify: die with lives remaining → scene reloads. Die on the last life → game over appears. The second case does not work today.

Slice 13 — delete the dead surface

• The four never-emitted signals (LevelRestarted, EnemyDamaged, SkillActivated, SkillDeactivated).
• Of the eleven emitted-but-unheard: keep the ones the republisher now feeds, delete the speculative ones.
• Rename EventBus.SkillUnlocked — it collides with SkillUnlockerComponent.SkillUnlocked, and every apparent subscriber is bound to the component's.
• SaveSystem.DeleteSave, SpeedRunManager.StopTimer, AchievementManager.IsAchievementUnlocked, Constants.CoinsGroup — zero callers each.
• SpeedRunManager: StopTimer carries a "save personal best" comment for logic that never existed. Best times are the Profile's job now; delete the comment and the dead method.

---

Order dependencies

0 ──> 1 ──> 2 ──┐
            3 ──┼──> 5 ──> 6 ──> 9 ──> 10 ──> 11 ──> 12 ──> 13
            4 ──┘         ↑
                     7 ───┘
                     8 ───┘

2, 3 and 4 are independent of each other after 1. 7 and 8 can be done any time after 5, and both must precede 9. 11, 12 and 13 each require 10.