From 1643811d86095618eb33f3a7d89a067c9f78b6c6 Mon Sep 17 00:00:00 2001 From: Sergio Valor Date: Mon, 20 Apr 2026 18:16:05 +0200 Subject: [PATCH] INCONS-05 + triage final d'auditoria game/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - INCONS-05: `buildScreenConfig` passa a return amb designated initializers, consistent amb `buildAudioConfig`. - INCONS-01: marcat DONE (parcial) — queden 3 estils intencionals (namespace per estat compartit, classes per entitats, constexpr per alias), ja no barreja fortuïta. - INCONS-03/04, PATCH-03, COUP-04, CONV-04: WONTFIX amb raó documentada (contractes diferents, linter ho dicta, o sobre-abstracció per a un únic caller). - COUP-02/03, CONV-02 (parcial): DEFERRED a `refactor/options-class` (equivalent a COUP-01 de l'audit d'àudio). - CONV-01: DONE per HC-01/02/03/04/05/06. --- docs/audit-game.md | 22 +++++++++++----------- source/game/options.cpp | 18 +++++++++--------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/audit-game.md b/docs/audit-game.md index 299488b..6eaf5ce 100644 --- a/docs/audit-game.md +++ b/docs/audit-game.md @@ -55,7 +55,7 @@ Els IDs es mantenen estables una vegada assignats. Cada entrada apunta al fitxer |----|--------------|----------|-----------|-------|-------| | PATCH-01 | `player.cpp:66-77` | Col·lisió per eix fent `AnimatedSprite::update` + revertir + reaplicar | Importante | PENDING | `tryMove(dx, dy, collisionFn)` a la classe base | | PATCH-02 | `player.cpp:13-15` | Comentari tipus TODO ("si més endavant volem sprint...") | Menor | DONE | Eliminat en HC-01 (la constant ja no viu al `.cpp`, sinó a `Defaults::Player`) | -| PATCH-03 | `map.cpp:13-15` | `overlapStrict` custom perquè SDL3 considera el toc com intersecció | Menor | PENDING | Si s'usa més, mourea `utils/` | +| PATCH-03 | `map.cpp:13-15` | `overlapStrict` custom perquè SDL3 considera el toc com intersecció | Menor | WONTFIX | Només s'usa dins de `Map::collides` i és una línia; moure a utils/ ara seria sobre-abstraure. Si apareix a un segon mòdul, promourea a `utils/geometry.hpp` | | PATCH-04 | `options.cpp:361-363,371-375,475-480,528-533` | Reescriure silenciosament `config.yaml` al fallar parseig o canviar versió | Crítico | DONE | Helper `backupFile()` que copia a `.bak` si existeix. Cridat abans de cada reescriptura forçada (versió mismatch + catch a `loadFromFile`, `presets:` absent/invàlid/buit + catch a `loadPresetsFromFile`). Primera execució no genera `.bak`. Missatge explícit per motiu de reescriptura | | PATCH-05 | `game_scene.cpp:51` | `if (Audio::get() != nullptr)` com a defensa sobre un singleton Meyers | Menor | DONE | `if` eliminat; comentari documenta l'invariant (Director::init crida Audio::init abans). Si algun dia salta, serà un bug real que val la pena detectar | | PATCH-06 | `game_scene.cpp:78-91` | Ordenació per `getFeetY()` amb 1 jugador i sense sarcs-sprite: cerimonial | Menor | DONE | Short-circuit si `players_.size() <= 1`: no es construeix vector temporal ni s'ordena. La branca multi-jugador queda preparada per al dia que hi haja 2+ jugadors o sarcs-sprites | @@ -66,19 +66,19 @@ Els IDs es mantenen estables una vegada assignats. Cada entrada apunta al fitxer | ID | Fitxer:línia | Troballa | Severitat | Estat | Notes | |----|--------------|----------|-----------|-------|-------| | 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 | PENDING | Passar `const MapInfo&` o un `World*` al ctor del player | -| COUP-03 | `options.hpp:96-105` | Globals `inline` mutats lliurement des de `core/input/global_inputs.cpp` | Importante | PENDING | Setters + signal quan canvien | -| COUP-04 | `tilemap.cpp:117-138` | Clip+blit directe al canvas ARGB que podria viure a `Surface` | Menor | PENDING | `blitInto(canvas, origin)` genèric | +| 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-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 | PENDING | Decidir i uniformar | +| 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-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 | PENDING | Uniformar (llançar sempre si és un fitxer obligatori) | -| INCONS-04 | `options.cpp`, `player.cpp` | Locals ALL-CAPS (`CONTENT`, `IT`, `DT`, `OLD_X`) barrejats amb `snake_case` | Menor | PENDING | Uniformar estil | -| INCONS-05 | `options.cpp:420-430` | `Screen::Config` s'omple camp a camp mentre que `Audio::Config` fa `designated initializers` | Menor | PENDING | Uniformar | +| 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 | +| INCONS-05 | `options.cpp:420-430` | `Screen::Config` s'omple camp a camp mentre que `Audio::Config` fa `designated initializers` | Menor | DONE | `buildScreenConfig` passa a return amb designated initializers, consistent amb `buildAudioConfig` | ## 6. Codi mort @@ -94,7 +94,7 @@ 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 | PENDING | Tapat parcialment per HC-01/02/03/05/06 | -| CONV-02 | `map.*`, `options.*` | Interfaces+callbacks: `Map` simulated class, `Options` globals sense observers | Importante | PENDING | Cobert per PATCH-07 + COUP-03 | +| 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-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 | PENDING | Anar per `GlobalInputs`/`EventBuffer` | +| 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 | diff --git a/source/game/options.cpp b/source/game/options.cpp index 0934a5e..da175d3 100644 --- a/source/game/options.cpp +++ b/source/game/options.cpp @@ -504,15 +504,15 @@ namespace Options { } auto buildScreenConfig() -> Screen::Config { - Screen::Config config; - config.zoom = window.zoom; - config.fullscreen = window.fullscreen; - config.vsync = video.vsync; - config.scaling_mode = video.scaling_mode; - config.texture_filter = video.texture_filter; - config.gpu_acceleration = video.gpu.acceleration; - config.preferred_driver = video.gpu.preferred_driver; - return config; + return { + .zoom = window.zoom, + .fullscreen = window.fullscreen, + .vsync = video.vsync, + .scaling_mode = video.scaling_mode, + .texture_filter = video.texture_filter, + .gpu_acceleration = video.gpu.acceleration, + .preferred_driver = video.gpu.preferred_driver, + }; } auto buildAudioConfig() -> ::Audio::Config {