From a1c9042652df0d4d34b6d1003c0ff2c2fee4ffb7 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Tue, 28 Jul 2026 19:57:01 +0200 Subject: [PATCH] feat: level progression moves to the new domain (slice 10b, step 1) narrowest safe step: only level identity and progression move. Coins, lives, skills and death stay entirely on GameStateStore and behave as before, so nothing is tracked in two places. GameManager addresses levels by stable LevelId via the catalogue; LevelScenes is gone, so reordering levels no longer invalidates saves. Every level entry goes through EnterLevel, which fixes LevelStarted never firing on the first level or after Continue - ghost recording and splits have been dead outside mid-campaign. GoTo announces after a frame, once the incoming scene is actually current. ChangeSceneToPacked is deferred, so the old code handed GhostEventHandler the scene being torn down. FinishLevel takes LevelTime? - GetCurrentLevelTime can legitimately be 0 and inventing a value is exactly the bad data LevelTime refuses. Continue distinguishes absent from corrupt and refuses to overwrite a corrupt save. Statistics move to the Profile. 69 tests. --- Autoloads/GameManager.cs | 137 ++++++++++++++----- Autoloads/ProgressionRoot.cs | 43 +++++- objects/game_manager.tscn | 9 +- progression.tests/ProgressionServiceTests.cs | 60 ++++++++ progression/ProgressionService.cs | 39 +++++- scripts/Events/StatisticsEventHandler.cs | 33 +++-- 6 files changed, 260 insertions(+), 61 deletions(-) diff --git a/Autoloads/GameManager.cs b/Autoloads/GameManager.cs index 15b0dcf..06ff12c 100644 --- a/Autoloads/GameManager.cs +++ b/Autoloads/GameManager.cs @@ -1,17 +1,27 @@ using Godot; -using Godot.Collections; +using Mr.BrickAdventures.Progression; using Mr.BrickAdventures.scripts.components; namespace Mr.BrickAdventures.Autoloads; /// -/// Game orchestrator - handles scene transitions and game flow. -/// State lives in GameStateStore; query it directly for reads/writes. +/// Game orchestrator: scene transitions and game flow. /// +/// +/// +/// Which Level the player is on now belongs to , addressed by a stable +/// and resolved to a scene by the level catalogue. The old +/// Array<PackedScene> LevelScenes is gone: it made a save's meaning depend on the order +/// of an inspector array, so reordering levels silently invalidated every save. +/// +/// +/// Transitional: coins, lives and skills are still owned by GameStateStore and persisted by +/// SaveSystem. Only level progression has moved. The two are kept apart deliberately — nothing +/// is tracked in both places at once. +/// +/// public partial class GameManager : Node { - [Export] public Array LevelScenes { get; set; } = []; - public PlayerController Player { get => GetPlayer(); @@ -21,6 +31,7 @@ public partial class GameManager : Node private PlayerController _player; private SpeedRunManager SpeedRunManager => SpeedRunManager.Instance; private GameStateStore Store => GameStateStore.Instance; + private ProgressionRoot Progression => ProgressionRoot.Instance; public static GameManager Instance { get; private set; } @@ -47,70 +58,122 @@ public partial class GameManager : Node public void StartNewGame() { + if (Progression == null) return; + Store?.ResetAll(); + Progression.StartPlaythrough(ProgressionRoot.SoleSlot); SpeedRunManager?.StartTimer(); - GetTree().ChangeSceneToPacked(LevelScenes[0]); SaveSystem.Instance.SaveGame(); EventBus.EmitGameStarted(); + + GoTo(Progression.Campaign.First); } public void ContinueGame() { - var save = SaveSystem.Instance; - if (!save.LoadGame()) + if (Progression == null) return; + + // Coins and skills still come from the old save; level progression from the new one. + if (!SaveSystem.Instance.LoadGame()) + GD.PrintErr("GameManager: no legacy save to continue; starting fresh."); + + if (!Progression.LoadPlaythrough(ProgressionRoot.SoleSlot)) { - GD.PrintErr("Failed to load game. Starting a new game instead."); + var why = Progression.PeekPlaythrough(ProgressionRoot.SoleSlot); + if (why == LoadOutcome.Corrupt) + { + // Deliberately not overwritten: the file is left where it is so it can be recovered. + GD.PushError("Saved progress could not be read. It has been left untouched."); + return; + } + StartNewGame(); return; } - var idx = Store?.Player.CurrentLevel ?? 0; - if (idx < LevelScenes.Count) - { - GetTree().ChangeSceneToPacked(LevelScenes[idx]); - EventBus.EmitGameContinued(); - } - else - { - GD.PrintErr("No levels unlocked to continue."); - } + SpeedRunManager?.StartTimer(); + EventBus.EmitGameContinued(); + + GoTo(Progression.CurrentLevel ?? Progression.Campaign.First); } public void RestartGame() { + if (Progression == null) return; + Store?.ResetAll(); - GetTree().ChangeSceneToPacked(LevelScenes[0]); + Progression.StartPlaythrough(ProgressionRoot.SoleSlot); + SpeedRunManager?.StartTimer(); SaveSystem.Instance.SaveGame(); + + GoTo(Progression.Campaign.First); } public void OnLevelComplete() { - if (Store == null) return; + if (Progression?.CurrentLevel is not { } finished) return; - var levelIndex = Store.Player.CurrentLevel; - Store.MarkLevelComplete(levelIndex); - Store.CommitSessionCoins(); - Store.CommitSessionSkills(); + var seconds = SpeedRunManager?.GetCurrentLevelTime() ?? 0.0; + LevelTime? time = seconds > 0 ? LevelTime.Of(seconds) : null; - var completionTime = SpeedRunManager?.GetCurrentLevelTime() ?? 0.0; - EventBus.EmitLevelCompleted(levelIndex, GetTree().CurrentScene, completionTime); + // Old path: coins and skills earned this attempt. + Store?.CommitSessionCoins(); + Store?.CommitSessionSkills(); - Store.ResetSession(); - TryToGoToNextLevel(); + EventBus.EmitLevelCompleted(IndexOf(finished), GetTree().CurrentScene, seconds); + + // New path: banks, marks cleared, and advances campaign position if this was a first clear. + Progression.FinishLevel(time); + Store?.ResetSession(); SaveSystem.Instance.SaveGame(); + + if (Progression.CurrentLevel is { } next && !next.Equals(finished)) + GoTo(next); } - public void TryToGoToNextLevel() + /// + /// Loads a Level's scene, then announces the entry once the new scene is actually current. + /// + /// + /// The announcement is deliberately after a frame. ChangeSceneToPacked is deferred, so at + /// the moment it returns CurrentScene is still the outgoing scene — which is why the old + /// code handed GhostEventHandler the scene being torn down and ghosts were spawned into it. + /// + private async void GoTo(LevelId level) { - if (Store == null) return; - - var next = Store.Player.CurrentLevel + 1; - if (next < LevelScenes.Count && Store.IsLevelUnlocked(next)) + if (!Progression.CanEnter(level)) { - Store.Player.CurrentLevel = next; - GetTree().ChangeSceneToPacked(LevelScenes[next]); - EventBus.EmitLevelStarted(next, GetTree().CurrentScene); + GD.PushError($"Level '{level}' is not reachable in this playthrough."); + return; } + + var scene = Progression.Catalogue.SceneFor(level); + if (scene == null) + { + GD.PushError($"Level '{level}' has no loadable scene."); + return; + } + + GetTree().ChangeSceneToPacked(scene); + await ToSignal(GetTree(), SceneTree.SignalName.ProcessFrame); + + Progression.EnterLevel(level); + } + + /// + /// Position in the catalogue, for the signals that are still index-based because GhostManager + /// names its files ghost_level_{index}.json. + /// + private int IndexOf(LevelId level) + { + var levels = Progression?.Catalogue?.Levels; + if (levels == null) return -1; + + for (var i = 0; i < levels.Count; i++) + if (levels[i]?.Id == level.Value) + return i; + + return -1; } public void PauseGame() diff --git a/Autoloads/ProgressionRoot.cs b/Autoloads/ProgressionRoot.cs index f9bfc95..1aa9ccc 100644 --- a/Autoloads/ProgressionRoot.cs +++ b/Autoloads/ProgressionRoot.cs @@ -1,3 +1,4 @@ +using System; using Godot; using Mr.BrickAdventures.Progression; using Mr.BrickAdventures.Progression.Adapters; @@ -23,6 +24,13 @@ public partial class ProgressionRoot : Node { public static ProgressionRoot Instance { get; private set; } + /// + /// The one slot the game currently uses. Phase B replaces this with a picker over several + /// Playthroughs; until then a fixed id keeps the file name stable across runs. + /// + public static readonly PlaythroughId SoleSlot = + PlaythroughId.Of(new Guid("a17e0000-0000-4000-8000-000000000001")); + [Export] public LevelCatalogue Catalogue { get; set; } /// Lives a new Playthrough starts with. A tuning value, so it is authored here rather than baked into the domain. @@ -109,6 +117,39 @@ public partial class ProgressionRoot : Node public void StartPlaythrough(PlaythroughId id) => Run(() => _service.StartPlaythrough(id)); + public bool LoadPlaythrough(PlaythroughId id) + { + if (_service == null) return false; + + var loaded = _service.LoadPlaythrough(id); + Publish(); + return loaded; + } + + public LoadOutcome PeekPlaythrough(PlaythroughId id) => + _service?.PeekPlaythrough(id) ?? LoadOutcome.Absent; + + public void RecordStat(StatName stat, int amount = 1) => _service?.RecordStat(stat, amount); + + #endregion + + #region Reads + + public bool IsPlaying => _service?.IsPlaying ?? false; + public bool CanEnter(LevelId level) => _service?.CanEnter(level) ?? false; + public LevelId? CurrentLevel => _service?.CurrentLevel; + public Coins TotalCoins => _service?.TotalCoins ?? Coins.Zero; + public Lives Lives => _service?.Lives ?? Lives.Of(0); + public bool IsRunOver => _service?.IsRunOver ?? false; + public SkillLevel? LevelOf(SkillId skill) => _service?.LevelOf(skill); + public bool IsOwned(SkillId skill) => _service?.IsOwned(skill) ?? false; + public bool CanAfford(SkillId skill) => _service?.CanAfford(skill) ?? false; + public bool HasCleared(LevelId level) => _service?.HasCleared(level) ?? false; + + #endregion + + #region Operations, continued + public bool EnterLevel(LevelId level) { if (_service == null) return false; @@ -122,7 +163,7 @@ public partial class ProgressionRoot : Node public void HandlePlayerDeath() => Run(() => _service.HandlePlayerDeath()); - public void FinishLevel(LevelTime time) => Run(() => _service.FinishLevel(time)); + public void FinishLevel(LevelTime? time) => Run(() => _service.FinishLevel(time)); public PurchaseOutcome PurchaseSkill(SkillId skill) { diff --git a/objects/game_manager.tscn b/objects/game_manager.tscn index 7867bcd..d32ccda 100644 --- a/objects/game_manager.tscn +++ b/objects/game_manager.tscn @@ -1,17 +1,10 @@ -[gd_scene format=3 uid="uid://b4eifkc31jsun"] +[gd_scene load_steps=3 format=3 uid="uid://b4eifkc31jsun"] [ext_resource type="Script" uid="uid://c6eoi3ymefc0x" path="res://Autoloads/GameManager.cs" id="1_t2tr6"] -[ext_resource type="PackedScene" uid="uid://dwr3avefuwnpy" path="res://scenes/level_tutorial_1.tscn" id="2_7rb6w"] -[ext_resource type="PackedScene" uid="uid://bol7g83v2accs" path="res://scenes/level_village_1.tscn" id="2_bentb"] -[ext_resource type="PackedScene" uid="uid://chqb11pfoqmeb" path="res://scenes/level_village_2.tscn" id="3_ajlkg"] -[ext_resource type="PackedScene" uid="uid://h60obxmju6mo" path="res://scenes/level_village_3.tscn" id="4_se5tb"] -[ext_resource type="PackedScene" uid="uid://bhad760x3vvco" path="res://scenes/level_village_4.tscn" id="5_mnosh"] -[ext_resource type="PackedScene" uid="uid://bbwef3n2gjkt8" path="res://scenes/level_village_5.tscn" id="6_7rb6w"] [ext_resource type="Script" uid="uid://chrhjch4ymfvr" path="res://scripts/Screenshot.cs" id="6_bbtu1"] [node name="GameManager" type="Node" unique_id=2076336659] script = ExtResource("1_t2tr6") -LevelScenes = Array[PackedScene]([ExtResource("2_7rb6w"), ExtResource("2_bentb"), ExtResource("3_ajlkg"), ExtResource("4_se5tb"), ExtResource("5_mnosh"), ExtResource("6_7rb6w")]) [node name="Screenshot" type="Node" parent="." unique_id=812485936] script = ExtResource("6_bbtu1") diff --git a/progression.tests/ProgressionServiceTests.cs b/progression.tests/ProgressionServiceTests.cs index 6e6064f..9cfacd6 100644 --- a/progression.tests/ProgressionServiceTests.cs +++ b/progression.tests/ProgressionServiceTests.cs @@ -170,6 +170,66 @@ public class ProgressionServiceTests Assert.Equal(LevelTime.Of(30.0), profileStore.Load().Value!.BestTimeFor(First)); } + [Fact] + public void Resuming_a_saved_playthrough_restores_where_it_had_got_to() + { + var store = new InMemoryPlaythroughStore(); + var first = new ProgressionService( + Profile.Empty(), store, new InMemoryProfileStore(), TwoLevels(), Skills(), Lives.Of(3)); + first.StartPlaythrough(Id); + first.EnterLevel(First); + first.FinishLevel(LevelTime.Of(20.0)); + + // A fresh service, as though the game had been restarted. + var resumed = new ProgressionService( + Profile.Empty(), store, new InMemoryProfileStore(), TwoLevels(), Skills(), Lives.Of(3)); + + Assert.True(resumed.LoadPlaythrough(Id)); + Assert.Equal(Second, resumed.CurrentLevel); + Assert.True(resumed.HasCleared(First)); + Assert.True(resumed.CanEnter(First)); + Assert.True(resumed.CanEnter(Second)); + } + + [Fact] + public void Resuming_when_there_is_no_save_leaves_the_service_not_playing() + { + var service = new ProgressionService( + Profile.Empty(), new InMemoryPlaythroughStore(), new InMemoryProfileStore(), + TwoLevels(), Skills(), Lives.Of(3)); + + Assert.False(service.LoadPlaythrough(Id)); + Assert.False(service.IsPlaying); + } + + [Fact] + public void Finishing_without_a_measured_time_still_settles_the_level_but_records_no_best_time() + { + // GetCurrentLevelTime returns 0 when the speedrun timer was never started. Passing null says + // so honestly rather than banking a zero-second record. + var profileStore = new InMemoryProfileStore(); + var service = new ProgressionService( + Profile.Empty(), new InMemoryPlaythroughStore(), profileStore, + TwoLevels(), Skills(), Lives.Of(3)); + service.StartPlaythrough(Id); + service.EnterLevel(First); + + service.FinishLevel(null); + + Assert.True(service.HasCleared(First)); + Assert.Equal(Second, service.CurrentLevel); + Assert.Null(profileStore.Load().Value!.BestTimeFor(First)); + } + + [Fact] + public void An_unreachable_level_cannot_be_entered_even_though_it_is_in_the_campaign() + { + var service = Service(); + + Assert.True(service.CanEnter(First)); + Assert.False(service.CanEnter(Second)); + } + [Fact] public void Picking_up_a_coin_announces_the_new_total_so_the_hud_never_polls() { diff --git a/progression/ProgressionService.cs b/progression/ProgressionService.cs index 6e33deb..1e76a86 100644 --- a/progression/ProgressionService.cs +++ b/progression/ProgressionService.cs @@ -89,6 +89,14 @@ public sealed class ProgressionService public bool HasCleared(LevelId level) => _playthrough?.HasCleared(level) ?? false; + /// + /// Whether this Level can be entered: the Campaign's first, anything cleared, or whatever follows + /// something cleared. Separate from so a caller can check before starting + /// a scene change it would then have to undo. + /// + public bool CanEnter(LevelId level) => + _playthrough is not null && _playthrough.ReachableLevels(_campaign).Contains(level); + #endregion public void StartPlaythrough(PlaythroughId id) @@ -99,6 +107,28 @@ public sealed class ProgressionService AnnounceChanges(); } + /// + /// Resumes a stored Playthrough. Returns false when there is nothing to resume, leaving the + /// service exactly as it was — a failed resume must not clear an active Playthrough, and it must + /// not be mistaken for "start a new game", which is what overwrote saves before. + /// + public bool LoadPlaythrough(PlaythroughId id) + { + var result = _playthroughs.Load(id); + if (result.Outcome != LoadOutcome.Loaded) return false; + + _playthrough = result.Value; + _attempt = null; + AnnounceChanges(); + return true; + } + + /// Why a resume failed, for a caller that wants to tell absent from corrupt. + public LoadOutcome PeekPlaythrough(PlaythroughId id) => _playthroughs.Load(id).Outcome; + + /// Adds to a lifetime statistic on the Profile. + public void RecordStat(StatName stat, int amount = 1) => Profile.Record(stat, amount); + /// /// Begins an Attempt at and announces it. Does not touch campaign /// position — that advances only on a First Clear, so a Replay leaves it alone. @@ -168,7 +198,12 @@ public sealed class ProgressionService /// (ADR-0003), while banking and advancing happen only on a First Clear — which the Playthrough /// decides for itself from the Attempt's Level. /// - public void FinishLevel(LevelTime time) + /// + /// The completion time, or null when none was measured. Null still settles the Level — it just + /// records no Best Time. That beats inventing a value: a zero-second time is exactly the bad data + /// exists to refuse. + /// + public void FinishLevel(LevelTime? time) { if (_playthrough is null || _attempt is null) return; if (_attempt.Outcome != AttemptOutcome.InProgress) return; @@ -179,7 +214,7 @@ public sealed class ProgressionService // ClearedLevels.Count, which counts distinct Levels. Conflating the two is what made the old // levels_completed statistic disagree with CompletedLevels. Profile.Record(StatName.LevelsCompleted, 1); - Profile.RecordTime(level, time); + if (time is { } measured) Profile.RecordTime(level, measured); var wasFirstClear = _playthrough.SettleCompletion(_attempt, _campaign); _attempt = null; diff --git a/scripts/Events/StatisticsEventHandler.cs b/scripts/Events/StatisticsEventHandler.cs index 00d196d..427a66a 100644 --- a/scripts/Events/StatisticsEventHandler.cs +++ b/scripts/Events/StatisticsEventHandler.cs @@ -1,12 +1,17 @@ using Godot; using Mr.BrickAdventures.Autoloads; -using Mr.BrickAdventures.scripts.State; +using Mr.BrickAdventures.Progression; namespace Mr.BrickAdventures.scripts.Events; /// -/// Handles game events and updates statistics accordingly. +/// Records lifetime statistics on the Profile. /// +/// +/// Statistics moved to the Profile alongside level progression, because they outlive a Playthrough — +/// see docs/adr/0001-three-progression-scopes.md. is a closed set, so a typo is +/// now a compile error rather than a silently invented sixth statistic. +/// public partial class StatisticsEventHandler : Node { public override void _Ready() @@ -14,7 +19,6 @@ public partial class StatisticsEventHandler : Node EventBus.Instance.CoinCollected += OnCoinCollected; EventBus.Instance.EnemyDefeated += OnEnemyDefeated; EventBus.Instance.PlayerDied += OnPlayerDied; - EventBus.Instance.LevelCompleted += OnLevelCompleted; EventBus.Instance.ChildRescued += OnChildRescued; } @@ -25,22 +29,25 @@ public partial class StatisticsEventHandler : Node EventBus.Instance.CoinCollected -= OnCoinCollected; EventBus.Instance.EnemyDefeated -= OnEnemyDefeated; EventBus.Instance.PlayerDied -= OnPlayerDied; - EventBus.Instance.LevelCompleted -= OnLevelCompleted; EventBus.Instance.ChildRescued -= OnChildRescued; } + private static void Record(StatName stat, int amount = 1) => + ProgressionRoot.Instance?.RecordStat(stat, amount); + + // Record rejects non-positive amounts, so a zero-value coin is skipped rather than throwing. private void OnCoinCollected(int amount, Vector2 position) - => GameStateStore.Instance?.IncrementStat(StatNames.CoinsCollected, amount); + { + if (amount > 0) Record(StatName.CoinsCollected, amount); + } - private void OnEnemyDefeated(Node enemy, Vector2 position) - => GameStateStore.Instance?.IncrementStat(StatNames.EnemiesDefeated); + private void OnEnemyDefeated(Node enemy, Vector2 position) => Record(StatName.EnemiesDefeated); - private void OnPlayerDied(Vector2 position) - => GameStateStore.Instance?.IncrementStat(StatNames.Deaths); + private void OnPlayerDied(Vector2 position) => Record(StatName.Deaths); - private void OnLevelCompleted(int levelIndex, Node currentScene, double completionTime) - => GameStateStore.Instance?.IncrementStat(StatNames.LevelsCompleted); + private void OnChildRescued(Vector2 position) => Record(StatName.ChildrenRescued); - private void OnChildRescued(Vector2 position) - => GameStateStore.Instance?.IncrementStat(StatNames.ChildrenRescued); + // LevelsCompleted is recorded by ProgressionService.FinishLevel, not here. It counts completions + // including Replays — deliberately a different fact from ClearedLevels.Count — and keeping it next + // to the settlement is what stops the two drifting apart the way the old pair did. }