JailDesignerandClaude Opus 4.7
964ff104ce
playSound (x2), stopAllSounds, fadeOutMusic, setSoundVolume i setMusicVolume es declaraven const però mutaven l'estat global de Ja::* (channels[], outgoing_music, music_volume, sound_volume[]). El const no protegia res: el compilador no detectava les mutacions perquè anaven a globals, no a membres, i podia induir a error a qui llegia el header. Els callers externs van via Audio* (no Audio const*), així que el canvi és source-compatible. Clang-tidy falla amb readability-make-member-function-const perquè tampoc veu la mutació — és exactament el símptoma que motiva el hallazgo. S'afegeix NOLINT localitzat a cada mètode amb la raó. Quan s'executi COUP-01 (encapsular Ja::* en classe membre) els NOLINT cauran sols perquè la mutació serà visible al compilador. effectiveVolume es queda const — és un càlcul pur sobre els membres. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
96 lines
8.6 KiB
Markdown
96 lines
8.6 KiB
Markdown
# Auditoría `core/audio/`
|
|
|
|
Rama: `audit/core-audio`
|
|
Estado global: en progreso.
|
|
|
|
Estados posibles:
|
|
- `PENDING` — identificado, sin tocar
|
|
- `WIP` — diff propuesto, pendiente de aplicar
|
|
- `DONE` — aplicado y commiteado
|
|
- `WONTFIX` — se decide no tocar (justificación en la nota)
|
|
- `DEFERRED` — se hará en fase posterior
|
|
|
|
Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fichero(s) y líneas afectados en la rama de origen (`main` en el momento del clone).
|
|
|
|
---
|
|
|
|
## Top 5 prioritario
|
|
|
|
| ID | Título | Estado |
|
|
|----|--------|--------|
|
|
| COUP-01 | Encapsular globals `Ja::` en clase (engine sin estado global) | PENDING |
|
|
| HC-02 | Formato/canales/freq hardcodeado en 3+ sitios | DONE |
|
|
| INCONS-02 | Doble `MusicState` + `getRealMusicState` como cache paralelo | PENDING |
|
|
| HC-01 | Parámetros de audio no configurables (viola convención del proyecto) | DONE |
|
|
| PATCH-01 | `toggleEnabled/toggleSound/toggleMusic` no reaplican volumen | DONE |
|
|
|
|
---
|
|
|
|
## 1. Duplicación
|
|
|
|
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|
|
|----|---------------|----------|-----------|--------|-------|
|
|
| DUP-01 | `audio.cpp:58-101` | Dos `playMusic` (nombre/puntero) repiten el bloque crossfade/stop+play | Menor | DONE | `playMusicInternal(Ja::Music*, loop, crossfade_ms)` privat; els dos overloads fan gating + name-handling i deleguen el dispatch |
|
|
| DUP-02 | `audio.cpp:172-185` | `setSoundVolume`/`setMusicVolume` idéntica estructura de clamp+gate+producto | Menor | DONE | `effectiveVolume(volume, channel_enabled)` privat; els dos setters deleguen |
|
|
| DUP-03 | `jail_audio.hpp:196-222` | `updateOutgoingFade`/`updateIncomingFade` comparten la curva lineal | Menor | DONE | `fadeProgress(FadeState&)` concentra la corba en un sol lloc; també cobreix `duration_ms <= 0` com a 1.0 |
|
|
| DUP-04 | `jail_audio.hpp:391-471` | `fadeOutMusic`/`crossfadeMusic` repiten "destruir outgoing + pre-fill + robar stream + reset vorbis" | Importante | DONE | `stealCurrentIntoOutgoing(duration_ms)` concentra el robatori; `fadeOutMusic` i `crossfadeMusic` hi deleguen. També homogeneïtza `outgoing.fade.active = false` en la destrucció (abans era inconsistent) |
|
|
| DUP-05 | `jail_audio.hpp:575-631` | `pauseChannel`/`resumeChannel`/`stopChannel` repiten dispatch `channel==-1 / [0,MAX)` | Menor | DONE | Template `forEachTargetChannel(channel, fn)`; cada funció només descriu què fa per canal |
|
|
|
|
## 2. Hardcoding
|
|
|
|
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|
|
|----|---------------|----------|-----------|--------|-------|
|
|
| HC-01 | `audio.hpp:53-55` | `VOLUME_STEP`, `FREQUENCY`, `DEFAULT_CROSSFADE_MS` viven en la clase, no en `Defaults::Audio` | Importante | DONE | Movidos a `Defaults::Audio::{VOLUME_STEP, FREQUENCY, CROSSFADE_MS}` en `utils/defaults.hpp` |
|
|
| HC-02 | `jail_audio.hpp:57,76,110` + `audio.cpp:208` | `{SDL_AUDIO_S16, 2, 48000}` literal en 3 sitios; `S16LE` vs `S16` inconsistente | Importante | DONE | `Defaults::Audio::{FORMAT,CHANNELS,FREQUENCY}` al call-site d'`Audio::initSDLAudio`; `Ja::DEFAULT_SPEC` com a fallback local perquè l'engine no depengui del projecte |
|
|
| HC-03 | `jail_audio.hpp:53-54` | `MAX_SIMULTANEOUS_CHANNELS=20`, `MAX_GROUPS=2` fijos sin justificación | Menor | DONE | `static_assert` a `audio.cpp` protegeix `Audio::Group ↔ Ja::MAX_GROUPS`. No es mou a Defaults — el motor és independent del projecte per disseny |
|
|
| HC-04 | `jail_audio.hpp:131,134` | `MUSIC_CHUNK_SHORTS=8192`, `MUSIC_LOW_WATER_SECONDS=0.5F` tuning hidden | Menor | WONTFIX | Són tuning intern del streaming (vorbis + SDL stream). Ja són `inline constexpr` amb comentaris; moure-les a Defaults trencaria la independència del motor (mateix criteri que HC-03) |
|
|
| HC-05 | `jail_audio.hpp:266` | `fill(sound_volume, 0.5F)` valor mágico que se sobreescribe enseguida | Menor | DONE | Eliminada la línia. El `inline float[]` zero-inicialitza i `Audio::initSDLAudio` sobreescriu immediatament via `enable()` |
|
|
|
|
## 3. Parches e improvisaciones
|
|
|
|
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|
|
|----|---------------|----------|-----------|--------|-------|
|
|
| PATCH-01 | `audio.hpp:98,105,111` | `toggleEnabled/toggleSound/toggleMusic` sólo flipan el flag; no reaplican volúmenes | Importante | DONE | `toggle*()` i variants sense bool passen per `enable[Sound|Music](bool)`. Aquestes funcions ara reapliquen el volum cap a `Ja::` perquè els canals actius responguin al toggle |
|
|
| PATCH-02 | `audio.cpp:98` | `music_.name.clear()` en overload por puntero → `getCurrentMusicName()` miente | Importante | PENDING | Derivar nombre de `Ja::Music::filename` |
|
|
| PATCH-03 | `audio.cpp:14-21` | `#undef L,C,R,PLAYBACK_*` workaround para macros filtradas por stb_vorbis | Importante | WONTFIX | Documentado; aislar stb_vorbis en TU propio sería más invasivo — se hará con COUP-01 |
|
|
| PATCH-04 | `jail_audio.hpp:501-507` | `setMusicPosition`/`getMusicPosition` stubs no-op | Importante | PENDING | Borrar si nadie los llama |
|
|
| PATCH-05 | `jail_audio.hpp:532-535` | "No hay canal libre → canal 0" robo de voz arbitrario | Importante | DEFERRED | Política de voice-stealing es una feature; se documenta |
|
|
|
|
## 4. Acoplamiento innecesario
|
|
|
|
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|
|
|----|---------------|----------|-----------|--------|-------|
|
|
| COUP-01 | `jail_audio.hpp:107-118` | `inline` globals en namespace: estado del engine sin encapsulación | Crítico | DEFERRED | Refactor grande; se hará en fase posterior |
|
|
| COUP-02 | `jail_audio.hpp` entero | Engine header-only: 678 líneas compilándose en cada TU | Importante | DEFERRED | Depende de COUP-01 |
|
|
| COUP-03 | `audio.cpp:3`, `jail_audio.hpp:8` | `#include <iostream>` sólo para `std::cout` de errores | Importante | PENDING | Sustituir por `std::fprintf(stderr,…)` con `<cstdio>` |
|
|
| COUP-04 | `audio.cpp:43-45`, `208` | `Ja::init` en `initSDLAudio` vs `Ja::quit` en destructor (asimetría) | Menor | WONTFIX | Convive con singleton; se revisará con COUP-01 |
|
|
| COUP-05 | `audio.hpp:71,79` | API pública expone `Ja::Music*`/`Ja::Sound*` | Menor | WONTFIX | Forward-declare ya mitiga; evita doble lookup por nombre |
|
|
|
|
## 5. Inconsistencias internas
|
|
|
|
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|
|
|----|---------------|----------|-----------|--------|-------|
|
|
| INCONS-01 | `audio.cpp:128,135,142,149,172,180` | Métodos `const` que mutan globals de `Ja` (const miente) | Importante | DONE | `const` tret de 6 mètodes (`playSound` x2, `stopAllSounds`, `fadeOutMusic`, `setSoundVolume`, `setMusicVolume`). `effectiveVolume` es queda `const` (pur sobre membres). Cap caller extern via `const Audio*` |
|
|
| INCONS-02 | `audio.hpp:44-48` + `jail_audio.hpp:44-50` | Dos `MusicState` paralelos + `getRealMusicState` como puente | Importante | PENDING | Un enum único; eliminar `getRealMusicState` |
|
|
| INCONS-03 | `audio.cpp` varios | Comentarios mezclan catalán y español | Menor | PENDING | Unificar a catalán |
|
|
| INCONS-04 | `audio.hpp:70,78` | `playMusic` `loop=-1` vs `playSound` `loop=0` | Menor | WONTFIX | Convención razonable (música loop / sonido one-shot); documentar |
|
|
| INCONS-05 | `audio.hpp:102-105` | 4 formas de enable/disable sound (API redundante) + divergencia con `Ja::enableSound` | Importante | PENDING | Dejar `enableSound(bool)` + `toggleSound()`; delegar a `Ja` |
|
|
|
|
## 6. Código muerto
|
|
|
|
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|
|
|----|---------------|----------|-----------|--------|-------|
|
|
| DEAD-01 | `jail_audio.hpp:501-507` | `setMusicPosition`/`getMusicPosition` stubs | Importante | PENDING | Verificar grep y borrar (= PATCH-04) |
|
|
| DEAD-02 | `jail_audio.hpp:671-675` | `Ja::setVolume` master no usado desde `Audio` | Menor | PENDING | Borrar si nadie lo llama |
|
|
| DEAD-03 | `jail_audio.hpp:346-350` | `getMusicFilename` presunto unused | Menor | PENDING | Verificar y borrar |
|
|
| DEAD-04 | `jail_audio.hpp:633-638` | `getChannelState` sin getter público en `Audio` | Menor | PENDING | Verificar y borrar |
|
|
| DEAD-05 | `audio.hpp:102-103` | `enableSound()`/`disableSound()` sin argumento (duplican la versión con bool) | Menor | PENDING | Cubierto por INCONS-05 |
|
|
|
|
## 7. Violaciones de convenciones
|
|
|
|
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|
|
|----|---------------|----------|-----------|--------|-------|
|
|
| CONV-01 | varios | Parámetros de audio fuera de `Defaults`/`config.yaml` | Importante | PENDING | Cubierto por HC-01..HC-04 |
|
|
| CONV-02 | `jail_audio.hpp` | Engine no es clase (namespace + globals), viola OOP | Importante | DEFERRED | Cubierto por COUP-01 |
|
|
| CONV-03 | `jail_audio.hpp` | No hay callback "music ended"; polling desde `update()` | Menor | DEFERRED | Requiere COUP-01 |
|