- MovingSprite: el ctor delegat Sprite(surface, pos) ja assigna pos_;
el Sprite::pos_ = pos; manual que seguia era redundant (INC-06).
- Sprite(surface) usava initializer posicional {0.0F, 0.0F, ...} mentre
la resta del projecte usa designated initializers ({.x = ..., .y = ...}).
Unificat (INC-07).
INC-05 (el static_cast<int> de pos_.x a MovingSprite::move i setters)
es marca WONTFIX al document: és snap intencional a píxel per al
pixel-art, divergent només al setter per SDL_FRect que es reserva per
a posicionament directe sense arrodoniment.
Build + smoke 5s xvfb OK (exit 124).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
11 KiB
11 KiB
Auditoría core/rendering/
Rama: audit/core-rendering
Estado global: en curso
Estados posibles:
PENDING— identificado, sin tocarWIP— diff propuesto, pendiente de aplicarDONE— aplicado y commiteadoWONTFIX— 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/* |
DONE |
| DUP-02 | Surface::render / renderWithColorReplace duplicados |
DONE |
| CONV-03 | Shaders MSL como raw strings en .cpp + helpers duplicados |
DONE |
| DUP-03 | Familia Text::write* con bucle UTF-8 repetido 5 veces |
DONE |
| COUP-02 | Screen instancia SDL3GPUShader directamente + Surface → Screen::get() |
DONE |
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 | DONE | skipExtensionBlock i switch eliminat |
| DUP-02 | surface.cpp:80-137 |
render vs renderWithColorReplace ~90% idénticos |
Crítico | DONE | render() delega amb source==target==transparent |
| DUP-03 | text.cpp:145-289 |
bucle UTF-8 repetido en write/writeColored/writeMonoDigits/length/lengthMonoDigits | Importante | DONE | resolveGlyph + write delega a writeColored |
| DUP-04 | sdl3gpu_shader.cpp:165-201 + crtpi_pipeline.cpp:165-202 |
createShaderMSL/SPIRV duplicados |
Importante | DONE | sdl3gpu/shader_factory.hpp |
| DUP-05 | sdl3gpu_shader.cpp:18-35 + crtpi_pipeline.cpp:17-34 |
vertex shader MSL duplicado | Menor | DONE | sdl3gpu/msl/postfx_vert.msl.h compartit |
| DUP-06 | surface.cpp:14-43 |
constructor Surface abre y parsea GIF 2 veces | Importante | DONE | buffer carregat una volta, pixels+palette del mateix |
| 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 | DONE | Text::INK_COLOR_INDEX i DEFAULT_TEXT_COLOR |
| HC-02 | surface.cpp:29, screen.cpp:56,218,244 |
0xFF000000U/0xFF alfa repetido |
Menor | DONE | Rendering::OPAQUE_ALPHA a canvas.hpp |
| HC-03 | gif.cpp:71,95,… |
LZW <11/<12/<2 y máscaras 0x80/0x70/0x07… sin nombre |
Menor | DONE | LZW_MIN/MAX_CODE_LENGTH, SCREEN_GCT_FLAG, SCREEN_GCT_SIZE_MASK, etc. |
| 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 | DONE | constants FPS/DRIVER/SHADER/TIME_SEGMENT |
| HC-07 | render_info.cpp:207 |
% 3 hardcode nº de entradas enum |
Menor | DONE | Defaults::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 | DONE | SHADOW_OFFSET_PX |
| HC-10 | gif.cpp:162 |
processImageDescriptor params [[maybe_unused]] vestigiales |
Menor | DONE | signatura sense gct ni resolution_bits; RGB també esborrat |
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 | DONE | resolt amb DEAD-10 |
| PATCH-05 | gif.cpp:12-15 |
comentario "reemplaçar macro READ" | Menor | DONE | comentari neutre nou |
| 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 | DONE | eliminats amb CONV-03 |
| PATCH-08 | render_info.cpp:64,138-139 |
mono_digits=true asignado 2 veces |
Menor | DONE | inicialització única via refreshSegments |
4. Acoplamiento innecesario
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|---|---|---|---|---|---|
| COUP-01a | notifier.hpp:9 |
core/rendering/notifier.hpp incluye game/ui/notification_presets.hpp |
Crítico | DONE | helper Notification::notify() en game/ui |
| COUP-01b | render_info.cpp:12-13 |
core/rendering/render_info.cpp incluye game/options.hpp + game/ui/ui_palette.hpp |
Crítico | DONE | RenderInfo::Frame injectada; Options.buildRenderInfoFrame() |
| COUP-02a | screen.cpp:13-14,109 |
Screen instancia SDL3GPUShader concreto |
Importante | DONE | Rendering::makeDefaultShaderBackend() factory |
| COUP-03 | surface.cpp:81-83,110-112 |
Surface::render usa Screen::get() global |
Importante | DONE | Rendering::activeCanvas() intermediari |
| 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<int>(x_) sobre SDL_FRect |
Importante | WONTFIX | És snap intencional a píxel per al pixel-art; setPos(SDL_FRect) es reserva per a posicionament directe sense arrodoniment. La divergència és una decisió, no un bug |
| INC-06 | moving_sprite.cpp:12-21 |
Sprite::pos_ = pos; redundante tras ctor delegado |
Menor | DONE | línies eliminades |
| INC-07 | sprite.cpp:19-22 |
initializer posicional vs designated | Menor | DONE | designated initializer uniforme |
6. Código muerto
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|---|---|---|---|---|---|
| DEAD-01 | text.hpp:50-51 |
writeShadowed, writeCentered sin callers externos |
Importante | DONE | esborrats |
| DEAD-02 | text.hpp:62-63,67 |
glyphWidth, getGlyphClip, codepointToUtf8 sin callers |
Importante | DONE | esborrats |
| DEAD-03 | text.hpp:64 |
getSprite() sin callers |
Menor | DONE | esborrat amb DEAD-01/02 |
| DEAD-04 | sprite.hpp:33 |
getRect() sin callers |
Menor | DONE | esborrat |
| DEAD-05 | sprite.hpp:42-43 |
incX, incY sin callers |
Menor | DONE | esborrats |
| DEAD-06 | sprite.hpp:12, moving_sprite.hpp:13 |
ctors default sin callers | Menor | WONTFIX | usats transitivament pel ctor de AnimatedSprite (base-class-construction) |
| DEAD-07 | animated_sprite.hpp:30 |
getCurrentAnimationSize() sin callers |
Menor | DONE | esborrat |
| DEAD-08 | surface.hpp:61 |
getSurfaceData() sin callers externos |
Menor | DONE | esborrat |
| DEAD-09 | gif.hpp:45-65 |
Extension, ApplicationExtension, PlaintextExtension, GraphicControlExtension no instanciados |
Menor | DONE | esborrats (el parser processa bytes cru) |
| DEAD-10 | sprite.cpp:47-49 |
Sprite::update hook no invocado |
Menor | DONE | inlined buit al header (resol 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 | DONE |
| CONV-04 | Singletons vs "interfaces + callbacks" | COUP-06 | DEFERRED |
| CONV-05 | core → game deps |
COUP-01 | cubierto por COUP-01 |
Registro de commits
9243db1— docs: full de treball inicial (aquest fitxer)9434e4e— COUP-01a:Notifierdesacoblat degame/ui/notification_presets9395520— COUP-01b:RenderInfodesacoblat degame/options+game/ui/ui_paletted479475— DUP-02:Surface::renderdelega arenderWithColorReplace5f46fb2— CONV-03 + DUP-04 + DUP-05 + PATCH-07: shaders MSL i helpers extrets asdl3gpu/msl/isdl3gpu/shader_factory.hppa63deac— DUP-03 + HC-01:TextambresolveGlyphi constantsINK_COLOR_INDEX/DEFAULT_TEXT_COLOR029907d— COUP-02a:Screendesacoblat deSDL3GPUShaderviamakeDefaultShaderBackend()81fe826— COUP-02b (= COUP-03):SurfaceusaRendering::activeCanvas()en lloc deScreen::get()b51109e— DEAD-01 + DEAD-02 + DEAD-03: esborrats mètodes mort deText597402a— DEAD-04/05/07/08/10 + PATCH-04: neteja mort aSprite/AnimatedSprite/Surface+Sprite::updateinlinece3b6da— DEAD-09: esborrats structs d'extensions GIF no usatsbbb397a— HC-06 + HC-07 + HC-09 + PATCH-08: constants nominals arender_info(segments, shadow offset, position count) i dedup initbe94a43— DUP-01 + DUP-06 + HC-03 + HC-10 + PATCH-05: neteja parser GIF (extension single-skip, constants nominals LZW/GCT, signatura sense args morts, un sol parse per construcció deSurface)8b413af— HC-02:Rendering::OPAQUE_ALPHAacanvas.hppreemplaça0xFF000000Urepetit<pendent>— INC-06 + INC-07: eliminats assignaments redundantsSprite::pos_aMovingSprite; designated initializer uniforme