From 9243db1b9f030372f370300fbea883862b803eca Mon Sep 17 00:00:00 2001 From: Sergio Valor Date: Mon, 20 Apr 2026 09:02:08 +0200 Subject: [PATCH] docs: afegir full de treball de l'auditoria de core/rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inventari inicial dels hallazgos detectats a source/core/rendering (duplicació, hardcoding, parches, acoblament, inconsistències, codi mort i violacions de convencions), amb ID estable i estat PENDING. S'anirà actualitzant a mesura que s'apliquen els canvis. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/audit-core-rendering.md | 121 +++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 docs/audit-core-rendering.md diff --git a/docs/audit-core-rendering.md b/docs/audit-core-rendering.md new file mode 100644 index 0000000..5fc1286 --- /dev/null +++ b/docs/audit-core-rendering.md @@ -0,0 +1,121 @@ +# Auditoría `core/rendering/` + +Rama: `audit/core-rendering` +Estado global: en curso + +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 | Inversión de dependencia `core/rendering → game/*` | PENDING | +| DUP-02 | `Surface::render` / `renderWithColorReplace` duplicados | PENDING | +| CONV-03 | Shaders MSL como raw strings en `.cpp` + helpers duplicados | PENDING | +| DUP-03 | Familia `Text::write*` con bucle UTF-8 repetido 5 veces | PENDING | +| COUP-02 | `Screen` instancia `SDL3GPUShader` directamente + `Surface` → `Screen::get()` | PENDING | + +--- + +## 1. Duplicación + +| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas | +|----|---------------|----------|-----------|--------|-------| +| DUP-01 | `gif.cpp:238-276` | 3 ramas del switch extension saltan bloque+subloops idénticos | Importante | PENDING | helper `skipExtensionBlocks` | +| DUP-02 | `surface.cpp:80-137` | `render` vs `renderWithColorReplace` ~90% idénticos | Crítico | PENDING | inner loop compartido | +| DUP-03 | `text.cpp:145-289` | bucle UTF-8 repetido en write/writeColored/writeMonoDigits/length/lengthMonoDigits | Importante | PENDING | iterador de glifos | +| DUP-04 | `sdl3gpu_shader.cpp:165-201` + `crtpi_pipeline.cpp:165-202` | `createShaderMSL/SPIRV` duplicados | Importante | PENDING | extraer a `shader_factory` | +| DUP-05 | `sdl3gpu_shader.cpp:18-35` + `crtpi_pipeline.cpp:17-34` | vertex shader MSL duplicado | Menor | PENDING | compartir string | +| DUP-06 | `surface.cpp:14-43` | constructor Surface abre y parsea GIF 2 veces | Importante | PENDING | `loadGif` devuelve pixels+palette | +| DUP-07 | `crtpi_pipeline.hpp:70-87` + `shader_backend.hpp:30-45` + `crtpi_pipeline.cpp:45-62` | defaults CRT-Pi triples | Importante | PENDING | single source of truth | + +## 2. Hardcoding + +| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas | +|----|---------------|----------|-----------|--------|-------| +| HC-01 | `text.cpp:159` | `render(1, 15)` — `1` (tinta) y `15` (color por defecto) literales | Importante | PENDING | `INK_INDEX` / `DEFAULT_COLOR` | +| HC-02 | `surface.cpp:29`, `screen.cpp:56,218,244` | `0xFF000000U`/`0xFF` alfa repetido | Menor | PENDING | `OPAQUE_ALPHA` | +| HC-03 | `gif.cpp:71,95,…` | LZW `<11`/`<12`/`<2` y máscaras `0x80/0x70/0x07…` sin nombre | Menor | PENDING | constantes con nombre | +| HC-04 | `notifier.hpp:41-46`, `render_info.hpp:38-40` | HUD constants hardcoded en clase, no en `defaults`/YAML | Importante | PENDING | `Defaults::Hud::…` o config.yaml | +| HC-05 | `render_info.cpp:19` + `notifier.hpp:43` | `TEXT_SIZE=8` duplicado | Menor | PENDING | constante global font | +| HC-06 | `render_info.cpp:64,138-139,193-195` | `segments_[3]` hardcode del segmento tiempo | Importante | PENDING | `enum SegmentId` | +| HC-07 | `render_info.cpp:207` | `% 3` hardcode nº de entradas enum | Menor | PENDING | `RENDER_INFO_POSITION_COUNT` | +| HC-08 | `crtpi_pipeline.cpp:97` | `768.0f / h / 3.0f` magic en shader | Menor | PENDING | uniforme o comentario | +| HC-09 | `render_info.cpp:179-183,194-195` | shadow offset +1,+1 magic | Menor | PENDING | `SHADOW_DIST` | +| HC-10 | `gif.cpp:162` | `processImageDescriptor` params `[[maybe_unused]]` vestigiales | Menor | PENDING | quitar args | + +## 3. Parches e improvisaciones + +| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas | +|----|---------------|----------|-----------|--------|-------| +| PATCH-01 | `screen.cpp:71-92,210-214`, `sdl3gpu_shader.cpp:445-493` | workarounds Wayland/Vulkan | Importante | WONTFIX (de momento) | RAII para forzar orden — deferred | +| PATCH-02 | `render_info.cpp:85-88` | guard bug-fix animation | Menor | PENDING | refactor FSM animación | +| PATCH-03 | `notifier.cpp:78-85` | TODO implícito de cola FIFO | Menor | WONTFIX | documentar decisión | +| PATCH-04 | `sprite.cpp:47-49` | `Sprite::update` vacío hook no invocado | Menor | PENDING | borrar (ver DEAD-10) | +| PATCH-05 | `gif.cpp:12-15` | comentario "reemplaçar macro READ" | Menor | PENDING | limpiar comentario | +| PATCH-06 | `gif.cpp:83-86` | guard self-reference defensivo | Menor | WONTFIX | dejar, cambiar `cerr` → logger futuro | +| PATCH-07 | `sdl3gpu_shader.cpp:17,162` + `crtpi_pipeline.cpp:16,162` | `NOLINTBEGIN` gigantes sobre MSL | Importante | PENDING | se resuelve con CONV-03 | +| PATCH-08 | `render_info.cpp:64,138-139` | `mono_digits=true` asignado 2 veces | Menor | PENDING | init única | + +## 4. Acoplamiento innecesario + +| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas | +|----|---------------|----------|-----------|--------|-------| +| COUP-01 | `notifier.hpp:9`, `render_info.cpp:12-13` | `core/rendering` incluye `game/ui/*` y `game/options.hpp` | Crítico | PENDING | inyectar via Config/callback | +| COUP-02 | `screen.cpp:13-14,109` | `Screen` instancia `SDL3GPUShader` concreto | Importante | PENDING | factory/registry | +| COUP-03 | `surface.cpp:81-83,110-112` | `Surface::render` usa `Screen::get()` global | Importante | PENDING | pasar destino por parámetro | +| COUP-04 | `notifier.cpp:19`, `render_info.cpp:52` | `Resource::Cache::get().getSurface("8bithud.gif")` con string literal | Importante | PENDING | inyectar la surface | +| COUP-05 | `animated_sprite.cpp:9,95-116` | parser YAML incrustado en sprite | Importante | PENDING | `AnimationLoader` en `core/resources` | +| COUP-06 | `notifier.hpp:22`, `render_info.hpp:17`, `screen.hpp:13` | 3 Meyers singletons en mismo subsistema | Importante | DEFERRED | requiere tocar Director | + +## 5. Inconsistencias internas + +| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas | +|----|---------------|----------|-----------|--------|-------| +| INC-01 | todo el subsistema | smart pointers mezclados (shared/unique/raw sin política) | Importante | DEFERRED | requiere decisión de política | +| INC-02 | `screen.cpp:52,104,125` vs `gif.cpp:96` vs `sdl3gpu_shader.cpp:214` | `exit`/throw/`bool` mezclados | Importante | DEFERRED | decisión global | +| INC-03 | `screen.cpp`, `sdl3gpu_shader.cpp`, `surface.cpp`, `gif.cpp` | `fprintf`/`cerr`/`cout` mezclados | Menor | DEFERRED | logger inexistente | +| INC-04 | `text.hpp:48-57` | API discreta + `writeDX(flags)` coexisten | Importante | PENDING | elegir una | +| INC-05 | `moving_sprite.cpp:39-40,58-72` | `pos_.x = static_cast(x_)` sobre `SDL_FRect` | Importante | PENDING | fijar semántica | +| INC-06 | `moving_sprite.cpp:12-21` | `Sprite::pos_ = pos;` redundante tras ctor delegado | Menor | PENDING | limpiar | +| INC-07 | `sprite.cpp:19-22` | initializer posicional vs designated | Menor | PENDING | uniformar | + +## 6. Código muerto + +| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas | +|----|---------------|----------|-----------|--------|-------| +| DEAD-01 | `text.hpp:50-51` | `writeShadowed`, `writeCentered` sin callers externos | Importante | PENDING | borrar | +| DEAD-02 | `text.hpp:62-63,67` | `glyphWidth`, `getGlyphClip`, `codepointToUtf8` sin callers | Importante | PENDING | borrar | +| DEAD-03 | `text.hpp:64` | `getSprite()` sin callers | Menor | PENDING | borrar | +| DEAD-04 | `sprite.hpp:33` | `getRect()` sin callers | Menor | PENDING | borrar | +| DEAD-05 | `sprite.hpp:42-43` | `incX`, `incY` sin callers | Menor | PENDING | borrar | +| DEAD-06 | `sprite.hpp:12`, `moving_sprite.hpp:13` | ctors default sin callers | Menor | PENDING | borrar | +| DEAD-07 | `animated_sprite.hpp:30` | `getCurrentAnimationSize()` sin callers | Menor | PENDING | borrar | +| DEAD-08 | `surface.hpp:61` | `getSurfaceData()` sin callers externos | Menor | PENDING | borrar o privatizar | +| DEAD-09 | `gif.hpp:45-65` | `Extension`, `ApplicationExtension`, `PlaintextExtension`, `GraphicControlExtension` no instanciados | Menor | PENDING | borrar | +| DEAD-10 | `sprite.cpp:47-49` | `Sprite::update` hook no invocado | Menor | PENDING | borrar (cruza PATCH-04) | + +## 7. Violaciones de convenciones del proyecto + +| ID | Hallazgo | Cruza con | Estado | +|----|----------|-----------|--------| +| CONV-01 | Magic numbers vs "sin magic numbers" | HC-* | cubierto por HC | +| CONV-02 | HUD hardcoded vs "configurabilidad desde ficheros" | HC-04 | cubierto por HC-04 | +| CONV-03 | Shaders MSL como raw strings en `.cpp` | DUP-04/05, PATCH-07 | PENDING | +| CONV-04 | Singletons vs "interfaces + callbacks" | COUP-06 | DEFERRED | +| CONV-05 | `core → game` deps | COUP-01 | cubierto por COUP-01 | + +--- + +## Registro de commits + +(Se irá rellenando a medida que se apliquen los cambios.)