orni-attack/CODE_REVIEW.md

# Auditoría arquitectónica — Orni Attack

> Pasada de revisión de **diseño y arquitectura** (no de estilo) sobre la rama `main` tras cerrar el ciclo de lint (cppcheck/clang-tidy a 0 hits).
> Generada el 2026-05-20 por el subagente `Plan` y verificada parcialmente.
> Las referencias `file:line` son relativas a la raíz del repo.

## Cómo usar este documento

- **Prioridad** indica urgencia subjetiva (alta/media/baja).
- **Tipo** distingue:
  - **[estructural]** = bug latente, deuda técnica medible, código muerto
  - **[opinable]** = cambio de estilo/diseño que el usuario puede rechazar legítimamente
- **Esfuerzo**: small (1 commit < 30 min) / medium (1-3 commits) / large (varias sesiones).

Antes de actuar sobre un hallazgo, **re-verificar** que sigue siendo válido (el código habrá cambiado).

---

## Hallazgos por área

### A. Escenas

#### 1. `Scene` no declara `init()`, pero `GameScene` lo expone — [PRIORIDAD: alta, estructural]
- **Dónde**: `source/core/system/scene.hpp:31-40`, `source/game/scenes/game_scene.hpp:47`, `source/game/scenes/game_scene.cpp:64` (`init()` se llama desde el ctor).
- **Problema**: La interfaz `Scene` declara `handleEvent/update/draw/isFinished` como obligatorios. `GameScene::init()` es un método público adicional, pero el Director nunca lo llama — `GameScene` lo invoca desde su propio ctor. El comentario del header dice "init() — llamado por Director tras crear la escena", pero eso no ocurre. `TitleScene` y `LogoScene` no tienen ese método: hacen todo en el ctor. Inconsistencia de lifecycle.
- **Propuesta**: (a) eliminar `init()` y mover su contenido al ctor (lo más simple), o (b) añadirlo a la interfaz `Scene` y llamarlo desde el Director tras `buildScene`. Lo coherente es (a).
- **Esfuerzo**: small

#### 2. No existe clase base `MenuScene` para el patrón `timer + state-machine + skip-button` — [PRIORIDAD: media, opinable]
- **Dónde**: `source/game/scenes/title_scene.cpp:368-388`, `source/game/scenes/logo_scene.cpp:219-280`, partes de `game_scene.cpp` (INIT_HUD/LEVEL_START).
- **Problema**: Las tres escenas implementan la misma idea: un enum `State`, un timer acumulado, un `switch` por estado, una transición "skip". `checkSkipButtonPressed()` aparece en `TitleScene` y `LogoScene` con la misma signatura estática.
- **Propuesta**: Mixin pequeño `TimedStateScene<EnumT>` con `enterState(EnumT)`, `time_in_state()`, helpers de "esperar N segundos y avanzar". Al menos consolidar `checkSkipButtonPressed()` y la pareja `temps_estat_actual_ + cambio de estado` en un helper.
- **Esfuerzo**: medium

#### 3. `TitleScene::initTitle()` duplica dos veces el cálculo de bounding box + posicionado — [PRIORIDAD: alta, estructural]
- **Dónde**: `source/game/scenes/title_scene.cpp:104-254` (función de 150 líneas).
- **Problema**: Carga "ORNI" (108-166) y "ATTACK!" (178-241) con el mismo código copiado. `inicialitzarJailgames()` (256-325) hace lo mismo por tercera vez. Tres copias del cómputo de bounding box.
- **Propuesta**: Extraer `loadLetterRow(filenames, scale, y_position, espai) -> std::vector<LetraLogo>`.
- **Esfuerzo**: small

#### 4. `PLAYER_JOIN_PHASE` en TitleScene mezcla animación + audio + late join — [PRIORIDAD: media, opinable]
- **Dónde**: `source/game/scenes/title_scene.cpp:451-483`.
- **Problema**: `updatePlayerJoinPhaseState` hace varias cosas no relacionadas. La lógica de "late join" reaprovecha `checkStartGameButtonPressed()` (que *muta* `match_config_` — efecto secundario no advertido por la firma).
- **Propuesta**: Renombrar `checkStartGameButtonPressed()` a `tryActivatePlayersOnStart()`. Separar la lógica de "late join" en un helper.
- **Esfuerzo**: small

#### 5. `SceneContext::actual` es estado global redundante — [PRIORIDAD: baja, opinable]
- **Dónde**: `source/core/system/scene_context.hpp:81`, escrita en `director.cpp:267,275` y `global_events.cpp:29,58`.
- **Problema**: `inline SceneContext::SceneType actual` como variable global, sincronizada manualmente. No la consulta nadie excepto el propio Director (que lee `context.nextScene()`, no `actual`).
- **Propuesta**: Verificación profunda. Si nadie la consulta, eliminarla.
- **Esfuerzo**: small

### B. Sistemas

#### 6. `StageManager::update(dt, pause_spawn)` tiene `pause_spawn` muerto — [PRIORIDAD: alta, estructural]
- **Dónde**: `source/game/stage_system/stage_manager.hpp:28`, `source/game/stage_system/stage_manager.cpp:128-135`, `game_scene.cpp:359-374,423-424`.
- **Problema**: El parámetro `pause_spawn` existe en la API pero ningún caller lo pasa con sentido — la pausa real ocurre por otra ruta (`spawn_controller_.update(dt, enemies, PAUSE_SPAWN)`). El comentario en `stage_manager.cpp:131-134` ya admite que no se usa.
- **Propuesta**: Quitar el parámetro.
- **Esfuerzo**: small

#### 7. `SpawnController` hardcoda `std::array<Enemy, 15>` en su firma — [PRIORIDAD: media, estructural]
- **Dónde**: `source/game/stage_system/spawn_controller.hpp:33,37,39`; `.cpp:44,89,96`.
- **Problema**: Literal `15` en lugar de `Defaults::Entities::MAX_ORNIS`. Acoplamiento silencioso por número.
- **Propuesta**: Usar `std::array<Enemy, Defaults::Entities::MAX_ORNIS>&` o un alias `using EnemyArray = ...;`.
- **Esfuerzo**: small

#### 8. `Systems::Collision::Context` y `Systems::ContinueScreen::Context` son "god structs" — [PRIORIDAD: alta, estructural]
- **Dónde**: `source/game/systems/collision_system.hpp:31-43`, `source/game/systems/continue_system.hpp:25-38`.
- **Problema**: Cada `Context` agrupa entre 9 y 12 referencias mutables a estado de `GameScene` + un `std::function` callback. Punto único donde cualquier cambio de campo en GameScene rompe el módulo. Los sistemas pueden mutar todo (no hay distinción "lo que leo" vs "lo que modifico"). La separación `GameScene` ↔ `Systems::*` es nominal.
- **Propuesta**: Que `detectAll` devuelva una struct "outcome" (qué balas se desactivaron, qué enemigos murieron, qué jugador fue hit) y que GameScene aplique el resultado. Si es demasiado, al menos partir el Context en `In` (lectura) e `Out` (escritura).
- **Esfuerzo**: large

#### 9. `CollisionSystem` y `PhysicsWorld` se solapan — [PRIORIDAD: media, opinable]
- **Dónde**: `source/game/systems/collision_system.cpp` (todo), `source/core/physics/physics_world.cpp:118-179`.
- **Problema**: `PhysicsWorld::resolveBodyCollisions` resuelve colisiones cuerpo-cuerpo con impulsos elásticos; `CollisionSystem::detectBulletEnemy` itera otra vez los mismos pares con un *amplifier* distinto y aplica daño. Dos bucles O(n²) sobre el mismo conjunto. El comentario en `game_scene.cpp:159` explica la sutileza pero es frágil.
- **Propuesta**: Verificación profunda. ¿Deberían las balas vivir fuera del world directamente?
- **Esfuerzo**: medium

#### 10. `init_hud_animator` es solo 3 funciones libres en un namespace — [PRIORIDAD: baja, opinable]
- **Dónde**: `source/game/systems/init_hud_animator.hpp`, `.cpp` (99 líneas).
- **Problema**: A diferencia de `CollisionSystem`/`ContinueSystem`, no tiene `Context`; son 3 funciones puras. La nomenclatura sugiere algo más grande.
- **Propuesta**: Mover a `source/core/rendering/` o renombrar `systems/` a `gameplay/`.
- **Esfuerzo**: small

### C. Entidades

#### 11. `Ship` expone tres métodos equivalentes: `isAlive()`, `isHit()`, `isActive()` — [PRIORIDAD: alta, estructural]
- **Dónde**: `source/game/entities/ship.hpp:27,38,39`.
- **Problema**: `isActive() = !is_hit_`, `isAlive() = !is_hit_`, `isHit() = is_hit_`. Tres formas de preguntar lo mismo, usadas en sitios distintos.
- **Propuesta**: Quedarse con `isActive()` (override de Entity) y reescribir los call-sites.
- **Esfuerzo**: small

#### 12. `Entity::renderer_` raw pointer; ctor por defecto deja la entidad inservible — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/entities/entity.hpp:56,67`, `source/game/entities/ship.hpp:15-16` (`Ship() : Entity(nullptr) {}`), idem en `enemy.hpp:38-39`, `bullet.hpp:15-16`.
- **Problema**: El ctor por defecto existe solo para soportar `std::array<Ship, 2>`, pero deja `renderer_` nullptr. Si alguien dibuja antes de la reasignación, segfault. `renderer_` no tiene in-class initializer.
- **Propuesta**: (a) Inicializador en miembro `Rendering::Renderer* renderer_{nullptr};`. (b) Guard en `draw()` para `renderer_ == nullptr`. (c) Considerar `std::array<std::optional<Ship>, 2>`.
- **Esfuerzo**: small (a)+(b); medium (c)

#### 13. Duplicación de estado: `center_/angle_` en Entity vs `body_.position/body_.angle` en RigidBody — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/entities/entity.hpp:58-59,64`; `source/game/entities/ship.cpp:131-135` (postUpdate sincroniza), `enemy.cpp:214-219`, `bullet.cpp:121-125`.
- **Problema**: Cada entidad mantiene `center_/angle_` como mirror del body. Cualquier código que lea `getCenter()` durante `update()` (antes del `postUpdate`) verá el valor del frame anterior. El patrón está documentado pero es frágil.
- **Propuesta**: Hacer que `getCenter()` devuelva `body_.position` directamente (eliminando `center_`).
- **Esfuerzo**: medium

#### 14. `Entity::renderer_` `protected:` con acceso directo + getter por referencia mutable a `body_` — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/entities/entity.hpp:51-52` (`getBody() -> RigidBody&`), `:54-64`.
- **Problema**: Encapsulación abierta + getters que devuelven referencias no-const al `body_`. El contrato de quién muta `body_.velocity`/`position` no está claro.
- **Propuesta**: Documentar el contrato. Hacer `body_` privado con API estricta (`registerInWorld(world)`).
- **Esfuerzo**: medium

#### 15. `Enemy` tiene `drotacio_/rotacio_` (rotación visual) Y `body_.angle/angular_velocity` — [PRIORIDAD: baja, estructural]
- **Dónde**: `source/game/entities/enemy.hpp:92-93`, `.cpp:210-212`.
- **Problema**: El body de un enemy nunca rota (angular_velocity siempre 0), pero la visual sí. Dos representaciones de orientación viviendo en la misma entidad.
- **Propuesta**: Eliminar `body_.angle/angular_velocity` para enemies (no se usa).
- **Esfuerzo**: small

### D. Renderizado y recursos

#### 16. `Rotation3D` (struct + parámetro en `renderShape`) es código muerto — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/rendering/shape_renderer.hpp:18-35,52`, `.cpp:13-78`.
- **Problema**: La función `renderShape` acepta `const Rotation3D* rotation_3d = nullptr`. **Todos** los callers pasan `nullptr` (`ship.cpp:161`, `enemy.cpp:232`, `bullet.cpp:137`). 28 líneas de código muerto.
- **Propuesta**: Eliminar `Rotation3D`, el parámetro y el branch.
- **Esfuerzo**: small

#### 17. `Resource::Helper` es una fachada delgada sobre `Resource::Loader` — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/resources/resource_helper.hpp`, `.cpp` (80 líneas); `resource_loader.hpp` (54 líneas).
- **Problema**: `Helper::loadFile()` es `Loader::get().loadResource(normalizePath(...))`. La única "lógica real" de Helper es `normalizePath()`. Duplicación de API.
- **Propuesta**: Mover `normalizePath` a `Resource::Loader` y eliminar `Helper`.
- **Esfuerzo**: small

#### 18. `ShapeLoader::resolvePath` parece código muerto — [PRIORIDAD: baja, opinable]
- **Dónde**: `source/core/graphics/shape_loader.hpp:36`, `.cpp:70-83`.
- **Problema**: `resolvePath` está declarada como helper privado, pero no se llama desde ningún sitio.
- **Propuesta**: Eliminar `resolvePath` y `BASE_PATH`.
- **Esfuerzo**: small

#### 19. `ShapeLoader::cache` sin política de invalidación — [PRIORIDAD: baja, estructural]
- **Dónde**: `source/core/graphics/shape_loader.cpp:13,15-60`.
- **Problema**: Caché estática `unordered_map<string, shared_ptr<Shape>>`. `clearCache()` existe pero nadie la llama. Sin límite de tamaño.
- **Propuesta**: Documentar en el header que la caché vive toda la vida del proceso.
- **Esfuerzo**: small

#### 20. `GpuFrameRenderer::device()` y `isInsideFrame()` son getters públicos sin callers — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/rendering/gpu/gpu_frame_renderer.hpp:96,97`. Similar `SDLManager::getScaleFactor()` (línea 42).
- **Problema**: API pública sin clientes.
- **Propuesta**: Eliminar o documentar como API reservada.
- **Esfuerzo**: small

### E. Configuración

#### 21. `Options::physics`, `Options::audio`, `Options::gameplay` se cargan desde YAML pero nunca se consultan en runtime — [PRIORIDAD: alta, estructural]
- **Dónde**:
  - `Options::physics` se rellena en `options.cpp:194-201`, parse 286-291, write 597-603. Único call-site lector: `director.cpp:109` (informativo). Ship/Enemy/Bullet leen `Defaults::Physics::*` directamente.
  - `Options::audio` documentado como "deliberadamente desacoplado" (`audio.hpp:22-25`); `director.cpp:235-244` construye `Audio::Config` desde `Defaults::Audio`.
  - `Options::gameplay` (max_enemies, max_bullets): las arrays son compile-time (`std::array<Enemy, MAX_ORNIS>`).
- **Problema**: El usuario edita su `config.yaml`, cambia `physics.rotation_speed: 6.0`, reinicia el juego — el juego sigue usando el default. Prometemos configurabilidad que no entregamos.
- **Propuesta**: O bien (a) conectar Options→runtime, o (b) eliminar los campos muertos. Híbrido: borrar `audio`/`gameplay`, conectar `physics` (apoya Fase 10).
- **Esfuerzo**: small (eliminar) o medium (conectar correctamente).

#### 22. `defaults.hpp` mezcla constantes de cinco dominios distintos en un solo archivo de 530 líneas — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/defaults.hpp` (todo).
- **Problema**: Contiene Window, Game, Zones, Entities, Palette, Ship, Physics, Math, Brightness, Rendering, Audio, Music, Sound, Controls, Enemies (con sub-namespaces), Title, FloatingScore. **22 archivos** lo incluyen. Editar una constante de Audio fuerza recompilación de todo. Dos `namespace Game` separados (líneas 22-25 y 132-210).
- **Propuesta**: Partir en `defaults/window.hpp`, `defaults/game.hpp`, `defaults/zones.hpp`, etc. Con `defaults.hpp` como umbrella.
- **Esfuerzo**: medium

#### 23. `Defaults::Window::WIDTH = 1280` y `Defaults::Game::WIDTH = 1280` son duplicados sin relación enforced — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/defaults.hpp:10` vs `:23`.
- **Problema**: Dos `WIDTH` distintos pero con el mismo valor. Nadie obliga a que coincidan. Si alguien quisiera arrancar a 1366×768 cambiando `Window::WIDTH`, el cálculo `zoom_factor = (float)width / Window::WIDTH` (`sdl_manager.cpp:101`) usa `Window::WIDTH`, no `Game::WIDTH` — bug latente.
- **Propuesta**: Decidir la fuente de verdad. Si es `Game::{WIDTH,HEIGHT}`, derivar `Window::WIDTH = Game::WIDTH`.
- **Esfuerzo**: small

#### 24. `game/constants.hpp` contiene aliases legacy mayoritariamente muertos — [PRIORIDAD: baja, estructural]
- **Dónde**: `source/game/constants.hpp:10-23`.
- **Problema**: `Constants::MARGIN_*`, `MAX_ORNIS`, `MAX_BALES`, `VELOCITAT`, `VELOCITAT_MAX` — no se usan. Solo se usan los helpers (PI, isInPlayArea, getSafePlayAreaBounds, getPlayAreaCenter).
- **Propuesta**: Eliminar las constantes muertas.
- **Esfuerzo**: small

#### 25. `Defaults::Physics::{ENEMY_SPEED, BULLET_SPEED, VELOCITY_SCALE}` legacy de Pascal son código muerto — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/defaults.hpp:218-220`.
- **Problema**: Los valores están en unidades/frame, comentados como "actuales del juego", pero la migración a SDL3 los reemplazó (`bullet.cpp:22` define su propio `BULLET_SPEED = 140.0F`; enemy speeds vienen de `Defaults::Enemies::{Pentagon,Cuadrado,Molinillo}::VELOCITAT`).
- **Propuesta**: Eliminar.
- **Esfuerzo**: small

### F. Naming y consistencia de idioma

#### 26. Variables del estado de juego mezclan catalán y castellano — [PRIORIDAD: baja, opinable]
- **Dónde**: `source/game/scenes/game_scene.hpp:67-77`, `source/game/scenes/title_scene.hpp:65-84`.
- **Problema**: Conviven `hit_timer_per_player_` (inglés), `lives_per_player_` (inglés), `match_config_.jugador1_actiu` (catalán). En TitleScene: `estat_actual_` (catalán) junto a `factor_lerp_`. No hay convención clara dentro de la misma clase.
- **Propuesta**: Si el usuario quiere preservar el sabor, reglar: identificadores expuestos en headers públicos en catalán, detalles internos en inglés.
- **Esfuerzo**: medium

#### 27. Dos namespaces `Constants` no relacionados — [PRIORIDAD: baja, opinable]
- **Dónde**: `source/game/constants.hpp:8` (`namespace Constants`) y `source/game/stage_system/stage_config.hpp:82` (`namespace StageSystem::Constants`).
- **Problema**: Mismo nombre, scopes distintos. Lecturalmente confuso.
- **Propuesta**: Renombrar el del stage system a `StageSystem::Messages`.
- **Esfuerzo**: small

### G. Headers y dependencias

#### 28. `core/` depende de `game/` — violación de capas — [PRIORIDAD: alta, estructural]
- **Dónde**:
  - `source/core/system/director.cpp:25-28` incluye `game/scenes/*.hpp` y `game/options.hpp`
  - `source/core/rendering/sdl_manager.cpp:16` incluye `game/options.hpp`
  - `source/core/system/debug_overlay.cpp:8` incluye `game/options.hpp`
  - `source/core/input/input.cpp:11` incluye `game/options.hpp`
- **Problema**: La estructura sugiere un layered design, pero `core/` lee y muta `game::Options`. Si se intenta convertir `core/` en librería estática reusable, no compila.
- **Propuesta**: Mover `Options` a `source/core/config/`, o invertir la dependencia con `IConfigProvider*`. Relacionado con #21.
- **Esfuerzo**: medium

#### 29. `continue_system.hpp` forward-declara `enum class GameOverState` definida a nivel global en `game_scene.hpp` — [PRIORIDAD: media, estructural]
- **Dónde**: `source/game/scenes/game_scene.hpp:28-32` (declaración en namespace global, NO dentro de la clase), `source/game/systems/continue_system.hpp:18-20`.
- **Problema**: Si la subyacente cambia de `uint8_t` a `int`, las dos declaraciones divergen silenciosamente.
- **Propuesta**: Mover `GameOverState` a `source/game/systems/game_over_state.hpp` (header de tipos puros).
- **Esfuerzo**: small

#### 30. `core/defaults.hpp` se incluye en 22 archivos arrastrando ~530 líneas — [PRIORIDAD: media, estructural]
- **Dónde**: Ver #22.
- **Problema**: Tiempo de compilación incremental innecesariamente alto.
- **Propuesta**: Ver #22.
- **Esfuerzo**: medium

### H. Bugs latentes / code smells

#### 31. `hit_timer_per_player_` usa el valor mágico `999.0F` como sentinela "jugador permanentemente inactivo" — [PRIORIDAD: alta, estructural]
- **Dónde**: `source/game/scenes/game_scene.cpp:140,229,230,239,312,332`.
- **Problema**: El timer combina tres estados en un float: `== 0.0F` = vivo, `> 0 && < 3.0` = secuencia de muerte, `== 999.0F` = sin vidas. Si en algún momento se decrementa accidentalmente, el sentinela se pierde.
- **Propuesta**: Introducir un enum `PlayerStatus { ALIVE, DYING, OUT }` separado del timer; el timer solo cuenta `DYING`.
- **Esfuerzo**: medium

#### 32. `std::rand()` en todo el gameplay; `std::mt19937` solo en LogoScene; reseed solo en GameScene::init — [PRIORIDAD: media, estructural]
- **Dónde**: `std::srand` en `game_scene.cpp:80`; `std::rand` en `spawn_controller.cpp:131`, `enemy.cpp:133-447`, `stage_manager.cpp:84`, `debris_manager.cpp:121-352`. `std::mt19937` en `logo_scene.cpp:171-173`.
- **Problema**: Mezcla de PRNG. `std::rand()` tiene calidad mala y es global. `srand` solo se llama una vez por partida; el logo y el título usan la seed por defecto. Repetibilidad accidental.
- **Propuesta**: Centralizar un `Random` (single mt19937 con seeds explícitas). Al menos `srand` en main/Director para que se siembre antes.
- **Esfuerzo**: small (centralizar srand) o medium (reemplazar todos).

#### 33. `LineRenderer` mantiene dos globales mutables: `g_current_line_color` y `g_current_line_thickness` — [PRIORIDAD: media, estructural]
- **Dónde**: `source/core/rendering/line_renderer.cpp:11,15`.
- **Problema**: Estado mutable global en la TU. Si alguna escena cambia color y se olvida de restaurarlo, contamina las siguientes. Misma problemática con `g_current_scale_factor` en `coordinate_transform.cpp:9`.
- **Propuesta**: Moverlos a miembros del `GpuFrameRenderer`. El color global puede vivir como uniform de pipeline.
- **Esfuerzo**: medium

#### 34. `GameScene::init()` reasigna `enemies_[i] = Enemy(...)` duplicando el trabajo del constructor — [PRIORIDAD: alta, estructural]
- **Dónde**: `source/game/scenes/game_scene.cpp:56-59` (ctor) vs `:149-153` (init).
- **Problema**: En el ctor se hace `enemy = Enemy(sdl.getRenderer())`. Inmediatamente después se llama `init()` (línea 64), que reasigna otra vez. Trabajo perdido. Inconsistencia con bullets (que solo se inicializan una vez).
- **Propuesta**: Eliminar la inicialización en el ctor o en init. Decidir cuál es la fuente única.
- **Esfuerzo**: small

#### 35. `runCollisionDetections()` reconstruye un `Context` enorme cada frame — [PRIORIDAD: media, estructural]
- **Dónde**: `source/game/scenes/game_scene.cpp:470-484`, similar `stepContinueScreen` (`:252-282`).
- **Problema**: Cada frame en PLAYING se construye una struct con 9 referencias + un `std::function` (que aloca en heap por la captura `[this]`). 60 allocs/segundo del `std::function`.
- **Propuesta**: Cachear el Context como miembro o pasar el callback como puntero a función miembro.
- **Esfuerzo**: small

#### 36. `SpawnController::generateSpawnEvents` precomputa toda la cola pero `update()` solo procesa secuencialmente — [PRIORIDAD: media, estructural]
- **Dónde**: `source/game/stage_system/spawn_controller.cpp:54-82`.
- **Problema**: El flag `event.spawnejat` parece sobrante (el index ya marca progreso).
- **Propuesta**: Verificación profunda. Simplificable.
- **Esfuerzo**: small

#### 37. `Director::run()` es estático pero usa estado de instancia via singletons — [PRIORIDAD: baja, estructural]
- **Dónde**: `source/core/system/director.hpp:21-25`, `.cpp:41-48,150`.
- **Problema**: `executable_path_` se asigna en ctor pero nadie lo lee. `system_folder_` se usa en ctor pero no después. El patrón "Director con state" es decorativo.
- **Propuesta**: O hacer `run()` no-estática, o eliminar los miembros y mover `run()` a una función libre `App::main()`.
- **Esfuerzo**: small

### Otros

#### 38. `MatchConfig::jugador{1,2}_actiu` mutación desde dos sistemas independientes sin notificar — [PRIORIDAD: media, estructural]
- **Dónde**: `source/game/scenes/game_scene.cpp:912-916` (joinPlayer), `source/game/systems/continue_system.cpp:29-33` (revivePlayer).
- **Problema**: `MatchConfig` viaja como referencia mutable por los `Context` y se modifica desde múltiples sistemas. Rompe la intuición de que `MatchConfig` es "la configuración decidida en TitleScene".
- **Propuesta**: Renombrar el campo a `player_active_runtime` o mover el flag a otro lado (ej. dentro de `lives_per_player_`: si lives > 0 → activo).
- **Esfuerzo**: medium

#### 39. Inconsistencia: `LogoScene::~LogoScene()` llama `stopAllSounds()`, `TitleScene::~TitleScene()` llama `stopMusic()`, `GameScene::~GameScene()` es default — [PRIORIDAD: baja, opinable]
- **Dónde**: `source/game/scenes/logo_scene.cpp:62-66`, `title_scene.cpp:99-102`, `game_scene.hpp:38`.
- **Problema**: Cada escena gestiona limpieza de audio a su manera. Si una olvida un sonido en loop, fuga.
- **Propuesta**: Decidir si el Director debe llamar `Audio::stopAllSounds()` y `stopMusic()` antes de destruir cada escena.
- **Esfuerzo**: small

#### 40. `Bullet::BULLET_SPEED = 140.0F` en anonymous namespace, no en `defaults.hpp` — [PRIORIDAD: baja, opinable]
- **Dónde**: `source/game/entities/bullet.cpp:22-23`.
- **Problema**: Constante de gameplay escondida en una TU privada. Si alguien quiere tunear, mira en defaults.hpp y no la encuentra (ahí hay otro `BULLET_SPEED` diferente y muerto, ver #25).
- **Propuesta**: Mover a `Defaults::Entities::BULLET_SPEED_PXS`; eliminar el legacy de Pascal.
- **Esfuerzo**: small

---

## Resumen ejecutivo

### Top picks "bug latente / deuda clara"

| # | Hallazgo | Esfuerzo |
|---|---|---|
| **21** | `Options::physics/audio/gameplay` se cargan del YAML pero nadie las consulta | small-medium |
| **28** | `core/` depende de `game/` (4 ficheros incluyen `game/options.hpp`) | medium |
| **11** | `Ship::isAlive() == isActive() == !isHit()` (tres alias del mismo bool) | small |
| **34** | `GameScene` construye `enemies_` dos veces (en ctor y en init) | small |
| **31** | `hit_timer_per_player_ = 999.0F` como sentinela | medium |
| **16** | `Rotation3D` es código muerto (28 líneas que nunca se ejecutan) | small |
| **1** | `GameScene::init()` no encaja en el lifecycle de `Scene` | small |

### Top picks "limpieza con impacto"

| # | Hallazgo | Esfuerzo |
|---|---|---|
| **22** | `defaults.hpp` (530 líneas, 22 includes) dividido en subarchivos | medium |
| **13** | `Entity::center_/angle_` mirror de `body_.position/angle` | medium |
| **25** | Constantes legacy del Pascal muertas en `Defaults::Physics` | small |
| **18** | `ShapeLoader::resolvePath` + `BASE_PATH` sin callers | small |

### Hallazgos que requieren verificación más profunda antes de actuar
- **#5** (`SceneManager::actual`)
- **#9** (separación CollisionSystem/PhysicsWorld)
- **#20** (getters dead public)
- **#36** (`event.spawnejat`)

### Hallazgos puramente opinables que el usuario podría rechazar legítimamente
- **#2, #4, #10, #14, #15, #19, #26, #27, #37, #39, #40**

### Hallazgos estructurales claros (bug latente o deuda técnica medible)
- **#1, #6, #11, #12, #16, #21, #28, #31, #34**

---

## Ya identificados y resueltos en la rama `chore/lint` (no incluidos arriba)
- Patrón draw-by-state de `GameScene` (refactorizado: complejidad 59 → 3)
- Complejidad de `TitleScene::update` (68 → 4)
- Duplicado `LetraLogo` Title/Logo
- Double `isActive()` en Bullet (redeclaración tras rename)
- Refactor de `DebrisManager::explode` (39 → 3)
- `Options::loadXXXConfigFromYaml` boilerplate (con templates `readField`)
- `PhysicsWorld::resolveBodyCollisions` (35 → 5)

## Plan de ataque sugerido

1. **#21** (en curso): borrar `Options::audio` y `Options::gameplay`, conectar `Options::physics` a runtime
2. **#16, #18, #24, #25** (dead code, commits limpios y rápidos)
3. **#34** (eliminar doble inicialización de `enemies_`)
4. **#11** (consolidar `isActive/isAlive/isHit`)
5. **#1** (limpiar lifecycle de `Scene`)
6. **#22 + #30** (partir `defaults.hpp` en subarchivos)
7. **#28** (mover `Options` a `core/`)
8. **#31** (enum `PlayerStatus` reemplazando 999.0F)
9. **#13** (eliminar mirror `center_/angle_`)

A partir de ahí, decidir si seguir con el resto o pasar a otras prioridades (SDL_CALLBACKS, tuning de físicas).