Files
orni-attack/CODE_REVIEW.md
T
JailDesigner 9e54dde490 docs: añadir CODE_REVIEW.md con auditoría arquitectónica (40 hallazgos)
Reporte completo de la pasada de revisión arquitectónica realizada por
el subagente Plan tras cerrar el ciclo de lint. Organiza los 40
hallazgos en 8 áreas (escenas, sistemas, entidades, renderizado,
configuración, naming, headers, bugs latentes), cada uno con prioridad,
tipo (estructural/opinable), file:line, problema, propuesta y esfuerzo
estimado.

Incluye:
- Top picks de bugs latentes y limpieza con impacto
- Lista de hallazgos que requieren verificación profunda
- Lista de hallazgos opinables (potencialmente rechazables)
- Resumen de lo ya resuelto en chore/lint
- Plan de ataque sugerido para iterar

Permite continuar el code review en otro equipo sin perder el contexto.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 16:13:18 +02:00

26 KiB
Raw Blame History

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 GameSceneSystems::* 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).