refactor(game): COUP-03 + CONV-02 encapsulate Options state behind setters
L'estat mutable d'Options (window, video, audio, locale, presets, paths) es mou a un anonymous namespace dins options.cpp. L'API pública del namespace passa a ser: - Getters const-ref: window(), video(), audio(), locale(), configVersion(), postfxPresets(), crtpiPresets(). - Setters específics que muten i persisteixen via saveToFile(): setWindowZoom, setWindowFullscreen, setVideoVSync, setVideoScalingMode, setVideoRenderInfoPosition, setVideoRenderInfoShowTime, setShaderEnabled, setActiveShaderName, setPostFXPreset, setCrtPiPreset. Els setters NO notifiquen els subsistemes: el caller (global_inputs.cpp) crida explícitament Screen::setWindowZoom, reapplyShader, etc. La part "observers" del audit es deixa WONTFIX justificat — notificar des d'Options invertiria la direcció Director→Screen/Audio i Options aprenria del layout dels subsistemes. CONV-02 tanca al pas: la part Options dels "globals sense observers" queda resolta amb la nova encapsulació. Map::* (l'altra meitat de CONV-02) ja s'havia tancat amb PATCH-07. Smoke tests 13/13 OK; log Game start → Bye! sota xvfb-run + SDL_VIDEODRIVER=dummy + SIGINT idèntic al baseline de main. clang-tidy i cppcheck nets sobre els fitxers modificats. Refs: docs/audit-game.md (COUP-03, CONV-02, INCONS-01). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
32dcf264c4
commit
a3b03b87be
5 files changed
+256
-160
No files matched your search
+14
-4
@@ -3,7 +3,7 @@
|
||||
Rama: `audit/game`.
|
||||
Cobertura: `source/game/{entities,map,scenes,ui}` + `source/game/options.{hpp,cpp}` (12 fitxers, ~1.486 línies).
|
||||
|
||||
Estado global: **28 DONE, 2 DONE (parcial), 5 WONTFIX, 4 DEFERRED** sobre 39 ítems (el top 5 compta dins dels seus IDs individuals). Cada canvi té un commit aïllat i s'ha provat amb `SDL_VIDEODRIVER=dummy` + `SIGINT` (el log de `Game start` a `Bye!` és idèntic entre commits; la branca `config.yaml` de backup verificada manualment amb un fitxer corrupte). Els 4 DEFERRED (COUP-02, COUP-03, CONV-02 resta, PATCH-01) van a la futura branca `refactor/options-class`.
|
||||
Estado global: **28 DONE, 2 DONE (parcial), 5 WONTFIX, 4 DEFERRED** sobre 39 ítems (el top 5 compta dins dels seus IDs individuals). Cada canvi té un commit aïllat i s'ha provat amb `SDL_VIDEODRIVER=dummy` + `SIGINT` (el log de `Game start` a `Bye!` és idèntic entre commits; la branca `config.yaml` de backup verificada manualment amb un fitxer corrupte). Els 4 DEFERRED (COUP-02, COUP-03, CONV-02 resta, PATCH-01) es tanquen a la branca `refactor/options-class` (registre de commits al final del fitxer).
|
||||
|
||||
Estados posibles:
|
||||
- `PENDING` — identificat, sense tocar
|
||||
@@ -69,14 +69,14 @@ Els IDs es mantenen estables una vegada assignats. Cada entrada apunta al fitxer
|
||||
|----|--------------|----------|-----------|-------|-------|
|
||||
| COUP-01 | `core/input/global_inputs.cpp:15` | `core/` inclou `game/ui/notification_presets.hpp` | Crítico | DONE | `notification_presets.hpp` mogut a `core/rendering/` (al costat del `Notifier` que consumeix). `global_inputs.cpp` i el comentari de `notifier.hpp` actualitzats |
|
||||
| COUP-02 | `player.cpp` + `Map::PLAYFIELD_*`, `PITCH_*`, `GRID_*`, `collides` | `Player` coneix la geometria interna del mapa | Importante | DEFERRED | Player només llegeix globals de `Map::`, no muta res. Injectar `MapInfo&` té sentit quan hi haja múltiples mapes simultanis o un sistema tipus ECS. Mentre hi haja un Map actiu global, l'acoblament és acceptable. Es refacta amb COUP-03 a `refactor/options-class` |
|
||||
| COUP-03 | `options.hpp:96-105` | Globals `inline` mutats lliurement des de `core/input/global_inputs.cpp` | Importante | DEFERRED | Encapsular tots els globals darrere setters + observers per Screen/Audio/RenderInfo és un refactor ample (equivalent a COUP-01 de l'audit d'àudio, que va anar a una branca pròpia). Es tractarà a `refactor/options-class` perquè toca tots els callers de core/input. Es fa en branca separada |
|
||||
| COUP-03 | `options.hpp:96-105` | Globals `inline` mutats lliurement des de `core/input/global_inputs.cpp` | Importante | DONE | Estat mutable mogut a anonymous namespace dins `options.cpp`; API pública són getters const-ref (`window()`, `video()`, `audio()`, `locale()`, `postfxPresets()`, `crtpiPresets()`) + setters específics (`setWindowZoom`, `setShaderEnabled`, `setActiveShaderName`, ...) que muten i persisteixen via `saveToFile()`. Els setters NO notifiquen els subsistemes: el caller (`global_inputs.cpp`) crida després `Screen::setWindowZoom`, `reapplyShader()`, etc. La part "observers" del audit es deixa WONTFIX (veure CONV-02): notificar des d'Options invertiria la direcció `Director→Screen/Audio` i Options aprenria del layout dels subsistemes |
|
||||
| COUP-04 | `tilemap.cpp:117-138` | Clip+blit directe al canvas ARGB que podria viure a `Surface` | Menor | WONTFIX | Únic caller en tot el projecte. Promoure a `Surface::blitInto` ara afegiria una API genèrica per a un ús únic (YAGNI). Si un segon client apareix, es mourea en aquell moment |
|
||||
|
||||
## 5. Inconsistències internes
|
||||
|
||||
| ID | Fitxer:línia | Troballa | Severitat | Estat | Notes |
|
||||
|----|--------------|----------|-----------|-------|-------|
|
||||
| INCONS-01 | `game/*` | 3 estils de mòdul: namespace+inline (Options), classe all-static (Map), OOP (Player/Tilemap), namespace+constexpr (UiColor/Notification) | Importante | DONE (parcial) | Resolt "classe all-static" amb PATCH-07 (Map → namespace). Queden 3 estils diferenciats per responsabilitat: `namespace` per a estat compartit (Options, Map), classes per a entitats amb estat (Player, Tilemap), `namespace+constexpr` per a alias/presets (UiColor, Notification). La distinció ara és intencional, no fortuïta |
|
||||
| INCONS-01 | `game/*` | 3 estils de mòdul: namespace+inline (Options), classe all-static (Map), OOP (Player/Tilemap), namespace+constexpr (UiColor/Notification) | Importante | DONE (parcial) | Resolt "classe all-static" amb PATCH-07 (Map → namespace). Queden 3 estils diferenciats per responsabilitat: `namespace` per a estat compartit (Options, Map), classes per a entitats amb estat (Player, Tilemap), `namespace+constexpr` per a alias/presets (UiColor, Notification). La distinció ara és intencional, no fortuïta. NOTA (post `refactor/options-class`): "encapsular" en COUP-03 vol dir ocultar l'estat mutable darrere de l'API del namespace (anonymous namespace + getters/setters), no convertir Options a classe — el patró `namespace per a estat compartit` queda preservat |
|
||||
| INCONS-02 | `options.cpp:372,477,530,579,607` | `std::cerr` en comptes de `std::cout` (convenció del projecte) | Importante | DONE | Totes les ocurrències (10, incloent les afegides a PATCH-04) substituïdes per `std::cout`. Test manual: amb config.yaml trencat el missatge de backup va per stdout |
|
||||
| INCONS-03 | `options.cpp` vs `tilemap.cpp:77-78` | Diferent política en YAML absent: `contains()` vs accés directe que llança | Menor | WONTFIX | Les dues polítiques són correctes per al seu cas: `Tilemap`/`Map::loadFrom` tenen fitxers de definició (asset obligatori → llançar); `Options` parseja preferències de l'usuari (camp absent → manté default). No és inconsistència sinó dos contractes diferents |
|
||||
| INCONS-04 | `options.cpp`, `player.cpp` | Locals ALL-CAPS (`CONTENT`, `IT`, `DT`, `OLD_X`) barrejats amb `snake_case` | Menor | WONTFIX | El `.clang-tidy` del projecte exigeix `LocalConstantCase = UPPER_CASE` i `VariableCase = lower_case` — els ALL-CAPS són per a `const`/`constexpr` i els snake_case per a mutables. La barreja es correcta i està dictada pel linter |
|
||||
@@ -97,6 +97,16 @@ Els IDs es mantenen estables una vegada assignats. Cada entrada apunta al fitxer
|
||||
| ID | Fitxer:línia | Troballa | Severitat | Estat | Notes |
|
||||
|----|--------------|----------|-----------|-------|-------|
|
||||
| CONV-01 | `player.*`, `map.*`, `ui/*`, `game_scene.cpp` | Speed, caixa de col·lisió, geometria, índexs paleta, rutes d'assets, nom del track: tot cablejat | Crítico | DONE | Tapat per HC-01/02/03/04/05/06: speed a Defaults::Player, col·lisió a player.yaml, geometria a pyramid.yaml, índexs paleta a Defaults::UI, rutes a Defaults::Pyramid + Config |
|
||||
| CONV-02 | `map.*`, `options.*` | Interfaces+callbacks: `Map` simulated class, `Options` globals sense observers | Importante | DONE (parcial) | Map resolt amb PATCH-07 (namespace). La part d'Options/observers va amb COUP-03 (DEFERRED a `refactor/options-class`) |
|
||||
| CONV-02 | `map.*`, `options.*` | Interfaces+callbacks: `Map` simulated class, `Options` globals sense observers | Importante | DONE | Map resolt amb PATCH-07 (namespace). Options resolt amb COUP-03: estat mutable encapsulat darrere de l'API del namespace (anonymous ns + getters/setters). La part "observers" (notificacions automàtiques a Screen/Audio) queda WONTFIX justificat: invertiria la direcció de dependència `Director→Screen/Audio` que el projecte manté intencionadament |
|
||||
| CONV-03 | `options.cpp` | `std::cerr` viola la regla "std::cout only" | Importante | DONE | Resolt amb INCONS-02 |
|
||||
| CONV-04 | `player.cpp:27` | `SDL_GetKeyboardState` directe en `Player::update` (fuga de SDL al gameplay) | Menor | WONTFIX | `SDL_GetKeyboardState` és l'accés correcte per a state-based polling a 60Hz; `EventBuffer` és per a esdeveniments discrets. La fuga és cosmètica — abstraure-la no aportaria valor |
|
||||
|
||||
---
|
||||
|
||||
## Registro de commits — `refactor/options-class`
|
||||
|
||||
Branca per a tancar els 4 ítems DEFERRED de `audit/game`. Base: `main` (32dcf26).
|
||||
|
||||
- `<pendent>` — COUP-03 + CONV-02: estat mutable d'`Options` mogut a anonymous ns + API de getters/setters; `global_inputs.cpp` i `director.cpp` adaptats. CONV-02 (part Options) tanca al pas. La part "observers" del audit es deixa WONTFIX (invertiria `Director→Screen/Audio`)
|
||||
- `<pendent>` — COUP-02: `MapInfo` plana injectada al `Player` (només Player; momia/roca/loadlevel resten amb `Map::*` global, com diu la nota original del audit)
|
||||
- `<pendent>` — PATCH-01: `tryMove(dx, dy, collisionFn)` a `MovingSprite` amb hook virtual `collisionBox()`; `Player::update` deixa de fer `update + revertir + reaplicar`
|
||||
Reference in new issue
Block a user