openthebox/refactoring_plan.md
Samuel Bouchet 92ce67684d 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.
2026-03-14 09:56:53 +01:00

8.6 KiB

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 :
    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.