diff --git a/Autoloads/EventBus.cs b/Autoloads/EventBus.cs index 7bcb112..e2deec3 100644 --- a/Autoloads/EventBus.cs +++ b/Autoloads/EventBus.cs @@ -161,6 +161,16 @@ public partial class EventBus : Node /// [Signal] public delegate void RunEndedEventHandler(); + /// + /// A death has been settled. The life is already spent and the attempt already discarded, so + /// is safe to display directly — unlike reading Lives from a + /// handler racing the one that writes it. + /// + [Signal] public delegate void AttemptFailedEventHandler(int remainingLives, bool runOver); + + public static void EmitAttemptFailed(int remainingLives, bool runOver) + => Instance?.EmitSignal(SignalName.AttemptFailed, remainingLives, runOver); + public static void EmitRunEnded() => Instance?.EmitSignal(SignalName.RunEnded); diff --git a/Autoloads/GameManager.cs b/Autoloads/GameManager.cs index 1650565..ea5b81c 100644 --- a/Autoloads/GameManager.cs +++ b/Autoloads/GameManager.cs @@ -15,9 +15,9 @@ namespace Mr.BrickAdventures.Autoloads; /// 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. +/// Progression is persisted by 's stores, which save themselves at the +/// points that matter — a First Clear and a death — so there is no separate "save the game" step to +/// forget. /// /// public partial class GameManager : Node @@ -30,7 +30,6 @@ 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; } @@ -60,10 +59,8 @@ public partial class GameManager : Node { if (Progression == null) return; - Store?.ResetAll(); Progression.StartPlaythrough(ProgressionRoot.SoleSlot); SpeedRunManager?.StartTimer(); - SaveSystem.Instance.SaveGame(); EventBus.EmitGameStarted(); GoTo(Progression.Campaign.First); @@ -73,10 +70,6 @@ public partial class GameManager : Node { 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)) { var why = Progression.PeekPlaythrough(ProgressionRoot.SoleSlot); @@ -101,10 +94,8 @@ public partial class GameManager : Node { if (Progression == null) return; - Store?.ResetAll(); Progression.StartPlaythrough(ProgressionRoot.SoleSlot); SpeedRunManager?.StartTimer(); - SaveSystem.Instance.SaveGame(); GoTo(Progression.Campaign.First); } @@ -122,9 +113,6 @@ public partial class GameManager : Node // position if this was a First Clear. Progression.FinishLevel(time); - // Still needed for lives, which move with the death path in the next commit. - SaveSystem.Instance.SaveGame(); - if (Progression.CurrentLevel is { } next && !next.Equals(finished)) GoTo(next); } diff --git a/Autoloads/ProgressionEventRepublisher.cs b/Autoloads/ProgressionEventRepublisher.cs index e60f13b..60a0042 100644 --- a/Autoloads/ProgressionEventRepublisher.cs +++ b/Autoloads/ProgressionEventRepublisher.cs @@ -51,6 +51,10 @@ public sealed class ProgressionEventRepublisher EventBus.EmitRunEnded(); break; + case AttemptFailed failed: + EventBus.EmitAttemptFailed(failed.Remaining.Value, failed.RunOver); + break; + case AchievementUnlocked unlocked: EventBus.EmitAchievementUnlocked(unlocked.Achievement.Value); break; diff --git a/objects/level/base_level.tscn b/objects/level/base_level.tscn index fa9de44..a860055 100644 --- a/objects/level/base_level.tscn +++ b/objects/level/base_level.tscn @@ -110,8 +110,6 @@ tile_set = ExtResource("11_0vx27") [node name="Killzone" parent="." unique_id=319936093 instance=ExtResource("13_0vx27")] position = Vector2(215, 324) -[connection signal="Death" from="Brick Player/HealthComponent" to="UI Layer/DeathScreen" method="OnPlayerDeath"] -[connection signal="Death" from="Brick Player/HealthComponent" to="UI Layer/GameOverScreen" method="OnPlayerDeath"] [editable path="Brick Player"] [editable path="UI Layer"] diff --git a/progression.tests/ProgressionServiceTests.cs b/progression.tests/ProgressionServiceTests.cs index 63f89a2..b8c64ee 100644 --- a/progression.tests/ProgressionServiceTests.cs +++ b/progression.tests/ProgressionServiceTests.cs @@ -230,6 +230,50 @@ public class ProgressionServiceTests Assert.False(service.CanEnter(Second)); } + [Fact] + public void A_settled_death_announces_the_outcome_so_no_screen_has_to_read_lives_itself() + { + // DeathScreen and GameOverScreen were both wired to HealthComponent.Death by scene + // connections that fire before lives are decremented, so on the last life neither saw zero. + // Announcing the settled result means the value is already correct when they hear about it. + var service = ServiceWithSkills(); + + service.HandlePlayerDeath(); + + var failed = service.DrainEvents().OfType().Single(); + Assert.Equal(2, failed.Remaining.Value); + Assert.False(failed.RunOver); + } + + [Fact] + public void Dying_on_the_last_life_announces_the_run_is_over_in_the_same_breath() + { + var service = new ProgressionService( + Profile.Empty(), new InMemoryPlaythroughStore(), new InMemoryProfileStore(), + TwoLevels(), Skills(), Lives.Of(1)); + service.StartPlaythrough(Id); + service.EnterLevel(First); + service.DrainEvents(); + + service.HandlePlayerDeath(); + + var events = service.DrainEvents(); + Assert.True(events.OfType().Single().RunOver); + Assert.Single(events.OfType()); + } + + [Fact] + public void A_repeated_death_event_announces_nothing_further() + { + var service = ServiceWithSkills(); + service.HandlePlayerDeath(); + service.DrainEvents(); + + service.HandlePlayerDeath(); + + Assert.Empty(service.DrainEvents().OfType()); + } + [Fact] public void A_collected_skill_is_granted_free_and_only_once() { diff --git a/progression/ProgressionEvent.cs b/progression/ProgressionEvent.cs index e973f6e..c94d705 100644 --- a/progression/ProgressionEvent.cs +++ b/progression/ProgressionEvent.cs @@ -10,6 +10,18 @@ public abstract record ProgressionEvent; /// Raised when a Playthrough loses its last life. public sealed record RunEnded : ProgressionEvent; +/// +/// An Attempt ended in death and has been settled: the life is already spent and the ledger already +/// discarded when this is raised. +/// +/// +/// Exists so no screen has to read Lives for itself. The death and game-over screens were wired +/// straight to HealthComponent.Death by scene connections, which fire before the life is charged +/// — so on the last life the game-over screen saw one life left and showed nothing, and the death +/// screen then saw zero and refused to reload. Carrying the settled values removes the race entirely. +/// +public sealed record AttemptFailed(Lives Remaining, bool RunOver) : ProgressionEvent; + /// /// Raised the first time an Achievement is unlocked, and only then — so a subscriber that shows a /// popup or pushes to Steam cannot fire twice however often it is told. diff --git a/progression/ProgressionService.cs b/progression/ProgressionService.cs index 59bc293..af26d2b 100644 --- a/progression/ProgressionService.cs +++ b/progression/ProgressionService.cs @@ -246,10 +246,13 @@ public sealed class ProgressionService if (!_playthrough.SettleDeath(_attempt)) return; - // The ledger was discarded, so it must stop contributing to TotalCoins. + // The ledger was discarded, so it must stop contributing to TotalCoins or to owned skills. _attempt = null; _playthroughs.Save(_playthrough); AnnounceChanges(); + + // Announced last, once lives and the discard have both landed. + _events.Raise(new AttemptFailed(_playthrough.Lives, _playthrough.IsRunOver)); } /// diff --git a/scripts/Events/LivesStateHandler.cs b/scripts/Events/LivesStateHandler.cs index 77179b1..86285a9 100644 --- a/scripts/Events/LivesStateHandler.cs +++ b/scripts/Events/LivesStateHandler.cs @@ -4,8 +4,14 @@ using Mr.BrickAdventures.Autoloads; namespace Mr.BrickAdventures.scripts.Events; /// -/// Handles player death events and updates lives in GameStateStore. +/// Charges a death to the Playthrough. /// +/// +/// One call now does what three did: spend a life, discard the Attempt's ledger, and announce the +/// settled result. It is also idempotent — overlapping hazards in a kill zone, or a hazard re-firing +/// during the two seconds before the scene reloads, cost one life. See +/// docs/adr/0005-playthrough-settles-attempts.md. +/// public partial class LivesStateHandler : Node { public override void _Ready() @@ -16,14 +22,11 @@ public partial class LivesStateHandler : Node public override void _ExitTree() { if (EventBus.Instance != null) - { EventBus.Instance.PlayerDied -= OnPlayerDied; - } } private void OnPlayerDied(Vector2 position) { - GameStateStore.Instance?.RemoveLife(); - GameStateStore.Instance?.ResetSession(); + ProgressionRoot.Instance?.HandlePlayerDeath(); } } diff --git a/scripts/UI/DeathScreen.cs b/scripts/UI/DeathScreen.cs index ecf1d0e..67071b3 100644 --- a/scripts/UI/DeathScreen.cs +++ b/scripts/UI/DeathScreen.cs @@ -1,7 +1,6 @@ using Godot; using Mr.BrickAdventures.Autoloads; using Mr.BrickAdventures.scripts.Resources; -using Mr.BrickAdventures.scripts.State; namespace Mr.BrickAdventures.scripts.UI; @@ -14,46 +13,49 @@ public partial class DeathScreen : Control [Export] public float TimeoutTime { get; set; } = 2.0f; [Export] public Godot.Collections.Array NodesToDisable { get; set; } = new(); - private GameManager GameManager => GameManager.Instance; private Timer _timer; + private bool _runOver; public override void _Ready() { - // Subscribe to lives changed event for reactive updates - EventBus.Instance.LivesChanged += OnLivesChanged; + EventBus.Instance.AttemptFailed += OnAttemptFailed; } public override void _ExitTree() { if (EventBus.Instance != null) - { - EventBus.Instance.LivesChanged -= OnLivesChanged; - } + EventBus.Instance.AttemptFailed -= OnAttemptFailed; } - private void OnLivesChanged(int lives) + /// + /// Shows the death screen. + /// + /// + /// Driven by the settled outcome rather than by a scene connection to + /// HealthComponent.Death, so the life is already spent and the count already correct. The old + /// arrangement read Lives from a handler racing the one that wrote it, then read it a second + /// time on timeout — two read paths for one fact, because neither was reliable alone. + /// + private void OnAttemptFailed(int remainingLives, bool runOver) { - // Update the label when lives change - LivesLeftLabel.Text = $" x {lives}"; - } + _runOver = runOver; + LivesLeftLabel.Text = $" x {remainingLives}"; - private void SetLabels() - { if (CurrentLevel != null) - { CurrentLevelLabel.Text = CurrentLevel.LevelName; - } - // Read current lives from store - var lives = GameStateStore.Instance?.Player.Lives ?? 0; - LivesLeftLabel.Text = $" x {lives}"; + ToggleNodes(); + Show(); + SetupTimer(); } private void SetupTimer() { - _timer = new Timer(); - _timer.WaitTime = TimeoutTime; - _timer.OneShot = true; + _timer = new Timer + { + WaitTime = TimeoutTime, + OneShot = true, + }; _timer.Timeout += OnTimeout; AddChild(_timer); _timer.Start(); @@ -69,21 +71,11 @@ public partial class DeathScreen : Control } } - public void OnPlayerDeath() - { - if (GameManager == null) return; - - ToggleNodes(); - SetLabels(); - Show(); - SetupTimer(); - } - private void OnTimeout() { - var lives = GameStateStore.Instance?.Player.Lives ?? 0; - if (lives == 0) return; + // Game over is the other screen's job; this one only retries. + if (_runOver) return; GetTree().ReloadCurrentScene(); } -} \ No newline at end of file +} diff --git a/scripts/UI/GameOverScreen.cs b/scripts/UI/GameOverScreen.cs index 1a98455..738a1ef 100644 --- a/scripts/UI/GameOverScreen.cs +++ b/scripts/UI/GameOverScreen.cs @@ -21,12 +21,17 @@ public partial class GameOverScreen : Control if (InputDeviceManager.Instance != null) InputDeviceManager.Instance.DeviceChanged += OnDeviceChanged; + + EventBus.Instance.RunEnded += OnRunEnded; } public override void _ExitTree() { if (InputDeviceManager.Instance != null) InputDeviceManager.Instance.DeviceChanged -= OnDeviceChanged; + + if (EventBus.Instance != null) + EventBus.Instance.RunEnded -= OnRunEnded; } private void OnDeviceChanged(int device) @@ -38,7 +43,6 @@ public partial class GameOverScreen : Control private void OnMainMenuClicked() { - GameStateStore.Instance?.ResetAll(); SpeedRunManager.Instance?.StopTimer(); GetTree().ChangeSceneToPacked(MainMenuScene); } @@ -48,10 +52,13 @@ public partial class GameOverScreen : Control GameManager.Instance?.RestartGame(); } - public void OnPlayerDeath() + /// + /// Shown when the run is actually over. Previously this read Lives from a scene connection that + /// fired before the life was charged, so on the last life it saw one remaining and showed nothing — + /// leaving the death screen up forever. + /// + private void OnRunEnded() { - if (GameStateStore.Instance?.Player.Lives != 0) return; - GameOverPanel.Show(); if (InputDeviceManager.Instance?.IsPointerless == true) RestartButton.GrabFocus(); diff --git a/scripts/UI/Hud.cs b/scripts/UI/Hud.cs index 47680f3..ebdcd86 100644 --- a/scripts/UI/Hud.cs +++ b/scripts/UI/Hud.cs @@ -12,29 +12,36 @@ public partial class Hud : Control [Export] public ProgressBar HealthBar { get; set; } [Export] public Label LivesLabel { get; set; } - private GameStateStore Store => GameStateStore.Instance; - public override void _Ready() { if (EventBus.Instance != null) + { EventBus.Instance.CoinsChanged += OnCoinsChanged; + EventBus.Instance.LivesChanged += OnLivesChanged; + } - SetCoinsLabel(ProgressionRoot.Instance?.TotalCoins.Value ?? 0); + var progression = ProgressionRoot.Instance; + SetCoinsLabel(progression?.TotalCoins.Value ?? 0); + SetLivesLabel(progression?.Lives.Value ?? 0); } public override void _ExitTree() { if (EventBus.Instance != null) + { EventBus.Instance.CoinsChanged -= OnCoinsChanged; + EventBus.Instance.LivesChanged -= OnLivesChanged; + } } - // Lives still polls the old store; it moves with the death path in the next commit. + // Health belongs to the component on the player, not to progression, so it stays a direct read. public override void _Process(double delta) { SetHealthBar(); - SetLivesLabel(); } + private void OnLivesChanged(int lives) => SetLivesLabel(lives); + private void OnCoinsChanged(int totalCoins) => SetCoinsLabel(totalCoins); private void SetCoinsLabel(int totalCoins) @@ -42,9 +49,9 @@ public partial class Hud : Control CoinsLabel.Text = Tr("COINS_LABEL") + ": " + totalCoins; } - private void SetLivesLabel() + private void SetLivesLabel(int lives) { - LivesLabel.Text = Tr("LIVES_LABEL") + ": " + (Store?.Player.Lives ?? 0); + LivesLabel.Text = Tr("LIVES_LABEL") + ": " + lives; } private void SetHealthBar() diff --git a/scripts/UI/MainMenu.cs b/scripts/UI/MainMenu.cs index 49345e6..e53d0be 100644 --- a/scripts/UI/MainMenu.cs +++ b/scripts/UI/MainMenu.cs @@ -15,7 +15,6 @@ public partial class MainMenu : Control [Export] public Control SettingsControl { get; set; } [Export] public Control CreditsControl { get; set; } - private SaveSystem SaveSystem => SaveSystem.Instance; private GameManager GameManager => GameManager.Instance; private UIManager UIManager => UIManager.Instance; @@ -34,7 +33,7 @@ public partial class MainMenu : Control ExitButton.Pressed += OnExitPressed; VersionLabel.Text = $"v. {ProjectSettings.GetSetting("application/config/version")}"; - ContinueButton.Disabled = !SaveSystem.CheckSaveExists(); + ContinueButton.Disabled = !(ProgressionRoot.Instance?.HasSavedPlaythrough ?? false); if (InputDeviceManager.Instance != null) InputDeviceManager.Instance.DeviceChanged += OnDeviceChanged; @@ -66,7 +65,7 @@ public partial class MainMenu : Control private void GrabFirstFocus() { - if (SaveSystem.CheckSaveExists()) + if (ProgressionRoot.Instance?.HasSavedPlaythrough ?? false) ContinueButton.GrabFocus(); else NewGameButton.GrabFocus(); diff --git a/scripts/UI/PauseMenu.cs b/scripts/UI/PauseMenu.cs index e8bb83a..928b4da 100644 --- a/scripts/UI/PauseMenu.cs +++ b/scripts/UI/PauseMenu.cs @@ -83,7 +83,7 @@ public partial class PauseMenu : Control private void OnMainMenuPressed() { GameManager.ResumeGame(); - GameStateStore.Instance?.ResetSession(); + ProgressionRoot.Instance?.AbandonAttempt(); // The run is over once you leave to the menu; otherwise the timer keeps ticking there. SpeedRunManager.Instance?.StopTimer(); GetTree().ChangeSceneToPacked(MainMenuScene);