refactor: fix bugs and improve architecture

- Fix double-execution bug in LevelStateHandler (coins/skills were committed twice per level)
- Fix DamageComponent to track multiple targets via HashSet instead of single node
- Fix HealthComponent: update health immediately, decouple from PlayerController via signal wiring in PlayerController
- Remove dead loop in SkillManager.RemoveSkill; simplify O(n²) throw skill loop
- Replace GetNode<T>(path) with .Instance across managers; add Instance to StatisticsManager/SpeedRunManager
- GameManager.GetPlayer() now uses EventBus.PlayerSpawned instead of scanning all scene nodes
- UIManager.UiStack: remove [Export], use private List<Control>
- PlayerState: extract DefaultLives constant, simplify CreateDefault()
This commit is contained in:
2026-03-19 01:41:14 +01:00
parent 427bef6509
commit 321905e68e
11 changed files with 85 additions and 161 deletions

View File

@@ -1,4 +1,3 @@
using System.Collections.Generic;
using Godot; using Godot;
using Godot.Collections; using Godot.Collections;
using Mr.BrickAdventures.scripts.components; using Mr.BrickAdventures.scripts.components;
@@ -21,7 +20,6 @@ public partial class GameManager : Node
private set => _player = value; private set => _player = value;
} }
private List<Node> _sceneNodes = [];
private PlayerController _player; private PlayerController _player;
private SpeedRunManager _speedRunManager; private SpeedRunManager _speedRunManager;
@@ -32,38 +30,24 @@ public partial class GameManager : Node
public static GameManager Instance { get; private set; } public static GameManager Instance { get; private set; }
public override void _EnterTree() public override void _Ready()
{ {
GetTree().NodeAdded += OnNodeAdded; Instance = this;
GetTree().NodeRemoved += OnNodeRemoved; _speedRunManager = SpeedRunManager.Instance;
EventBus.Instance.PlayerSpawned += OnPlayerSpawned;
} }
public override void _ExitTree() public override void _ExitTree()
{ {
if (Instance == this) Instance = null; if (Instance == this) Instance = null;
GetTree().NodeAdded -= OnNodeAdded; if (EventBus.Instance != null)
GetTree().NodeRemoved -= OnNodeRemoved; EventBus.Instance.PlayerSpawned -= OnPlayerSpawned;
_sceneNodes.Clear();
} }
public override void _Ready() private void OnPlayerSpawned(PlayerController player)
{ {
Instance = this; _player = player;
_speedRunManager = GetNode<SpeedRunManager>(Constants.SpeedRunManagerPath); player.TreeExiting += () => { if (_player == player) _player = null; };
}
private void OnNodeAdded(Node node)
{
_sceneNodes.Add(node);
}
private void OnNodeRemoved(Node node)
{
_sceneNodes.Remove(node);
if (node == _player)
{
_player = null;
}
} }
#region Coin Operations #region Coin Operations
@@ -187,7 +171,7 @@ public partial class GameManager : Node
{ {
Store?.ResetAll(); Store?.ResetAll();
GetTree().ChangeSceneToPacked(LevelScenes[0]); GetTree().ChangeSceneToPacked(LevelScenes[0]);
GetNode<SaveSystem>(Constants.SaveSystemPath).SaveGame(); SaveSystem.Instance.SaveGame();
} }
public void QuitGame() => GetTree().Quit(); public void QuitGame() => GetTree().Quit();
@@ -209,13 +193,13 @@ public partial class GameManager : Node
Store?.ResetAll(); Store?.ResetAll();
_speedRunManager?.StartTimer(); _speedRunManager?.StartTimer();
GetTree().ChangeSceneToPacked(LevelScenes[0]); GetTree().ChangeSceneToPacked(LevelScenes[0]);
GetNode<SaveSystem>(Constants.SaveSystemPath).SaveGame(); SaveSystem.Instance.SaveGame();
EventBus.EmitGameStarted(); EventBus.EmitGameStarted();
} }
public void ContinueGame() public void ContinueGame()
{ {
var save = GetNode<SaveSystem>(Constants.SaveSystemPath); var save = SaveSystem.Instance;
if (!save.LoadGame()) if (!save.LoadGame())
{ {
GD.PrintErr("Failed to load game. Starting a new game instead."); GD.PrintErr("Failed to load game. Starting a new game instead.");
@@ -249,7 +233,7 @@ public partial class GameManager : Node
Store.ResetSession(); Store.ResetSession();
TryToGoToNextLevel(); TryToGoToNextLevel();
GetNode<SaveSystem>(Constants.SaveSystemPath).SaveGame(); SaveSystem.Instance.SaveGame();
} }
#endregion #endregion
@@ -259,18 +243,7 @@ public partial class GameManager : Node
public PlayerController GetPlayer() public PlayerController GetPlayer()
{ {
if (_player != null && IsInstanceValid(_player)) return _player; if (_player != null && IsInstanceValid(_player)) return _player;
_player = null; _player = null;
foreach (var node in _sceneNodes)
{
if (node is not PlayerController player) continue;
_player = player;
return _player;
}
GD.PrintErr("PlayerController not found in the scene tree.");
return null; return null;
} }

View File

@@ -2,7 +2,6 @@ using System.Collections.Generic;
using System.Linq; using System.Linq;
using Godot; using Godot;
using Godot.Collections; using Godot.Collections;
using Mr.BrickAdventures;
using Mr.BrickAdventures.scripts.components; using Mr.BrickAdventures.scripts.components;
using Mr.BrickAdventures.scripts.interfaces; using Mr.BrickAdventures.scripts.interfaces;
using Mr.BrickAdventures.scripts.Resources; using Mr.BrickAdventures.scripts.Resources;
@@ -11,8 +10,8 @@ namespace Mr.BrickAdventures.Autoloads;
public partial class SkillManager : Node public partial class SkillManager : Node
{ {
private GameManager _gameManager;
private PlayerController _player; private PlayerController _player;
private GameManager GameManager => GameManager.Instance;
[Export] public Array<SkillData> AvailableSkills { get; set; } = []; [Export] public Array<SkillData> AvailableSkills { get; set; } = [];
@@ -28,7 +27,6 @@ public partial class SkillManager : Node
public override void _Ready() public override void _Ready()
{ {
Instance = this; Instance = this;
_gameManager = GetNode<GameManager>(Constants.GameManagerPath);
} }
public override void _ExitTree() public override void _ExitTree()
@@ -85,21 +83,11 @@ public partial class SkillManager : Node
if (skillData.Type == SkillType.Throw) if (skillData.Type == SkillType.Throw)
{ {
var unlocked = _gameManager.GetUnlockedSkills(); // Remove any other active throw skill before adding the new one
foreach (var sd in unlocked) foreach (var sd in AvailableSkills)
{ {
SkillData data = null; if (sd != skillData && sd.Type == SkillType.Throw)
foreach (var s in AvailableSkills) RemoveSkill(sd.Name);
{
if (s == sd)
{
data = s;
break;
}
}
// Remove other throw skills if a new one is added
if (data is { Type: SkillType.Throw })
RemoveSkill(data.Name);
} }
} }
@@ -146,14 +134,6 @@ public partial class SkillManager : Node
if (IsInstanceValid(inst)) if (IsInstanceValid(inst))
inst.QueueFree(); inst.QueueFree();
var skills = _gameManager.GetUnlockedSkills();
foreach (var s in skills)
{
if (s.Name == skillName)
{
break;
}
}
ActiveComponents.Remove(skillName); ActiveComponents.Remove(skillName);
var sd = GetSkillByName(skillName); var sd = GetSkillByName(skillName);
if (sd != null) EmitSignalSkillRemoved(sd); if (sd != null) EmitSignalSkillRemoved(sd);
@@ -174,10 +154,11 @@ public partial class SkillManager : Node
public void ApplyUnlockedSkills() public void ApplyUnlockedSkills()
{ {
if (_player == null || !IsInstanceValid(_player)) return; if (_player == null || !IsInstanceValid(_player)) return;
if (GameManager == null) return;
foreach (var sd in AvailableSkills) foreach (var sd in AvailableSkills)
{ {
if (_gameManager.IsSkillUnlocked(sd)) if (GameManager.IsSkillUnlocked(sd))
{ {
CallDeferred(MethodName.AddSkill, sd); CallDeferred(MethodName.AddSkill, sd);
} }

View File

@@ -5,6 +5,11 @@ namespace Mr.BrickAdventures.Autoloads;
public partial class SpeedRunManager : Node public partial class SpeedRunManager : Node
{ {
public static SpeedRunManager Instance { get; private set; }
public override void _EnterTree() => Instance = this;
public override void _ExitTree() { if (Instance == this) Instance = null; }
public bool IsRunning { get; private set; } = false; public bool IsRunning { get; private set; } = false;
public bool IsVisible { get; private set; } = false; public bool IsVisible { get; private set; } = false;

View File

@@ -9,6 +9,10 @@ namespace Mr.BrickAdventures.Autoloads;
/// </summary> /// </summary>
public partial class StatisticsManager : Node public partial class StatisticsManager : Node
{ {
public static StatisticsManager Instance { get; private set; }
public override void _Ready() => Instance = this;
public override void _ExitTree() { if (Instance == this) Instance = null; }
/// <summary> /// <summary>
/// Gets the statistics dictionary from the store. /// Gets the statistics dictionary from the store.

View File

@@ -1,11 +1,10 @@
using Godot; using Godot;
using Godot.Collections;
namespace Mr.BrickAdventures.Autoloads; namespace Mr.BrickAdventures.Autoloads;
public partial class UIManager : Node public partial class UIManager : Node
{ {
[Export] public Array<Control> UiStack { get; set; } = new(); private readonly System.Collections.Generic.List<Control> UiStack = new();
[Signal] public delegate void ScreenPushedEventHandler(Control screen); [Signal] public delegate void ScreenPushedEventHandler(Control screen);
[Signal] public delegate void ScreenPoppedEventHandler(Control screen); [Signal] public delegate void ScreenPoppedEventHandler(Control screen);
@@ -34,19 +33,19 @@ public partial class UIManager : Node
return; return;
} }
var top = (Control)UiStack[^1]; var top = UiStack[^1];
UiStack.RemoveAt(UiStack.Count - 1); UiStack.RemoveAt(UiStack.Count - 1);
top.Hide(); top.Hide();
top.SetProcessInput(false); top.SetProcessInput(false);
EmitSignalScreenPopped(top); EmitSignalScreenPopped(top);
top.AcceptEvent(); top.AcceptEvent();
if (UiStack.Count > 0) ((Control)UiStack[^1]).GrabFocus(); if (UiStack.Count > 0) UiStack[^1].GrabFocus();
} }
public Control TopScreen() => UiStack.Count > 0 ? (Control)UiStack[^1] : null; public Control TopScreen() => UiStack.Count > 0 ? UiStack[^1] : null;
public bool IsScreenOnTop(Control screen) => UiStack.Count > 0 && (Control)UiStack[^1] == screen; public bool IsScreenOnTop(Control screen) => UiStack.Count > 0 && UiStack[^1] == screen;
public bool IsVisibleOnStack(Control screen) => UiStack.Contains(screen) && screen.Visible; public bool IsVisibleOnStack(Control screen) => UiStack.Contains(screen) && screen.Visible;

View File

@@ -23,17 +23,7 @@ public partial class LevelStateHandler : Node
private void OnLevelCompleted(int levelIndex, Node currentScene, double completionTime) private void OnLevelCompleted(int levelIndex, Node currentScene, double completionTime)
{ {
var store = GameStateStore.Instance; // State mutations (commit coins/skills, reset session) are handled by GameManager.OnLevelComplete
if (store == null) return; // before this event fires. This handler is reserved for future level-specific side-effects.
// Mark level complete and unlock next
store.MarkLevelComplete(levelIndex);
// Commit session data to persistent state
store.CommitSessionCoins();
store.CommitSessionSkills();
// Reset session for next level
store.ResetSession();
} }
} }

View File

@@ -1,5 +1,4 @@
using Godot; using Godot;
using Mr.BrickAdventures;
using Mr.BrickAdventures.Autoloads; using Mr.BrickAdventures.Autoloads;
namespace Mr.BrickAdventures.scripts.Events; namespace Mr.BrickAdventures.scripts.Events;
@@ -14,7 +13,7 @@ public partial class StatisticsEventHandler : Node
public override void _Ready() public override void _Ready()
{ {
_statisticsManager = GetNode<StatisticsManager>(Constants.StatisticsManagerPath); _statisticsManager = StatisticsManager.Instance;
// Subscribe to events // Subscribe to events
EventBus.Instance.CoinCollected += OnCoinCollected; EventBus.Instance.CoinCollected += OnCoinCollected;

View File

@@ -9,6 +9,8 @@ namespace Mr.BrickAdventures.scripts.State;
/// </summary> /// </summary>
public class PlayerState public class PlayerState
{ {
private const int DefaultLives = 3;
/// <summary> /// <summary>
/// Saved coins (not including current session). /// Saved coins (not including current session).
/// </summary> /// </summary>
@@ -17,7 +19,7 @@ public class PlayerState
/// <summary> /// <summary>
/// Remaining lives. /// Remaining lives.
/// </summary> /// </summary>
public int Lives { get; set; } = 3; public int Lives { get; set; } = DefaultLives;
/// <summary> /// <summary>
/// Indices of completed levels. /// Indices of completed levels.
@@ -45,17 +47,9 @@ public class PlayerState
public List<string> UnlockedAchievements { get; set; } = new(); public List<string> UnlockedAchievements { get; set; } = new();
/// <summary> /// <summary>
/// Creates a fresh default player state. /// Creates a fresh default player state (same as <c>new PlayerState()</c>).
/// </summary> /// </summary>
public static PlayerState CreateDefault() => new() public static PlayerState CreateDefault() => new();
{
Coins = 0,
Lives = 3,
CompletedLevels = new List<int>(),
UnlockedLevels = new List<int> { 0 },
UnlockedSkills = new List<SkillData>(),
Statistics = new Dictionary<string, int>()
};
/// <summary> /// <summary>
/// Resets this state to default values. /// Resets this state to default values.
@@ -63,7 +57,7 @@ public class PlayerState
public void Reset() public void Reset()
{ {
Coins = 0; Coins = 0;
Lives = 3; Lives = DefaultLives;
CompletedLevels.Clear(); CompletedLevels.Clear();
UnlockedLevels.Clear(); UnlockedLevels.Clear();
UnlockedLevels.Add(0); UnlockedLevels.Add(0);

View File

@@ -11,7 +11,7 @@ public partial class DamageComponent : Node
[Export] public StatusEffectDataResource StatusEffectData { get; set; } [Export] public StatusEffectDataResource StatusEffectData { get; set; }
[Export] public Timer DamageTimer { get; set; } [Export] public Timer DamageTimer { get; set; }
private Node _currentTarget = null; private readonly System.Collections.Generic.HashSet<Node> _currentTargets = new();
private KnockbackComponent _knockbackComponent = null; private KnockbackComponent _knockbackComponent = null;
[Signal] public delegate void EffectInflictedEventHandler(Node2D target, StatusEffectDataResource effect); [Signal] public delegate void EffectInflictedEventHandler(Node2D target, StatusEffectDataResource effect);
@@ -35,10 +35,11 @@ public partial class DamageComponent : Node
public override void _Process(double delta) public override void _Process(double delta)
{ {
if (_currentTarget == null) return; if (_currentTargets.Count == 0) return;
if (DamageTimer != null) return; if (DamageTimer != null) return;
ProcessEntityAndApplyDamage(_currentTarget as Node2D); foreach (var target in _currentTargets)
ProcessEntityAndApplyDamage(target as Node2D);
} }
public void DealDamage(HealthComponent target) => target.DecreaseHealth(Damage); public void DealDamage(HealthComponent target) => target.DecreaseHealth(Damage);
@@ -56,18 +57,19 @@ public partial class DamageComponent : Node
private void OnAreaBodyExited(Node2D body) private void OnAreaBodyExited(Node2D body)
{ {
if (body != _currentTarget) return; _currentTargets.Remove(body);
_currentTarget = null; if (_currentTargets.Count == 0)
DamageTimer?.Stop(); DamageTimer?.Stop();
} }
private void OnAreaBodyEntered(Node2D body) private void OnAreaBodyEntered(Node2D body)
{ {
_currentTarget = body; _currentTargets.Add(body);
if (!CheckIfProcessingIsOn()) if (!CheckIfProcessingIsOn())
return; return;
if (_currentTargets.Count == 1)
DamageTimer?.Start(); DamageTimer?.Start();
ProcessEntityAndApplyDamage(body); ProcessEntityAndApplyDamage(body);
@@ -75,9 +77,8 @@ public partial class DamageComponent : Node
private void OnDamageTimerTimeout() private void OnDamageTimerTimeout()
{ {
if (_currentTarget == null) return; foreach (var target in _currentTargets)
ProcessEntityAndApplyDamage(target as Node2D);
ProcessEntityAndApplyDamage(_currentTarget as Node2D);
} }
private void ProcessEntityAndApplyDamage(Node2D body) private void ProcessEntityAndApplyDamage(Node2D body)

View File

@@ -1,6 +1,4 @@
using System.Threading.Tasks;
using Godot; using Godot;
using Mr.BrickAdventures;
using Mr.BrickAdventures.Autoloads; using Mr.BrickAdventures.Autoloads;
namespace Mr.BrickAdventures.scripts.components; namespace Mr.BrickAdventures.scripts.components;
@@ -20,27 +18,16 @@ public partial class HealthComponent : Node2D
public override void _Ready() public override void _Ready()
{ {
_floatingTextManager = GetNode<FloatingTextManager>(Constants.FloatingTextManagerPath); _floatingTextManager = FloatingTextManager.Instance;
} }
public void SetHealth(float newValue) public void SetHealth(float newValue) => ApplyHealthChange(newValue);
{ public void IncreaseHealth(float delta) => ApplyHealthChange(Health + delta);
_ = ApplyHealthChange(newValue); public void DecreaseHealth(float delta) => ApplyHealthChange(Health - delta);
}
public void IncreaseHealth(float delta)
{
_ = ApplyHealthChange(Health + delta);
}
public void DecreaseHealth(float delta)
{
_ = ApplyHealthChange(Health - delta);
}
public float GetDelta(float newValue) => newValue - Health; public float GetDelta(float newValue) => newValue - Health;
private async Task ApplyHealthChange(float newHealth, bool playSfx = true) private void ApplyHealthChange(float newHealth, bool playSfx = true)
{ {
newHealth = Mathf.Clamp(newHealth, 0.0f, MaxHealth); newHealth = Mathf.Clamp(newHealth, 0.0f, MaxHealth);
var delta = newHealth - Health; var delta = newHealth - Health;
@@ -53,39 +40,19 @@ public partial class HealthComponent : Node2D
else else
_floatingTextManager?.ShowHeal(delta, GlobalPosition); _floatingTextManager?.ShowHeal(delta, GlobalPosition);
if (playSfx)
{
if (delta > 0f && HealSfx != null)
{
HealSfx.Play();
}
else if (delta < 0f && HurtSfx != null)
{
HurtSfx.Play();
await HurtSfx.ToSignal(HurtSfx, AudioStreamPlayer2D.SignalName.Finished);
}
}
Health = newHealth; Health = newHealth;
if (playSfx)
{
if (delta > 0f)
HealSfx?.Play();
else
HurtSfx?.Play();
}
if (Health <= 0f) if (Health <= 0f)
{
EmitSignalDeath(); EmitSignalDeath();
// Emit global event if this is the player
if (Owner is PlayerController)
EventBus.EmitPlayerDied(GlobalPosition);
}
else else
{
EmitSignalHealthChanged(delta, Health); EmitSignalHealthChanged(delta, Health);
// Emit global events if this is the player
if (Owner is PlayerController)
{
if (delta < 0f)
EventBus.EmitPlayerDamaged(Mathf.Abs(delta), Health, GlobalPosition);
else
EventBus.EmitPlayerHealed(delta, Health, GlobalPosition);
}
}
} }
} }

View File

@@ -2,7 +2,6 @@ using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Threading.Tasks; using System.Threading.Tasks;
using Godot; using Godot;
using Mr.BrickAdventures;
using Mr.BrickAdventures.Autoloads; using Mr.BrickAdventures.Autoloads;
namespace Mr.BrickAdventures.scripts.components; namespace Mr.BrickAdventures.scripts.components;
@@ -34,8 +33,19 @@ public partial class PlayerController : CharacterBody2D
public override void _Ready() public override void _Ready()
{ {
var skillManager = GetNodeOrNull<SkillManager>(Constants.SkillManagerPath); SkillManager.Instance?.RegisterPlayer(this);
skillManager?.RegisterPlayer(this);
// Wire HealthComponent signals to global EventBus events
var health = GetNodeOrNull<HealthComponent>("HealthComponent");
if (health != null)
{
health.Death += () => EventBus.EmitPlayerDied(GlobalPosition);
health.HealthChanged += (delta, total) =>
{
if (delta < 0f) EventBus.EmitPlayerDamaged(Mathf.Abs(delta), total, GlobalPosition);
else EventBus.EmitPlayerHealed(delta, total, GlobalPosition);
};
}
_inputHandler = GetNode<PlayerInputHandler>("PlayerInputHandler"); _inputHandler = GetNode<PlayerInputHandler>("PlayerInputHandler");
foreach (var child in MovementAbilitiesContainer.GetChildren()) foreach (var child in MovementAbilitiesContainer.GetChildren())
@@ -49,6 +59,7 @@ public partial class PlayerController : CharacterBody2D
_ = ConnectJumpAndGravityAbilities(); _ = ConnectJumpAndGravityAbilities();
EmitSignalMovementAbilitiesChanged(); EmitSignalMovementAbilitiesChanged();
EventBus.EmitPlayerSpawned(this);
} }
public override void _PhysicsProcess(double delta) public override void _PhysicsProcess(double delta)