diff --git a/refactoring_plan.md b/refactoring_plan.md new file mode 100644 index 0000000..8242fd7 --- /dev/null +++ b/refactoring_plan.md @@ -0,0 +1,165 @@ +# Refactoring Plan — Black Box Sim Compliance + +## Objectif + +Rendre le code conforme au pattern **Black Box Sim** décrit dans le README : + +``` +Input → GameAction → GameSimulation (ZERO I/O, pure logic) → GameEvent[] → IRenderer +``` + +La simulation ne lit jamais d'entrée et n'écrit jamais de sortie. Elle reçoit des actions, elle retourne des événements. Program.cs orchestre l'entrée/sortie et connecte le tout. + +--- + +## Violations actuelles et corrections proposées + +### 1. CraftingEngine fonctionne hors de GameSimulation (CRITIQUE) + +**Problème** : `CraftingEngine` est instancié directement dans `Program.cs` (ligne 27) et appelé en dehors de `GameSimulation` via `TickCraftingJobs()` et `CollectCrafting()`. De plus, un `MetaEngine` est créé ad-hoc dans `CollectCrafting()` pour traiter les items craftés. + +**Solution** : +- Internaliser `CraftingEngine` dans `GameSimulation` +- Ajouter `TickCraftingAction` et `CollectCraftingAction` comme GameActions +- `GameSimulation.ProcessAction(TickCraftingAction)` appelle `CraftingEngine.TickJobs()` en interne +- `GameSimulation.ProcessAction(CollectCraftingAction)` appelle `CollectCompleted()` + `MetaEngine` + `AutoCraftCheck()` en interne +- Supprimer `_craftingEngine` de `Program.cs` + +**Fichiers** : `Program.cs`, `GameSimulation.cs`, `CraftingEngine.cs`, `GameAction.cs` + +--- + +### 2. AdventureEngine appelle directement le renderer (CRITIQUE) + +**Problème** : `AdventureEngine` reçoit un `IRenderer` dans son constructeur et appelle `ShowAdventureDialogue()`, `ShowAdventureChoice()`, `WaitForKeyPress()` directement. Ceci viole fondamentalement le pattern car la simulation fait de l'I/O. + +**Solution** : +- Introduire un `IAdventureUI` interface avec les méthodes de callback : + - `ShowDialogue(string? character, string text)` + - `int ShowChoice(List options, List? hints)` + - `ShowMessage(string message)` +- `Program.cs` fournit l'implémentation de `IAdventureUI` qui délègue au `IRenderer` +- `AdventureEngine` retourne des `AdventureEvent` pour les changements d'état (items, resources) au lieu d'appeler `ShowMessage()` pour les effets de jeu +- Alternative : Garder le callback pour le dialogue interactif (nécessaire car Loreline est bloquant) mais retirer tous les appels `ShowMessage` des fonctions custom Loreline (grantItem, addResource, removeItem, markSecretBranch) — ces effets sont déjà retournés comme `GameEventResult` + +**Fichiers** : `AdventureEngine.cs`, `Program.cs`, nouveau `IAdventureUI.cs` + +--- + +### 3. Program.cs mute directement GameState (IMPORTANT) + +**Problème** : Plusieurs endroits dans `Program.cs` modifient `GameState` sans passer par `GameSimulation` : +- `_state.AddItem(starterBox)` dans `NewGame()` (ligne 261) +- `_state.AddItem(...)` dans `RunAdventure()` (ligne 875) +- `_state.CurrentLocale = newLocale` dans `ChangeLanguage()` (ligne 358) +- `_state.EventLog.Add(message)` dans `AddEventLog()` (ligne 1054) + +**Solution** : +- Créer `NewGameAction` → retourne un `ItemReceivedEvent` avec la starter box +- Les événements d'aventure (ItemGranted) devraient être traités par `GameSimulation.ProcessAction(AdventureResultAction)` au lieu de muter directement l'inventaire +- Le changement de locale passe déjà par `ChangeLocaleAction` quand fait en jeu — s'assurer que `ChangeLanguage()` utilise aussi ce chemin +- L'EventLog devrait être alimenté par le renderer ou un composant de présentation séparé, pas en mutant `GameState` + +**Fichiers** : `Program.cs`, `GameSimulation.cs`, `GameAction.cs` + +--- + +### 4. Les renderers dépendent de ContentRegistry (IMPORTANT) + +**Problème** : `SpectreRenderer`, `TerminalGuiRenderer`, `InventoryPanel`, `CraftingPanel` reçoivent tous un `ContentRegistry?` et font des lookups d'`ItemDefinition` et `BoxDefinition` directement. + +**Solution** : +- Créer un modèle de vue (ViewModel) dans le `RenderContext` ou à côté de `GameState` : + ```csharp + record DisplayItem(string Name, string Rarity, string Category, int Qty, string? Description, ...); + record DisplayCraftingJob(string Name, string Station, int ProgressPercent, bool IsComplete); + ``` +- La résolution `DefinitionId → nom localisé + rarity + description` se fait UNE FOIS au moment du rendu, dans Program.cs ou un nouveau `ViewModelBuilder` +- Les renderers reçoivent des `DisplayItem[]` au lieu de `GameState + ContentRegistry` +- **Phase transitoire** : Garder le `ContentRegistry` dans les panels pour l'instant mais créer le `ViewModelBuilder` progressivement + +**Fichiers** : Nouveau `Rendering/ViewModels/`, `InventoryPanel.cs`, `CraftingPanel.cs`, `SpectreRenderer.cs`, `TerminalGuiRenderer.cs` + +--- + +### 5. RenderEvents mélange logique et présentation (MODÉRÉ) + +**Problème** : La méthode `RenderEvents()` dans `Program.cs` (lignes 475-605) fait : +- Du rendu (`_renderer.ShowXxx()`) +- De la logique (`_renderContext.Unlock()`, `RefreshRenderer()`) +- Du tracking (`AddEventLog()`) +- De l'orchestration (`await RunAdventure()`) + +**Solution** : +- Séparer en deux passes : + 1. **Pass logique** : Traite les événements qui modifient l'état du programme (unlock features, update render context) + 2. **Pass présentation** : Appelle le renderer pour chaque événement à afficher +- Déplacer `RefreshRenderer()` dans la pass logique, avant la pass présentation +- L'EventLog devrait être alimenté dans la pass logique, pas en parallèle du rendu + +**Fichiers** : `Program.cs` + +--- + +### 6. Program.cs connaît les types de domaine (MODÉRÉ) + +**Problème** : Le game loop inspecte directement `ItemCategory`, `ItemDefinition.ResourceType`, `CosmeticSlot`, etc. pour construire les menus et déterminer les actions disponibles. + +**Solution** : +- Ajouter une méthode `GameSimulation.GetAvailableActions(GameState) → List` qui retourne les actions possibles avec leurs labels +- Ou : ajouter des propriétés helper sur `GameState` : + - `HasBoxes`, `HasConsumables`, `HasCosmeticsToEquip`, `HasCompletedCrafting` +- L'objectif n'est pas d'éliminer toute connaissance des catégories dans Program.cs (c'est la couche d'orchestration) mais de réduire les requêtes directes au registre + +**Fichiers** : `GameSimulation.cs` ou `GameState.cs`, `Program.cs` + +--- + +### 7. Portrait dupliqué entre les renderers (FAIBLE) + +**Problème** : `TerminalGuiRenderer` (lignes 614-677) duplique tout l'art ASCII du portrait qui existe déjà dans `PortraitPanel.cs`. + +**Solution** : +- Extraire les méthodes `GetHairArt()`, `GetEyeArt()`, etc. dans un helper statique partagé `PortraitArt` +- `PortraitPanel` et `TerminalGuiRenderer` utilisent tous les deux `PortraitArt.GetHairArt()`, etc. +- Alternative : `PortraitPanel` expose une méthode `RenderPlainText()` que `TerminalGuiRenderer` peut appeler + +**Fichiers** : Nouveau `Rendering/Panels/PortraitArt.cs`, `PortraitPanel.cs`, `TerminalGuiRenderer.cs` + +--- + +### 8. EventLog est de l'état de présentation dans GameState (FAIBLE) + +**Problème** : `GameState.EventLog` est une liste de strings de présentation ("★ Text Colors", "+ Bronze [Uncommon]") stockée dans le modèle de simulation. Elle n'est pas persistée (pas dans les saves), confirmant qu'il s'agit d'état de présentation. + +**Solution** : +- Déplacer `EventLog` hors de `GameState` dans un composant de présentation séparé (ex: `ChatLog` dans le namespace `Rendering`) +- `Program.cs` alimente le `ChatLog` dans sa pass de rendu des événements +- Les renderers reçoivent le `ChatLog` au lieu de lire `state.EventLog` + +**Fichiers** : `GameState.cs`, nouveau `Rendering/ChatLog.cs`, `Program.cs`, `ChatPanel.cs`, `TerminalGuiRenderer.cs` + +--- + +## Ordre de priorité recommandé + +| Priorité | Tâche | Impact | Effort | +|----------|-------|--------|--------| +| 1 | CraftingEngine dans GameSimulation | Critique | Moyen | +| 2 | Mutations directes de GameState | Important | Faible | +| 3 | RenderEvents deux passes | Modéré | Moyen | +| 4 | ViewModels pour renderers | Important | Élevé | +| 5 | AdventureEngine I/O callbacks | Critique | Élevé | +| 6 | GetAvailableActions helper | Modéré | Faible | +| 7 | Portrait partagé | Faible | Faible | +| 8 | EventLog hors de GameState | Faible | Faible | + +--- + +## Principes directeurs + +1. **GameSimulation est une boîte noire** — elle reçoit des `GameAction`, elle retourne des `GameEvent[]`. Rien d'autre. +2. **Les renderers ne connaissent pas le domaine** — ils reçoivent des données pré-formatées (ViewModels) et les affichent. +3. **Program.cs est le câblage** — il connecte l'input au simulateur et le simulateur au renderer. Il ne contient pas de logique de jeu. +4. **Les mutations de GameState passent par GameSimulation** — aucune écriture directe depuis Program.cs. +5. **Pas de refactoring big-bang** — chaque tâche peut être faite indépendamment et testée. diff --git a/tests/OpenTheBox.Tests/RendererTests.cs b/tests/OpenTheBox.Tests/RendererTests.cs index db5e3f0..04a44db 100644 --- a/tests/OpenTheBox.Tests/RendererTests.cs +++ b/tests/OpenTheBox.Tests/RendererTests.cs @@ -879,12 +879,13 @@ public class BasicRendererOutputTests : IDisposable } [Fact] - public void ShowGameState_WithoutCompletionTracker_NoOutput() + public void ShowGameState_WithoutCompletionTracker_ShowsBoxCount() { var state = GameState.Create("Test", Locale.EN); var ctx = new RenderContext(); _renderer.ShowGameState(state, ctx); - Assert.Empty(_capture.Output); + // Early-game: shows box counter even without panels unlocked + Assert.Contains("Boxes Opened", _capture.Output); } [Fact]