From 360f73ba249264ff3dc3fd14bf05becd4bcb3f74 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Tue, 28 Jul 2026 20:13:24 +0200 Subject: [PATCH] feat: coins and skills move to the new domain (commit B of the swap) atomic pair - purchasing couples them, so neither moves alone. SkillData.Level is gone. It was an [Export] on a cached Resource: one instance per process, so the level never persisted, leaked between save slots, and made "unowned" indistinguishable from "level 1". Level now comes from the Playthrough and is passed to ISkill.Initialize. SkillManager reconciles from SkillSetChanged instead of being told by each caller. The old shape was "record it, then tell SkillManager" as two calls at four sites; AddSkill's early return when no player is registered left the skill owned but inert. Reconciliation also tracks the level each component was built at, so an upgrade rebuilds it rather than being patched in from outside. SkillUnlockerComponent collapses to one TryBuy - unlock and upgrade were two methods each doing affordability, debit, level mutation and manager notification in order. The guard three components copy-pasted around Upgrades[Level - 1] - and which BrickArmor omitted - now lives once in SkillComponentBase. Hud reads coins from CoinsChanged instead of rebuilding translated strings every frame. Lives still polls; it moves with the death path. --- Autoloads/GameManager.cs | 10 +- Autoloads/SkillManager.cs | 119 ++++++++++-------- scripts/Events/CoinStateHandler.cs | 14 ++- scripts/Events/SkillCollectHandler.cs | 24 ++-- scripts/Resources/SkillData.cs | 4 +- scripts/UI/Hud.cs | 22 +++- scripts/UI/Marketplace.cs | 78 +++++------- scripts/UI/MarketplaceButton.cs | 76 ++++++----- .../components/BrickArmorSkillComponent.cs | 8 +- .../components/BrickShieldSkillComponent.cs | 4 +- scripts/components/BrickThrowComponent.cs | 9 +- .../components/GroundPoundSkillComponent.cs | 4 +- scripts/components/MagneticSkillComponent.cs | 9 +- scripts/components/SkillComponentBase.cs | 26 +++- scripts/components/SkillUnlockerComponent.cs | 64 ++++------ .../components/XRayVisionSkillComponent.cs | 4 +- scripts/interfaces/ISkill.cs | 14 ++- 17 files changed, 251 insertions(+), 238 deletions(-) diff --git a/Autoloads/GameManager.cs b/Autoloads/GameManager.cs index 06ff12c..1650565 100644 --- a/Autoloads/GameManager.cs +++ b/Autoloads/GameManager.cs @@ -116,15 +116,13 @@ public partial class GameManager : Node var seconds = SpeedRunManager?.GetCurrentLevelTime() ?? 0.0; LevelTime? time = seconds > 0 ? LevelTime.Of(seconds) : null; - // Old path: coins and skills earned this attempt. - Store?.CommitSessionCoins(); - Store?.CommitSessionSkills(); - EventBus.EmitLevelCompleted(IndexOf(finished), GetTree().CurrentScene, seconds); - // New path: banks, marks cleared, and advances campaign position if this was a first clear. + // Banks the attempt's coins and skills, marks the level cleared, and advances campaign + // position if this was a First Clear. Progression.FinishLevel(time); - Store?.ResetSession(); + + // 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)) diff --git a/Autoloads/SkillManager.cs b/Autoloads/SkillManager.cs index 82bf84b..efa1d59 100644 --- a/Autoloads/SkillManager.cs +++ b/Autoloads/SkillManager.cs @@ -1,22 +1,40 @@ -using System.Collections.Generic; using System.Linq; using Godot; using Godot.Collections; +using Mr.BrickAdventures.Progression; using Mr.BrickAdventures.scripts.components; using Mr.BrickAdventures.scripts.interfaces; using Mr.BrickAdventures.scripts.Resources; namespace Mr.BrickAdventures.Autoloads; +/// +/// Owns the live skill components attached to the player, and keeps them matching what the Playthrough +/// says is owned. +/// +/// +/// +/// Reconciles from rather than being told by each caller. The old +/// arrangement had four call sites doing "record it in the store, then tell SkillManager" as two +/// separate calls — anything between them, including AddSkill's own early return when no player +/// is registered, left a skill owned but inert. +/// +/// +/// Tracks the level each component was built at, so an upgrade rebuilds it. Previously +/// AddSkill returned early for an already-active skill, and the upgrade was applied by the +/// purchase code reaching into the component instead. +/// +/// public partial class SkillManager : Node { private PlayerController _player; - private GameStateStore Store => GameStateStore.Instance; [Export] public Array AvailableSkills { get; set; } = []; public Dictionary ActiveComponents { get; private set; } = new(); + private readonly System.Collections.Generic.Dictionary _activeLevels = new(); + public static SkillManager Instance { get; private set; } [Signal] @@ -27,10 +45,14 @@ public partial class SkillManager : Node public override void _Ready() { Instance = this; + EventBus.Instance.SkillSetChanged += Reconcile; } public override void _ExitTree() { + if (EventBus.Instance != null) + EventBus.Instance.SkillSetChanged -= Reconcile; + if (Instance == this) Instance = null; } @@ -41,24 +63,17 @@ public partial class SkillManager : Node { if (_player == player) return; - // If a player is already registered (e.g., from a previous scene), unregister it first. if (_player != null && IsInstanceValid(_player)) - { UnregisterPlayer(); - } _player = player; if (_player != null) { - // Automatically unregister when the player node is removed from the scene. _player.TreeExiting += UnregisterPlayer; - ApplyUnlockedSkills(); + Reconcile(); } } - /// - /// Cleans up skills and references related to the current player. - /// private void UnregisterPlayer() { if (_player != null && IsInstanceValid(_player)) @@ -69,9 +84,37 @@ public partial class SkillManager : Node _player = null; } - public void AddSkill(SkillData skillData) + /// + /// Brings the live components in line with what is owned: adds what is missing, rebuilds anything + /// whose level changed, removes what is no longer owned. + /// + public void Reconcile() + { + if (_player == null || !IsInstanceValid(_player)) return; + + var progression = ProgressionRoot.Instance; + if (progression == null) return; + + foreach (var skill in AvailableSkills) + { + if (skill == null || string.IsNullOrWhiteSpace(skill.Name)) continue; + + var owned = progression.LevelOf(SkillId.Of(skill.Name))?.Value; + var active = _activeLevels.TryGetValue(skill.Name, out var at) ? at : (int?)null; + + if (owned == active) continue; + + if (active is not null) RemoveSkill(skill.Name); + if (owned is not null) AddSkill(skill, owned.Value); + } + } + + /// Kept for the marketplace's activate/deactivate toggle, which is a display concern. + public void AddSkill(SkillData skillData) => + AddSkill(skillData, ProgressionRoot.Instance?.LevelOf(SkillId.Of(skillData.Name))?.Value ?? 1); + + private void AddSkill(SkillData skillData, int level) { - // Ensure a valid player is registered before adding a skill. if (_player == null || !IsInstanceValid(_player)) { GD.Print("SkillManager: Player not available to add skill."); @@ -83,10 +126,9 @@ public partial class SkillManager : Node if (skillData.Type == SkillType.Throw) { - // Remove any other active throw skill before adding the new one foreach (var sd in AvailableSkills) { - if (sd != skillData && sd.Type == SkillType.Throw) + if (sd != skillData && sd?.Type == SkillType.Throw) RemoveSkill(sd.Name); } } @@ -94,8 +136,7 @@ public partial class SkillManager : Node var instance = skillData.Node.Instantiate(); if (instance is ISkill skill) { - // Initialize the skill with the registered player instance. - skill.Initialize(_player, skillData); + skill.Initialize(_player, skillData, level); skill.Activate(); } else @@ -105,31 +146,27 @@ public partial class SkillManager : Node return; } - // Add the skill node as a child of the player. _player.AddChild(instance); ActiveComponents[skillData.Name] = instance; + _activeLevels[skillData.Name] = level; if (instance is BrickThrowComponent btc) - { EmitSignalActiveThrowSkillChanged(btc); - } } public void RemoveSkill(string skillName) { + _activeLevels.Remove(skillName); + if (!ActiveComponents.TryGetValue(skillName, out var component)) return; if (component.AsGodotObject() is BrickThrowComponent) - { EmitSignalActiveThrowSkillChanged(null); - } var inst = (Node)component; if (inst is ISkill skill) - { skill.Deactivate(); - } if (IsInstanceValid(inst)) inst.QueueFree(); @@ -141,56 +178,30 @@ public partial class SkillManager : Node private void RemoveAllActiveSkills() { - // Create a copy of keys to avoid modification during iteration - var keys = ActiveComponents.Keys.ToArray(); - var skillNames = keys.Select(key => key.ToString()).ToList(); - - foreach (var skillName in skillNames) - { + foreach (var skillName in ActiveComponents.Keys.Select(k => k.ToString()).ToList()) RemoveSkill(skillName); - } - } - - public void ApplyUnlockedSkills() - { - if (_player == null || !IsInstanceValid(_player)) return; - if (Store == null) return; - - foreach (var sd in AvailableSkills) - { - if (Store.IsSkillUnlocked(sd)) - CallDeferred(MethodName.AddSkill, sd); - else - RemoveSkill(sd.Name); - } } public SkillData GetSkillByName(string skillName) { foreach (var sd in AvailableSkills) - if (sd.Name == skillName) return sd; + if (sd?.Name == skillName) return sd; return null; } - public bool IsSkillActive(SkillData skill) - { - return skill != null && ActiveComponents.ContainsKey(skill.Name); - } + public bool IsSkillActive(SkillData skill) => + skill != null && ActiveComponents.ContainsKey(skill.Name); public void ActivateSkill(SkillData skill) { if (!ActiveComponents.ContainsKey(skill.Name)) - { AddSkill(skill); - } } public void DeactivateSkill(SkillData skill) { if (ActiveComponents.ContainsKey(skill.Name)) - { RemoveSkill(skill.Name); - } } public void ToggleSkillActivation(SkillData skill) @@ -202,4 +213,4 @@ public partial class SkillManager : Node else ActivateSkill(skill); } -} \ No newline at end of file +} diff --git a/scripts/Events/CoinStateHandler.cs b/scripts/Events/CoinStateHandler.cs index dc6b9b1..6b89769 100644 --- a/scripts/Events/CoinStateHandler.cs +++ b/scripts/Events/CoinStateHandler.cs @@ -1,12 +1,16 @@ using Godot; using Mr.BrickAdventures.Autoloads; +using Mr.BrickAdventures.Progression; namespace Mr.BrickAdventures.scripts.Events; /// -/// Handles coin collection events and updates the GameStateStore. -/// Replaces the manual coin logic in GameManager. +/// Records collected coins against the current Attempt. /// +/// +/// Coins are banked only by a First Clear, so dying loses what was found this attempt — including +/// anything spent from it. See docs/adr/0002-attempt-is-a-ledger.md. +/// public partial class CoinStateHandler : Node { public override void _Ready() @@ -17,13 +21,13 @@ public partial class CoinStateHandler : Node public override void _ExitTree() { if (EventBus.Instance != null) - { EventBus.Instance.CoinCollected -= OnCoinCollected; - } } private void OnCoinCollected(int amount, Vector2 position) { - GameStateStore.Instance?.AddSessionCoins(amount); + if (amount <= 0) return; + + ProgressionRoot.Instance?.RecordCoinsFound(Coins.Of(amount)); } } diff --git a/scripts/Events/SkillCollectHandler.cs b/scripts/Events/SkillCollectHandler.cs index 27728be..1c2f8c9 100644 --- a/scripts/Events/SkillCollectHandler.cs +++ b/scripts/Events/SkillCollectHandler.cs @@ -1,17 +1,20 @@ using Godot; using Mr.BrickAdventures.Autoloads; +using Mr.BrickAdventures.Progression; using Mr.BrickAdventures.scripts.Resources; namespace Mr.BrickAdventures.scripts.Events; /// -/// Handles skill collection events and unlocks skills via GameStateStore. -/// Skills are immediately activated but only persisted on level complete. +/// Grants a skill picked up in a level. /// +/// +/// Only records that it is owned. Attaching the live component is SkillManager's job, driven by +/// SkillSetChanged — previously this did both as two separate calls, and the second could silently do +/// nothing if no player was registered yet, leaving the skill owned but inert. +/// public partial class SkillCollectHandler : Node { - private SkillManager SkillManager => SkillManager.Instance; - public override void _Ready() { EventBus.Instance.SkillCollected += OnSkillCollected; @@ -20,22 +23,13 @@ public partial class SkillCollectHandler : Node public override void _ExitTree() { if (EventBus.Instance != null) - { EventBus.Instance.SkillCollected -= OnSkillCollected; - } } private void OnSkillCollected(SkillData skill, Vector2 position) { - if (skill == null) return; + if (skill == null || string.IsNullOrWhiteSpace(skill.Name)) return; - // Unlock in session (will be committed on level complete, lost on death) - GameStateStore.Instance?.UnlockSkillInSession(skill); - - // Immediately activate the skill for the player - SkillManager?.AddSkill(skill); - - // Emit skill unlocked event for UI/achievements - EventBus.EmitSkillUnlocked(skill.Name, skill.Level); + ProgressionRoot.Instance?.GrantSkill(SkillId.Of(skill.Name)); } } diff --git a/scripts/Resources/SkillData.cs b/scripts/Resources/SkillData.cs index 660aba0..9e67e0f 100644 --- a/scripts/Resources/SkillData.cs +++ b/scripts/Resources/SkillData.cs @@ -10,10 +10,12 @@ public partial class SkillData : Resource [Export] public string Name { get; set; } = "New Skill"; [Export] public string Description { get; set; } = "New Skill"; [Export] public Texture2D Icon { get; set; } - [Export] public int Level { get; set; } = 1; [Export] public SkillType Type { get; set; } = SkillType.Throw; [Export] public PackedScene Node { get; set; } [Export] public Array Upgrades { get; set; } = []; + /// + /// Highest level the authored upgrade table defines. Zero means this skill cannot be bought. + /// public int MaxLevel => Upgrades.Count; } \ No newline at end of file diff --git a/scripts/UI/Hud.cs b/scripts/UI/Hud.cs index 11c627d..47680f3 100644 --- a/scripts/UI/Hud.cs +++ b/scripts/UI/Hud.cs @@ -14,16 +14,32 @@ public partial class Hud : Control private GameStateStore Store => GameStateStore.Instance; + public override void _Ready() + { + if (EventBus.Instance != null) + EventBus.Instance.CoinsChanged += OnCoinsChanged; + + SetCoinsLabel(ProgressionRoot.Instance?.TotalCoins.Value ?? 0); + } + + public override void _ExitTree() + { + if (EventBus.Instance != null) + EventBus.Instance.CoinsChanged -= OnCoinsChanged; + } + + // Lives still polls the old store; it moves with the death path in the next commit. public override void _Process(double delta) { SetHealthBar(); SetLivesLabel(); - SetCoinsLabel(); } - private void SetCoinsLabel() + private void OnCoinsChanged(int totalCoins) => SetCoinsLabel(totalCoins); + + private void SetCoinsLabel(int totalCoins) { - CoinsLabel.Text = Tr("COINS_LABEL") + ": " + (Store?.GetTotalCoins() ?? 0); + CoinsLabel.Text = Tr("COINS_LABEL") + ": " + totalCoins; } private void SetLivesLabel() diff --git a/scripts/UI/Marketplace.cs b/scripts/UI/Marketplace.cs index f196707..1b89a29 100644 --- a/scripts/UI/Marketplace.cs +++ b/scripts/UI/Marketplace.cs @@ -3,6 +3,7 @@ using Godot; using Godot.Collections; using Mr.BrickAdventures.Autoloads; using Mr.BrickAdventures.scripts.components; +using Mr.BrickAdventures.Progression; using Mr.BrickAdventures.scripts.Resources; namespace Mr.BrickAdventures.scripts.UI; @@ -18,7 +19,7 @@ public partial class Marketplace : Control [Export] public PackedScene MarketplaceButtonScene { get; set; } [Export] public PackedScene SkillButtonScene { get; set; } - private GameStateStore Store => GameStateStore.Instance; + private static ProgressionRoot Progression => ProgressionRoot.Instance; private SkillManager SkillManager => SkillManager.Instance; private readonly List