Fix test for early-game box counter and add refactoring plan
Update ShowGameState test to expect box counter output (added in proposals_2.md). Add refactoring_plan.md documenting Black Box Sim pattern violations and prioritized fixes.
This commit is contained in:
parent
6e5bc6e35e
commit
92ce67684d
2 changed files with 168 additions and 2 deletions
165
refactoring_plan.md
Normal file
165
refactoring_plan.md
Normal file
|
|
@ -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<string> options, List<string>? 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<AvailableAction>` 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.
|
||||||
|
|
@ -879,12 +879,13 @@ public class BasicRendererOutputTests : IDisposable
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void ShowGameState_WithoutCompletionTracker_NoOutput()
|
public void ShowGameState_WithoutCompletionTracker_ShowsBoxCount()
|
||||||
{
|
{
|
||||||
var state = GameState.Create("Test", Locale.EN);
|
var state = GameState.Create("Test", Locale.EN);
|
||||||
var ctx = new RenderContext();
|
var ctx = new RenderContext();
|
||||||
_renderer.ShowGameState(state, ctx);
|
_renderer.ShowGameState(state, ctx);
|
||||||
Assert.Empty(_capture.Output);
|
// Early-game: shows box counter even without panels unlocked
|
||||||
|
Assert.Contains("Boxes Opened", _capture.Output);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue