JailDesignerandClaude Opus 4.7
b19a6e25da
- Registre de commits omplert amb els hashes reals dels 3 commits de la rama (a3b03b8/64e0178/d9f6fb4). - player-animations.md: el recompte de smoke tests deia "14/14" però `tests/test_entities_smoke.cpp` només invoca 13 funcions `test*` des de `main()`. Nota corregida a "13/13" amb explicació. Tanca els 4 ítems DEFERRED de `audit/game` (COUP-02, COUP-03, CONV-02 part Options, PATCH-01). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
113 lines
15 KiB
Markdown
113 lines
15 KiB
Markdown
# Auditoría `source/game/`
|
|
|
|
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) es tanquen a la branca `refactor/options-class` (registre de commits al final del fitxer).
|
|
|
|
Estados posibles:
|
|
- `PENDING` — identificat, sense tocar
|
|
- `WIP` — diff proposat, pendent d'aplicar
|
|
- `DONE` — aplicat i commiteat
|
|
- `WONTFIX` — es decideix no tocar (justificació a la nota)
|
|
- `DEFERRED` — es farà en fase posterior
|
|
|
|
Els IDs es mantenen estables una vegada assignats. Cada entrada apunta al fitxer i línies afectades en el moment del clone (branca `main`).
|
|
|
|
---
|
|
|
|
## Top 5 prioritari
|
|
|
|
| ID | Títol | Estat |
|
|
|----|-------|-------|
|
|
| HC-03 | Geometria del mapa cablejada a `map.hpp` (bloqueja "pirámide a pirámide") | DONE |
|
|
| COUP-01 | `core/` inclou `game/ui/notification_presets.hpp` (dependència invertida) | DONE |
|
|
| DUP-01 | PostFX i CrtPi són dos pipelines duplicats a `options.cpp` | DONE |
|
|
| PATCH-04 | Reescriure silenciosament `config.yaml` al fallar parseig o versió | DONE |
|
|
| HC-01+HC-02+HC-06 | `SPEED_PX_PER_S`, caixa de col·lisió i rutes d'assets cablejades | DONE |
|
|
|
|
---
|
|
|
|
## 1. Duplicació
|
|
|
|
| ID | Fitxer:línia | Troballa | Severitat | Estat | Notes |
|
|
|----|--------------|----------|-----------|-------|-------|
|
|
| DUP-01 | `options.cpp:152-335,443-547` | PostFX i CrtPi tenen `default*Presets/parse*Preset/write*Preset/load*FromFile/save*ToFile/set*File` duplicats | Importante | DONE | Afegits `loadPresetsFromFile<T>` i `savePresetsToFile<T>` a l'anonymous namespace; els 4 entry-points passen a ser one-liners que injecten parse/write/defaults per tipus. Els fitxers `postfx.yaml`/`crtpi.yaml` regenerats són idèntics a l'anterior. `std::cerr` manté el comportament — INCONS-02 el corregirà |
|
|
| DUP-02 | `player.cpp:100-140` | `snapToHorizontalLane` i `snapToVerticalLane` són el mateix algorisme amb eixos permutats | Importante | DONE | `snapToLane(dt, base, pitch, max_idx, get_pos, set_pos)` pren member-fn pointers; els dos wrappers deleguen només amb `Map::pitch_*`/`grid_*` distints |
|
|
| DUP-03 | `options.cpp:14-62` | Tres parelles `enumToString`/`enumFromString` amb la mateixa forma | Menor | DONE | Tres `constexpr std::array<std::pair<Enum, string_view>, N>` + helpers `enumToString`/`enumFromString` plantilla. La primera entrada és el fallback (documentat). També elimina HC-07 |
|
|
| DUP-04 | `options.cpp:79-149` | Cada `parse*` repeteix `if (node.contains) { ... get_value<T> }` | Menor | DONE | Afegit `readField<T>(node, key, dst)` plantilla a l'anonymous namespace. Aplicat a les 7 funcions `parse*` + `parsePostFXPreset`/`parseCrtPiPreset` |
|
|
| DUP-05 | `player.cpp:81-88` | Càlcul `MIN_/MAX_` + `clamp` per eix és la mateixa fórmula que el snap | Menor | DONE | Extret `Player::clampToPlayfield()`; `update()` el crida després de la col·lisió per eix |
|
|
|
|
## 2. Hardcoding
|
|
|
|
| ID | Fitxer:línia | Troballa | Severitat | Estat | Notes |
|
|
|----|--------------|----------|-----------|-------|-------|
|
|
| HC-01 | `player.cpp:15` | `SPEED_PX_PER_S = 60.0F` cablejat | Importante | DONE | Afegit `Defaults::Player::SPEED_PX_PER_S` a `utils/defaults.hpp` (patró usat a l'audit d'àudio); `player.cpp` fa alias local per reduir soroll. TODO-comment ("si més endavant volem sprint...") eliminat (PATCH-02) |
|
|
| HC-02 | `player.hpp:21-35` | `SPRITE_WIDTH/HEIGHT`, `LEGACY_BASE_SIZE`, `COLLISION_SIZE` cablejats | Importante | DONE | Bloc `collision:` afegit a `player.yaml` (size, base_size); `Player` exposa `inline static int sprite_*`/`base_size`/`collision_*` recalculats al `loadFrom()`. GameScene crida el load abans del spawn (mateix patró que Map) |
|
|
| HC-03 | `map.hpp:18-62` | Tota la geometria del mapa + 4 índexs de paleta en `.hpp` | Crítico | DONE | Bloc `geometry:` afegit a `pyramid.yaml`; `Map` exposa `inline static int` + `loadFrom(yaml)`; derivats (CORRIDOR/BOARD/ORIGIN/PLAYFIELD) recalculats al load. `constexpr` → `const` en els locals dependents a `player.cpp` i `game_scene.cpp`. Índexs de paleta queden per DEAD-02 |
|
|
| HC-04 | `notification_presets.hpp:24-38` | `text_color`, `bg_color`, `time_ms` en `constexpr` | Menor | DONE | Valors moguts a `Defaults::UI::{NOTIFICATION_*, EXIT_*}`; els `Preset` son alias. Tocar "tema" global es fa a un sol lloc |
|
|
| HC-05 | `ui_palette.hpp:12-21` | Índexs de paleta (`RENDER_INFO_TEXT=47`, `SHADOW=34`, `CANVAS_BG=65`) en `constexpr` | Menor | DONE | Valors moguts a `Defaults::UI`; `UiColor::*` queda com a alias exportat al joc |
|
|
| HC-06 | `game_scene.cpp:24-26,52` | Rutes `data/maps/pyramid.yaml`, `data/player/player.*`, `pyramid_1.ogg` cablejades | Importante | DONE | Afegit `Defaults::Pyramid::{MAP_YAML,PLAYER_SPRITE,PLAYER_YAML,MUSIC_TRACK}`; `GameScene::Config` struct + `defaultConfig()` passa les rutes al ctor (default ve de Defaults). `Player` ctor accepta `anim_key` en lloc de cablejar `"player.yaml"`. Cache key = basename (mateix conveni que abans) |
|
|
| HC-07 | `options.cpp:27,55` | Fallbacks `"letterbox"`/`"off"` com a strings màgics que dupliquen els enums | Menor | DONE | Resolt amb DUP-03: la primera entrada de cada taula és el fallback i només apareix una vegada |
|
|
| HC-08 | `tilemap.cpp:135` | `0xFF000000U` com a màscara alfa | Menor | DONE | `constexpr std::uint32_t ALPHA_MASK = 0xFF000000U` local amb comentari. No s'ha mogut a defines.hpp perquè només s'usa dins de `Tilemap::render` |
|
|
|
|
## 3. Parxes i improvisacions
|
|
|
|
| ID | Fitxer:línia | Troballa | Severitat | Estat | Notes |
|
|
|----|--------------|----------|-----------|-------|-------|
|
|
| PATCH-01 | `player.cpp:66-77` | Col·lisió per eix fent `AnimatedSprite::update` + revertir + reaplicar | Importante | DEFERRED | Afegir `tryMove(dx, dy, collisionFn)` a la classe base toca `core/rendering/sprite/` i canvia l'API de tots els descendents. Millor lligar-ho al refactor de COUP-02/03 (`refactor/options-class` / MapInfo injectat al Player) per no dispersar el canvi |
|
|
| 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 | 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 `<path>.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 |
|
|
| PATCH-07 | `map.hpp:64` + `map.cpp:19` | `Map` declara ctor però tots els mètodes són `static` (transició a mitges) | Importante | DONE | `Map` passa a ser `namespace Map`; callers no canvien (ja usen `Map::grid_cols`/`Map::collides`). Pattern alineat amb `Options` |
|
|
|
|
## 4. Acoplament innecessari
|
|
|
|
| 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 | 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 | 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. 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 |
|
|
| 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
|
|
|
|
| ID | Fitxer:línia | Troballa | Severitat | Estat | Notes |
|
|
|----|--------------|----------|-----------|-------|-------|
|
|
| DEAD-01 | `map.hpp:69,73,78` | `renderBackground`/`renderSarcsRow`/`getSarcRowBottomY` no tenen callers | Importante | DONE | Eliminats; el tilemap fa el render visual i els sarcs seran actors sprite. `getSarcRect` es queda (la usa `collides`) |
|
|
| DEAD-02 | `map.hpp:59-62` | Índexs `HUD_BG/WALL/FLOOR/SARC` només usats pel codi mort | Importante | DONE | Eliminats amb DEAD-01 |
|
|
| DEAD-03 | `map.cpp:3-5` | Includes `screen.hpp/resource_cache.hpp/defines.hpp` orfes si es neteja DEAD-01 | Menor | DONE | Eliminats amb DEAD-01 |
|
|
| DEAD-04 | `map.hpp:64` + `map.cpp:19` | Ctor de `Map` mai instanciat | Menor | DONE | Ja eliminat durant HC-03 |
|
|
| DEAD-05 | `game_scene.cpp:78-86` | Ordenació Z-order cerimonial (1 jugador, sense sarcs-sprite) | Menor | DONE | Resolt amb PATCH-06 |
|
|
|
|
## 7. Violacions de convencions
|
|
|
|
| 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 | 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).
|
|
|
|
- `a3b03b8` — 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`)
|
|
- `64e0178` — COUP-02: `Map::Info` plana injectada al `Player` (només Player; momia/roca/loadlevel resten amb `Map::*` global, com diu la nota original del audit)
|
|
- `d9f6fb4` — PATCH-01: `tryMove(dx, dy, CollisionFn)` a `MovingSprite` + hook virtual `collisionBox()` a `Sprite`; `Player::update` deixa de fer `update + revertir + reaplicar` i passa a `animate(dt) + tryMove`
|