Compare commits
110 Commits
b44de25113
...
master
| Author | SHA1 | Date | |
|---|---|---|---|
| 60d6a9e740 | |||
| e1921858ed | |||
| 14cb96345c | |||
| d28feb3276 | |||
| a7a3ae9055 | |||
| 5ec4bf926b | |||
| 0e04617968 | |||
| b450e2af21 | |||
| a65756f298 | |||
| dac312ef2b | |||
| 953d6af701 | |||
| c14e305a4a | |||
| fb036dede5 | |||
| 3c9dc0ff65 | |||
| d3ca238a51 | |||
| a729dc089e | |||
| 18463774e9 | |||
| 694243ac89 | |||
| 60ddc9ddd3 | |||
| 70946fc720 | |||
| fb82ac06d0 | |||
| 494f906435 | |||
| 50c869536d | |||
| 0b99911f5d | |||
| 33d5eedec8 | |||
| 744268fedd | |||
| 06aa63f548 | |||
| a9943ce8bf | |||
| b46af7ab1d | |||
| ab22d4c34f | |||
| e2d6ea64a4 | |||
| 322744c296 | |||
| cf3e897752 | |||
| 4efb60bb5b | |||
| afd7fdf18d | |||
| 5b9eb5f0e3 | |||
| 09c8d3c0ef | |||
| a6c2c78cb5 | |||
| 18c29fed1e | |||
| f3064e9dad | |||
| a1f16a7d94 | |||
| eb9822dac7 | |||
| 6c48af0bec | |||
| b69b090e45 | |||
| ad014e1de0 | |||
| 9a3c1a0688 | |||
| 970259e3d6 | |||
| 34447f0245 | |||
| fd29ae271e | |||
| 737dc71d8c | |||
| 8a4dc2771d | |||
| 212dd4c404 | |||
| a520de6c1f | |||
| 38dbc17ace | |||
| 783c12790d | |||
| 64fce596ce | |||
| adf418dff9 | |||
| fe0cd289e2 | |||
| 989b98002c | |||
| 0ab7121c5b | |||
| 492abc09bc | |||
| fe6c5e3c8a | |||
| 122de2b36f | |||
| a671825502 | |||
| cecf5cf68e | |||
| 3264672be0 | |||
| 29c1d6b745 | |||
| 81586aa768 | |||
| ec086b2cd4 | |||
| 8be2d68b98 | |||
| b0cec977a5 | |||
| 41d39b9bf7 | |||
| 9e0e1b2873 | |||
| afcc1dd77d | |||
| d53a1cde34 | |||
| 8569b467e0 | |||
| 5a755e9995 | |||
| 1e97b3cfa3 | |||
| b9791589f2 | |||
| 108caf7ffd | |||
| 3c6466e2a0 | |||
| d2fa5b2782 | |||
| d1d0d891fa | |||
| ea74af146d | |||
| e8a7b19213 | |||
| 57eac01bcb | |||
| 815913b15b | |||
| 972588fa59 | |||
| 9d989ab395 | |||
| a74d7135e6 | |||
| e27d1b60b1 | |||
| 12bc4870b1 | |||
| 2b4b07ae6a | |||
| 24779755a5 | |||
| d2ba311c5f | |||
| f5424e8f72 | |||
| 5a95c9180c | |||
| 59dc3a1638 | |||
| 262ae49496 | |||
| ec9eb45cc3 | |||
| 1355ce49fe | |||
| 8a549d14dc | |||
| 12110bd8b4 | |||
| f086ed3021 | |||
| f24d484496 | |||
| 26b4454eea | |||
| b531bbc798 | |||
| cb8293175b | |||
| fff14fe3e1 | |||
| ffdb67ce9b |
168
.copilot-rules.md
Normal file
@ -0,0 +1,168 @@
|
||||
# Copilot Rules — Spacetris (SDL3 / C++20)
|
||||
|
||||
These rules define **non-negotiable constraints** for all AI-assisted changes.
|
||||
They exist to preserve determinism, performance, and architecture.
|
||||
|
||||
If these rules conflict with `.github/copilot-instructions.md`,
|
||||
**follow `.github/copilot-instructions.md`.**
|
||||
|
||||
---
|
||||
|
||||
## Project Constraints (Non-Negotiable)
|
||||
|
||||
- Language: **C++20**
|
||||
- Runtime: **SDL3** + **SDL3_ttf**
|
||||
- Build system: **CMake**
|
||||
- Dependencies via **vcpkg**
|
||||
- Assets must use **relative paths only**
|
||||
- Deterministic gameplay logic is mandatory
|
||||
|
||||
Do not rewrite or refactor working systems unless explicitly requested.
|
||||
|
||||
---
|
||||
|
||||
## Repo Layout & Responsibilities
|
||||
|
||||
- Core gameplay loop/state: `src/Game.*`
|
||||
- Entry point: `src/main.cpp`
|
||||
- Text/TTF: `src/Font.*`
|
||||
- Audio: `src/Audio.*`, `src/SoundEffect.*`
|
||||
- Effects: `src/LineEffect.*`, `src/Starfield*.cpp`
|
||||
- High scores: `src/Scores.*`
|
||||
- Packaging: `build-production.ps1`
|
||||
|
||||
When adding a module:
|
||||
- Place it under `src/` (or an established subfolder)
|
||||
- Register it in `CMakeLists.txt`
|
||||
- Avoid circular includes
|
||||
- Keep headers minimal
|
||||
|
||||
---
|
||||
|
||||
## Build & Verification
|
||||
|
||||
Prefer existing scripts:
|
||||
|
||||
- Debug: `cmake --build build-msvc --config Debug`
|
||||
- Release:
|
||||
- Configure: `cmake -S . -B build-release -DCMAKE_BUILD_TYPE=Release`
|
||||
- Build: `cmake --build build-release --config Release`
|
||||
- Packaging (Windows): `./build-production.ps1`
|
||||
|
||||
Before finalizing changes:
|
||||
- Debug build must succeed
|
||||
- Packaging must succeed if assets or DLLs are touched
|
||||
|
||||
Do not introduce new build steps unless required.
|
||||
|
||||
---
|
||||
|
||||
## Coding & Architecture Rules
|
||||
|
||||
- Match local file style (naming, braces, spacing)
|
||||
- Avoid large refactors
|
||||
- Prefer small, testable helpers
|
||||
- Avoid floating-point math in core gameplay state
|
||||
- Game logic must be deterministic
|
||||
- Rendering code must not mutate game state
|
||||
|
||||
---
|
||||
|
||||
## Rendering & Performance Rules
|
||||
|
||||
- Do not allocate memory per frame
|
||||
- Do not load assets during rendering
|
||||
- No blocking calls in render loop
|
||||
- Visual effects must be time-based (`deltaTime`)
|
||||
- Rendering must not contain gameplay logic
|
||||
|
||||
---
|
||||
|
||||
## Threading Rules
|
||||
|
||||
- SDL main thread:
|
||||
- Rendering
|
||||
- Input
|
||||
- Game simulation
|
||||
- Networking must be **non-blocking** from the SDL main loop
|
||||
- Either run networking on a separate thread, or poll ENet frequently with a 0 timeout
|
||||
- Never wait/spin for remote inputs on the render thread
|
||||
- Cross-thread communication via queues or buffers only
|
||||
|
||||
---
|
||||
|
||||
## Assets, Fonts, and Paths
|
||||
|
||||
- Runtime expects adjacent `assets/` directory
|
||||
- `FreeSans.ttf` must remain at repo root
|
||||
- New assets:
|
||||
- Go under `assets/`
|
||||
- Must be included in `build-production.ps1`
|
||||
|
||||
Never hardcode machine-specific paths.
|
||||
|
||||
---
|
||||
|
||||
## AI Partner (COOPERATE Mode)
|
||||
|
||||
- AI is **supportive**, not competitive
|
||||
- AI must respect sync timing and shared grid logic
|
||||
- AI must not “cheat” or see hidden future pieces
|
||||
- AI behavior must be deterministic per seed/difficulty
|
||||
|
||||
---
|
||||
|
||||
## Networking (COOPERATE Network Mode)
|
||||
|
||||
Follow `docs/ai/cooperate_network.md`.
|
||||
If `network_cooperate_multiplayer.md` exists, keep it consistent with the canonical doc.
|
||||
|
||||
Mandatory model:
|
||||
- **Input lockstep**
|
||||
- Transmit inputs only (no board state replication)
|
||||
|
||||
Determinism requirements:
|
||||
- Fixed tick (e.g. 60 Hz)
|
||||
- Shared RNG seed
|
||||
- Deterministic gravity, rotation, locking, scoring
|
||||
|
||||
Technology:
|
||||
- Use **ENet**
|
||||
- Do NOT use SDL_net or TCP-only networking
|
||||
|
||||
Architecture:
|
||||
- Networking must be isolated (e.g. `src/network/NetSession.*`)
|
||||
- Game logic must not care if partner is local, AI, or network
|
||||
|
||||
Robustness:
|
||||
- Input delay buffer (4–6 ticks)
|
||||
- Periodic desync hashing
|
||||
- Graceful disconnect handling
|
||||
|
||||
Do NOT implement:
|
||||
- Rollback
|
||||
- Full state sync
|
||||
- Server-authoritative sim
|
||||
- Matchmaking SDKs
|
||||
- Versus mechanics
|
||||
|
||||
---
|
||||
|
||||
## Agent Behavior Rules (IMPORTANT)
|
||||
|
||||
- Always read relevant markdown specs **before coding**
|
||||
- Treat markdown specs as authoritative
|
||||
- Do not invent APIs
|
||||
- Do not assume external libraries exist
|
||||
- Generate code **file by file**, not everything at once
|
||||
- Ask before changing architecture or ownership boundaries
|
||||
|
||||
---
|
||||
|
||||
## When to Ask Before Proceeding
|
||||
|
||||
Ask the maintainer if unclear:
|
||||
- UX or menu flow decisions
|
||||
- Adding dependencies
|
||||
- Refactors vs local patches
|
||||
- Platform-specific behavior
|
||||
6
.github/copilot-instructions.md
vendored
@ -1,4 +1,4 @@
|
||||
# Copilot Instructions — Tetris (C++ SDL3)
|
||||
# Copilot Instructions — Spacetris (C++ SDL3)
|
||||
|
||||
Purpose: Speed up development on this native SDL3 Tetris. Follow these conventions to keep builds reproducible and packages shippable.
|
||||
|
||||
@ -9,11 +9,9 @@ Purpose: Speed up development on this native SDL3 Tetris. Follow these conventio
|
||||
- Assets: `assets/` (images/music/fonts), plus `FreeSans.ttf` at repo root.
|
||||
|
||||
## Build and run
|
||||
- Configure and build Release:
|
||||
- CMake picks up vcpkg toolchain if found (`VCPKG_ROOT`, local `vcpkg/`, or user path). Required packages: `sdl3`, `sdl3-ttf` (see `vcpkg.json`).
|
||||
- Typical sequence (PowerShell): `cmake -S . -B build-release -DCMAKE_BUILD_TYPE=Release` then `cmake --build build-release --config Release`.
|
||||
- Packaging: `.\build-production.ps1` creates `dist/TetrisGame/` with exe, DLLs, assets, README, and ZIP; it can clean via `-Clean` and package-only via `-PackageOnly`.
|
||||
- MSVC generator builds are under `build-msvc/` when using Visual Studio.
|
||||
Packaging: `.\build-production.ps1` creates `dist/SpacetrisGame/` with exe, DLLs, assets, README, and ZIP; it can clean via `-Clean` and package-only via `-PackageOnly`.
|
||||
|
||||
## Runtime dependencies
|
||||
- Links: `SDL3::SDL3`, `SDL3_ttf::SDL3_ttf`; on Windows also `mfplat`, `mfreadwrite`, `mfuuid` for media.
|
||||
|
||||
36
.github/workflows/build.yml
vendored
@ -1,4 +1,4 @@
|
||||
name: Build and Package Tetris
|
||||
name: Build and Package Spacetris
|
||||
|
||||
on:
|
||||
push:
|
||||
@ -43,20 +43,20 @@ jobs:
|
||||
- name: Upload artifacts
|
||||
uses: actions/upload-artifact@v4
|
||||
with:
|
||||
name: tetris-windows-x64
|
||||
path: dist/TetrisGame/
|
||||
name: spacetris-windows-x64
|
||||
path: dist/SpacetrisGame/
|
||||
|
||||
- name: Create Release ZIP
|
||||
if: startsWith(github.ref, 'refs/tags/v')
|
||||
run: |
|
||||
cd dist
|
||||
7z a ../TetrisGame-Windows-x64.zip TetrisGame/
|
||||
7z a ../SpacetrisGame-Windows-x64.zip SpacetrisGame/
|
||||
|
||||
- name: Release
|
||||
if: startsWith(github.ref, 'refs/tags/v')
|
||||
uses: softprops/action-gh-release@v1
|
||||
with:
|
||||
files: TetrisGame-Windows-x64.zip
|
||||
files: SpacetrisGame-Windows-x64.zip
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
|
||||
@ -83,32 +83,32 @@ jobs:
|
||||
|
||||
- name: Package
|
||||
run: |
|
||||
mkdir -p dist/TetrisGame-Linux
|
||||
cp build/tetris dist/TetrisGame-Linux/
|
||||
cp -r assets dist/TetrisGame-Linux/
|
||||
cp FreeSans.ttf dist/TetrisGame-Linux/
|
||||
echo '#!/bin/bash' > dist/TetrisGame-Linux/launch-tetris.sh
|
||||
echo 'cd "$(dirname "$0")"' >> dist/TetrisGame-Linux/launch-tetris.sh
|
||||
echo './tetris' >> dist/TetrisGame-Linux/launch-tetris.sh
|
||||
chmod +x dist/TetrisGame-Linux/launch-tetris.sh
|
||||
chmod +x dist/TetrisGame-Linux/tetris
|
||||
mkdir -p dist/SpacetrisGame-Linux
|
||||
cp build/spacetris dist/SpacetrisGame-Linux/
|
||||
cp -r assets dist/SpacetrisGame-Linux/
|
||||
cp FreeSans.ttf dist/SpacetrisGame-Linux/
|
||||
echo '#!/bin/bash' > dist/SpacetrisGame-Linux/launch-spacetris.sh
|
||||
echo 'cd "$(dirname "$0")"' >> dist/SpacetrisGame-Linux/launch-spacetris.sh
|
||||
echo './spacetris' >> dist/SpacetrisGame-Linux/launch-spacetris.sh
|
||||
chmod +x dist/SpacetrisGame-Linux/launch-spacetris.sh
|
||||
chmod +x dist/SpacetrisGame-Linux/spacetris
|
||||
|
||||
- name: Upload artifacts
|
||||
uses: actions/upload-artifact@v4
|
||||
with:
|
||||
name: tetris-linux-x64
|
||||
path: dist/TetrisGame-Linux/
|
||||
name: spacetris-linux-x64
|
||||
path: dist/SpacetrisGame-Linux/
|
||||
|
||||
- name: Create Release TAR
|
||||
if: startsWith(github.ref, 'refs/tags/v')
|
||||
run: |
|
||||
cd dist
|
||||
tar -czf ../TetrisGame-Linux-x64.tar.gz TetrisGame-Linux/
|
||||
tar -czf ../SpacetrisGame-Linux-x64.tar.gz SpacetrisGame-Linux/
|
||||
|
||||
- name: Release
|
||||
if: startsWith(github.ref, 'refs/tags/v')
|
||||
uses: softprops/action-gh-release@v1
|
||||
with:
|
||||
files: TetrisGame-Linux-x64.tar.gz
|
||||
files: SpacetrisGame-Linux-x64.tar.gz
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
|
||||
2
.gitignore
vendored
@ -1,4 +1,4 @@
|
||||
# .gitignore for Tetris (native C++ project and web subproject)
|
||||
# .gitignore for Spacetris (native C++ project and web subproject)
|
||||
|
||||
# Visual Studio / VS artifacts
|
||||
.vs/
|
||||
|
||||
@ -15,7 +15,7 @@ if(DEFINED CMAKE_TOOLCHAIN_FILE)
|
||||
set(CMAKE_TOOLCHAIN_FILE "${CMAKE_TOOLCHAIN_FILE}" CACHE STRING "" FORCE)
|
||||
endif()
|
||||
|
||||
project(tetris_sdl3 LANGUAGES CXX)
|
||||
project(spacetris_sdl3 LANGUAGES CXX)
|
||||
|
||||
set(CMAKE_CXX_STANDARD 20)
|
||||
set(CMAKE_CXX_STANDARD_REQUIRED ON)
|
||||
@ -28,10 +28,14 @@ find_package(SDL3_ttf CONFIG REQUIRED)
|
||||
find_package(SDL3_image CONFIG REQUIRED)
|
||||
find_package(cpr CONFIG REQUIRED)
|
||||
find_package(nlohmann_json CONFIG REQUIRED)
|
||||
find_package(unofficial-enet CONFIG REQUIRED)
|
||||
|
||||
set(TETRIS_SOURCES
|
||||
src/main.cpp
|
||||
src/app/TetrisApp.cpp
|
||||
src/gameplay/core/Game.cpp
|
||||
src/gameplay/coop/CoopGame.cpp
|
||||
src/gameplay/coop/CoopAIController.cpp
|
||||
src/core/GravityManager.cpp
|
||||
src/core/state/StateManager.cpp
|
||||
# New core architecture classes
|
||||
@ -42,40 +46,73 @@ set(TETRIS_SOURCES
|
||||
src/core/Settings.cpp
|
||||
src/graphics/renderers/RenderManager.cpp
|
||||
src/persistence/Scores.cpp
|
||||
src/network/supabase_client.cpp
|
||||
src/network/NetSession.cpp
|
||||
src/graphics/effects/Starfield.cpp
|
||||
src/graphics/effects/Starfield3D.cpp
|
||||
src/graphics/effects/SpaceWarp.cpp
|
||||
src/graphics/ui/Font.cpp
|
||||
src/graphics/ui/HelpOverlay.cpp
|
||||
src/graphics/renderers/GameRenderer.cpp
|
||||
src/graphics/renderers/SyncLineRenderer.cpp
|
||||
src/graphics/renderers/UIRenderer.cpp
|
||||
src/audio/Audio.cpp
|
||||
src/gameplay/effects/LineEffect.cpp
|
||||
src/audio/SoundEffect.cpp
|
||||
src/video/VideoPlayer.cpp
|
||||
src/ui/MenuLayout.cpp
|
||||
src/ui/BottomMenu.cpp
|
||||
src/app/BackgroundManager.cpp
|
||||
src/app/Fireworks.cpp
|
||||
src/app/AssetLoader.cpp
|
||||
src/app/TextureLoader.cpp
|
||||
src/states/LoadingManager.cpp
|
||||
# State implementations (new)
|
||||
src/states/LoadingState.cpp
|
||||
src/states/VideoState.cpp
|
||||
src/states/MenuState.cpp
|
||||
src/states/OptionsState.cpp
|
||||
src/states/LevelSelectorState.cpp
|
||||
src/states/PlayingState.cpp
|
||||
)
|
||||
|
||||
|
||||
if(APPLE)
|
||||
add_executable(tetris MACOSX_BUNDLE ${TETRIS_SOURCES})
|
||||
set(APP_ICON "${CMAKE_SOURCE_DIR}/assets/favicon/AppIcon.icns")
|
||||
if(EXISTS "${APP_ICON}")
|
||||
add_executable(spacetris MACOSX_BUNDLE ${TETRIS_SOURCES} "${APP_ICON}")
|
||||
set_source_files_properties("${APP_ICON}" PROPERTIES MACOSX_PACKAGE_LOCATION "Resources")
|
||||
set_target_properties(spacetris PROPERTIES
|
||||
MACOSX_BUNDLE_ICON_FILE "AppIcon"
|
||||
MACOSX_BUNDLE_INFO_PLIST "${CMAKE_SOURCE_DIR}/cmake/MacBundleInfo.plist.in"
|
||||
)
|
||||
else()
|
||||
message(WARNING "App icon not found at ${APP_ICON}; bundle will use default icon")
|
||||
add_executable(spacetris MACOSX_BUNDLE ${TETRIS_SOURCES})
|
||||
set_target_properties(spacetris PROPERTIES
|
||||
MACOSX_BUNDLE_INFO_PLIST "${CMAKE_SOURCE_DIR}/cmake/MacBundleInfo.plist.in"
|
||||
)
|
||||
endif()
|
||||
else()
|
||||
add_executable(tetris ${TETRIS_SOURCES})
|
||||
add_executable(spacetris ${TETRIS_SOURCES})
|
||||
endif()
|
||||
|
||||
# Ensure the built executable is named `spacetris`.
|
||||
set_target_properties(spacetris PROPERTIES OUTPUT_NAME "spacetris")
|
||||
|
||||
if (WIN32)
|
||||
# No compatibility copy; built executable is `spacetris`.
|
||||
endif()
|
||||
|
||||
if (WIN32)
|
||||
# Embed the application icon into the executable
|
||||
set_source_files_properties(src/app_icon.rc PROPERTIES LANGUAGE RC)
|
||||
target_sources(tetris PRIVATE src/app_icon.rc)
|
||||
target_sources(spacetris PRIVATE src/app_icon.rc)
|
||||
endif()
|
||||
|
||||
if(APPLE)
|
||||
set_target_properties(tetris PROPERTIES
|
||||
MACOSX_BUNDLE_INFO_PLIST "${CMAKE_SOURCE_DIR}/cmake/MacBundleInfo.plist.in"
|
||||
)
|
||||
if(MSVC)
|
||||
# Prevent PDB write contention on MSVC by enabling /FS for this target
|
||||
target_compile_options(spacetris PRIVATE /FS)
|
||||
endif()
|
||||
|
||||
if (WIN32)
|
||||
@ -90,14 +127,14 @@ if (WIN32)
|
||||
COMMENT "Copy favicon.ico to build dir for resource compilation"
|
||||
)
|
||||
add_custom_target(copy_favicon ALL DEPENDS ${FAVICON_DEST})
|
||||
add_dependencies(tetris copy_favicon)
|
||||
add_dependencies(spacetris copy_favicon)
|
||||
else()
|
||||
message(WARNING "Favicon not found at ${FAVICON_SRC}; app icon may not compile")
|
||||
endif()
|
||||
|
||||
# Also copy favicon into the runtime output folder (same dir as exe)
|
||||
add_custom_command(TARGET tetris POST_BUILD
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${FAVICON_SRC} $<TARGET_FILE_DIR:tetris>/favicon.ico
|
||||
add_custom_command(TARGET spacetris POST_BUILD
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${FAVICON_SRC} $<TARGET_FILE_DIR:spacetris>/favicon.ico
|
||||
COMMENT "Copy favicon.ico next to executable"
|
||||
)
|
||||
endif()
|
||||
@ -106,31 +143,42 @@ if(APPLE)
|
||||
set(_mac_copy_commands)
|
||||
if(EXISTS "${CMAKE_SOURCE_DIR}/assets")
|
||||
list(APPEND _mac_copy_commands
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_directory "${CMAKE_SOURCE_DIR}/assets" "$<TARGET_FILE_DIR:tetris>/assets"
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_directory "${CMAKE_SOURCE_DIR}/assets" "$<TARGET_FILE_DIR:spacetris>/assets"
|
||||
)
|
||||
endif()
|
||||
if(EXISTS "${CMAKE_SOURCE_DIR}/fonts")
|
||||
list(APPEND _mac_copy_commands
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_directory "${CMAKE_SOURCE_DIR}/fonts" "$<TARGET_FILE_DIR:tetris>/fonts"
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_directory "${CMAKE_SOURCE_DIR}/fonts" "$<TARGET_FILE_DIR:spacetris>/fonts"
|
||||
)
|
||||
endif()
|
||||
if(EXISTS "${CMAKE_SOURCE_DIR}/FreeSans.ttf")
|
||||
list(APPEND _mac_copy_commands
|
||||
COMMAND ${CMAKE_COMMAND} -E copy "${CMAKE_SOURCE_DIR}/FreeSans.ttf" "$<TARGET_FILE_DIR:tetris>/FreeSans.ttf"
|
||||
COMMAND ${CMAKE_COMMAND} -E copy "${CMAKE_SOURCE_DIR}/FreeSans.ttf" "$<TARGET_FILE_DIR:spacetris>/FreeSans.ttf"
|
||||
)
|
||||
endif()
|
||||
if(_mac_copy_commands)
|
||||
add_custom_command(TARGET tetris POST_BUILD
|
||||
add_custom_command(TARGET spacetris POST_BUILD
|
||||
${_mac_copy_commands}
|
||||
COMMENT "Copying game assets into macOS bundle"
|
||||
)
|
||||
endif()
|
||||
endif()
|
||||
|
||||
target_link_libraries(tetris PRIVATE SDL3::SDL3 SDL3_ttf::SDL3_ttf SDL3_image::SDL3_image cpr::cpr nlohmann_json::nlohmann_json)
|
||||
target_link_libraries(spacetris PRIVATE SDL3::SDL3 SDL3_ttf::SDL3_ttf SDL3_image::SDL3_image cpr::cpr nlohmann_json::nlohmann_json unofficial::enet::enet)
|
||||
|
||||
find_package(FFMPEG REQUIRED)
|
||||
if(FFMPEG_FOUND)
|
||||
target_include_directories(spacetris PRIVATE ${FFMPEG_INCLUDE_DIRS})
|
||||
target_link_directories(spacetris PRIVATE ${FFMPEG_LIBRARY_DIRS})
|
||||
target_link_libraries(spacetris PRIVATE ${FFMPEG_LIBRARIES})
|
||||
endif()
|
||||
|
||||
if (WIN32)
|
||||
target_link_libraries(tetris PRIVATE mfplat mfreadwrite mfuuid)
|
||||
target_link_libraries(spacetris PRIVATE mfplat mfreadwrite mfuuid ws2_32 winmm)
|
||||
endif()
|
||||
if(APPLE)
|
||||
# Needed for MP3 decoding via AudioToolbox on macOS
|
||||
target_link_libraries(spacetris PRIVATE "-framework AudioToolbox" "-framework CoreFoundation")
|
||||
endif()
|
||||
|
||||
# Include production build configuration
|
||||
@ -141,22 +189,23 @@ enable_testing()
|
||||
|
||||
# Unit tests (simple runner)
|
||||
find_package(Catch2 CONFIG REQUIRED)
|
||||
add_executable(tetris_tests
|
||||
add_executable(spacetris_tests
|
||||
tests/GravityTests.cpp
|
||||
src/core/GravityManager.cpp
|
||||
)
|
||||
target_include_directories(tetris_tests PRIVATE ${CMAKE_SOURCE_DIR}/src)
|
||||
target_link_libraries(tetris_tests PRIVATE Catch2::Catch2WithMain)
|
||||
add_test(NAME GravityTests COMMAND tetris_tests)
|
||||
target_include_directories(spacetris_tests PRIVATE ${CMAKE_SOURCE_DIR}/src)
|
||||
target_link_libraries(spacetris_tests PRIVATE Catch2::Catch2WithMain)
|
||||
add_test(NAME GravityTests COMMAND spacetris_tests)
|
||||
|
||||
if(EXISTS "${CMAKE_SOURCE_DIR}/vcpkg_installed/x64-windows/include")
|
||||
target_include_directories(tetris_tests PRIVATE "${CMAKE_SOURCE_DIR}/vcpkg_installed/x64-windows/include")
|
||||
target_include_directories(spacetris_tests PRIVATE "${CMAKE_SOURCE_DIR}/vcpkg_installed/x64-windows/include")
|
||||
endif()
|
||||
|
||||
# Add new src subfolders to include path so old #includes continue to work
|
||||
target_include_directories(tetris PRIVATE
|
||||
target_include_directories(spacetris PRIVATE
|
||||
${CMAKE_SOURCE_DIR}/src
|
||||
${CMAKE_SOURCE_DIR}/src/audio
|
||||
${CMAKE_SOURCE_DIR}/src/video
|
||||
${CMAKE_SOURCE_DIR}/src/gameplay
|
||||
${CMAKE_SOURCE_DIR}/src/graphics
|
||||
${CMAKE_SOURCE_DIR}/src/persistence
|
||||
|
||||
760
CODE_ANALYSIS.md
@ -1,760 +0,0 @@
|
||||
# Tetris SDL3 - Code Analysis & Best Practices Review
|
||||
|
||||
**Generated:** 2025-12-03
|
||||
**Project:** Tetris Game (SDL3)
|
||||
|
||||
---
|
||||
|
||||
## 📊 Executive Summary
|
||||
|
||||
Your Tetris project is **well-structured and follows many modern C++ best practices**. The codebase demonstrates:
|
||||
- ✅ Clean separation of concerns with a state-based architecture
|
||||
- ✅ Modern C++20 features and RAII patterns
|
||||
- ✅ Centralized configuration management
|
||||
- ✅ Proper dependency management via vcpkg
|
||||
- ✅ Good documentation and code organization
|
||||
|
||||
However, there are opportunities for improvement in areas like memory management, error handling, and code duplication.
|
||||
|
||||
---
|
||||
|
||||
## 🎯 Strengths
|
||||
|
||||
### 1. **Architecture & Design Patterns**
|
||||
- **State Pattern Implementation**: Clean state management with `MenuState`, `PlayingState`, `OptionsState`, `LevelSelectorState`, and `LoadingState`
|
||||
- **Separation of Concerns**: Game logic (`Game.cpp`), rendering (`GameRenderer`, `UIRenderer`), audio (`Audio`, `SoundEffect`), and persistence (`Scores`) are well-separated
|
||||
- **Centralized Configuration**: `Config.h` provides a single source of truth for constants, eliminating magic numbers
|
||||
- **Service Locator Pattern**: `StateContext` acts as a dependency injection container
|
||||
|
||||
### 2. **Modern C++ Practices**
|
||||
- **C++20 Standard**: Using modern features like `std::filesystem`, `std::jthread`
|
||||
- **RAII**: Proper resource management with smart pointers and automatic cleanup
|
||||
- **Type Safety**: Strong typing with enums (`PieceType`, `AppState`, `LevelBackgroundPhase`)
|
||||
- **Const Correctness**: Good use of `const` methods and references
|
||||
|
||||
### 3. **Code Organization**
|
||||
```
|
||||
src/
|
||||
├── audio/ # Audio system (music, sound effects)
|
||||
├── core/ # Core systems (state management, settings, global state)
|
||||
├── gameplay/ # Game logic (Tetris mechanics, effects)
|
||||
├── graphics/ # Rendering (UI, game renderer, effects)
|
||||
├── persistence/ # Score management
|
||||
├── states/ # State implementations
|
||||
└── utils/ # Utilities
|
||||
```
|
||||
This structure is logical and easy to navigate.
|
||||
|
||||
### 4. **Build System**
|
||||
- **CMake**: Modern CMake with proper target configuration
|
||||
- **vcpkg**: Excellent dependency management
|
||||
- **Cross-platform**: Support for Windows and macOS
|
||||
- **Testing**: Catch2 integration for unit tests
|
||||
|
||||
---
|
||||
|
||||
## ⚠️ Areas for Improvement
|
||||
|
||||
### 1. **Memory Management Issues**
|
||||
|
||||
#### **Problem: Raw Pointer Usage**
|
||||
**Location:** `MenuState.h`, `main.cpp`
|
||||
```cpp
|
||||
// MenuState.h (lines 17-21)
|
||||
SDL_Texture* playIcon = nullptr;
|
||||
SDL_Texture* levelIcon = nullptr;
|
||||
SDL_Texture* optionsIcon = nullptr;
|
||||
SDL_Texture* exitIcon = nullptr;
|
||||
```
|
||||
|
||||
**Issue:** Raw pointers to SDL resources without proper cleanup in all code paths.
|
||||
|
||||
**Recommendation:**
|
||||
```cpp
|
||||
// Create a smart pointer wrapper for SDL_Texture
|
||||
struct SDL_TextureDeleter {
|
||||
void operator()(SDL_Texture* tex) const {
|
||||
if (tex) SDL_DestroyTexture(tex);
|
||||
}
|
||||
};
|
||||
using SDL_TexturePtr = std::unique_ptr<SDL_Texture, SDL_TextureDeleter>;
|
||||
|
||||
// Usage in MenuState.h
|
||||
private:
|
||||
SDL_TexturePtr playIcon;
|
||||
SDL_TexturePtr levelIcon;
|
||||
SDL_TexturePtr optionsIcon;
|
||||
SDL_TexturePtr exitIcon;
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
- Automatic cleanup
|
||||
- Exception safety
|
||||
- No manual memory management
|
||||
- Clear ownership semantics
|
||||
|
||||
---
|
||||
|
||||
### 2. **Error Handling**
|
||||
|
||||
#### **Problem: Inconsistent Error Handling**
|
||||
**Location:** `main.cpp` (lines 86-114)
|
||||
```cpp
|
||||
static SDL_Texture* loadTextureFromImage(SDL_Renderer* renderer, const std::string& path, int* outW = nullptr, int* outH = nullptr) {
|
||||
if (!renderer) {
|
||||
return nullptr; // Silent failure
|
||||
}
|
||||
|
||||
SDL_Surface* surface = IMG_Load(resolvedPath.c_str());
|
||||
if (!surface) {
|
||||
SDL_LogError(...); // Logs but returns nullptr
|
||||
return nullptr;
|
||||
}
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
**Issues:**
|
||||
- Silent failures make debugging difficult
|
||||
- Callers must check for `nullptr` (easy to forget)
|
||||
- No way to distinguish between different error types
|
||||
|
||||
**Recommendation:**
|
||||
```cpp
|
||||
#include <expected> // C++23, or use tl::expected for C++20
|
||||
|
||||
struct TextureLoadError {
|
||||
std::string message;
|
||||
std::string path;
|
||||
};
|
||||
|
||||
std::expected<SDL_TexturePtr, TextureLoadError>
|
||||
loadTextureFromImage(SDL_Renderer* renderer, const std::string& path,
|
||||
int* outW = nullptr, int* outH = nullptr) {
|
||||
if (!renderer) {
|
||||
return std::unexpected(TextureLoadError{
|
||||
"Renderer is null", path
|
||||
});
|
||||
}
|
||||
|
||||
const std::string resolvedPath = AssetPath::resolveImagePath(path);
|
||||
SDL_Surface* surface = IMG_Load(resolvedPath.c_str());
|
||||
if (!surface) {
|
||||
return std::unexpected(TextureLoadError{
|
||||
std::string("Failed to load: ") + SDL_GetError(),
|
||||
resolvedPath
|
||||
});
|
||||
}
|
||||
|
||||
// ... success case
|
||||
return SDL_TexturePtr(texture);
|
||||
}
|
||||
|
||||
// Usage:
|
||||
auto result = loadTextureFromImage(renderer, "path.png");
|
||||
if (result) {
|
||||
// Use result.value()
|
||||
} else {
|
||||
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION,
|
||||
"Failed to load %s: %s",
|
||||
result.error().path.c_str(),
|
||||
result.error().message.c_str());
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 3. **Code Duplication**
|
||||
|
||||
#### **Problem: Repeated Patterns**
|
||||
**Location:** `MenuState.cpp`, `PlayingState.cpp`, `OptionsState.cpp`
|
||||
|
||||
Similar lambda patterns for exit popup handling:
|
||||
```cpp
|
||||
auto setExitSelection = [&](int value) {
|
||||
if (ctx.exitPopupSelectedButton) {
|
||||
*ctx.exitPopupSelectedButton = value;
|
||||
}
|
||||
};
|
||||
|
||||
auto getExitSelection = [&]() -> int {
|
||||
return ctx.exitPopupSelectedButton ? *ctx.exitPopupSelectedButton : 1;
|
||||
};
|
||||
```
|
||||
|
||||
**Recommendation:**
|
||||
Create a helper class in `StateContext`:
|
||||
```cpp
|
||||
// StateContext.h
|
||||
class ExitPopupHelper {
|
||||
public:
|
||||
ExitPopupHelper(int* selectedButton, bool* showPopup)
|
||||
: m_selectedButton(selectedButton), m_showPopup(showPopup) {}
|
||||
|
||||
void setSelection(int value) {
|
||||
if (m_selectedButton) *m_selectedButton = value;
|
||||
}
|
||||
|
||||
int getSelection() const {
|
||||
return m_selectedButton ? *m_selectedButton : 1;
|
||||
}
|
||||
|
||||
void show() {
|
||||
if (m_showPopup) *m_showPopup = true;
|
||||
}
|
||||
|
||||
void hide() {
|
||||
if (m_showPopup) *m_showPopup = false;
|
||||
}
|
||||
|
||||
bool isVisible() const {
|
||||
return m_showPopup && *m_showPopup;
|
||||
}
|
||||
|
||||
private:
|
||||
int* m_selectedButton;
|
||||
bool* m_showPopup;
|
||||
};
|
||||
|
||||
// Usage in states:
|
||||
ExitPopupHelper exitPopup(ctx.exitPopupSelectedButton, ctx.showExitConfirmPopup);
|
||||
exitPopup.setSelection(0);
|
||||
if (exitPopup.isVisible()) { ... }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 4. **Magic Numbers**
|
||||
|
||||
#### **Problem: Some Magic Numbers Still Present**
|
||||
**Location:** `MenuState.cpp` (lines 269-273)
|
||||
```cpp
|
||||
float btnW = 200.0f; // Fixed width to match background buttons
|
||||
float btnH = 70.0f; // Fixed height to match background buttons
|
||||
float btnX = LOGICAL_W * 0.5f + contentOffsetX;
|
||||
float btnY = LOGICAL_H * 0.865f + contentOffsetY;
|
||||
```
|
||||
|
||||
**Recommendation:**
|
||||
Add to `Config.h`:
|
||||
```cpp
|
||||
namespace Config::UI {
|
||||
constexpr float MENU_BUTTON_WIDTH = 200.0f;
|
||||
constexpr float MENU_BUTTON_HEIGHT = 70.0f;
|
||||
constexpr float MENU_BUTTON_Y_FRACTION = 0.865f;
|
||||
constexpr float MENU_BUTTON_SPACING = 210.0f;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 5. **File I/O for Debugging**
|
||||
|
||||
#### **Problem: Debug Logging to Files**
|
||||
**Location:** `MenuState.cpp` (lines 182-184, 195-203, etc.)
|
||||
```cpp
|
||||
FILE* f = fopen("tetris_trace.log", "a");
|
||||
if (f) {
|
||||
fprintf(f, "MenuState::render entry\n");
|
||||
fclose(f);
|
||||
}
|
||||
```
|
||||
|
||||
**Issues:**
|
||||
- File handles not checked properly
|
||||
- No error handling
|
||||
- Performance overhead in production
|
||||
- Should use proper logging framework
|
||||
|
||||
**Recommendation:**
|
||||
```cpp
|
||||
// Create a simple logger utility
|
||||
// src/utils/Logger.h
|
||||
#pragma once
|
||||
#include <string>
|
||||
#include <fstream>
|
||||
#include <mutex>
|
||||
|
||||
class Logger {
|
||||
public:
|
||||
enum class Level { TRACE, DEBUG, INFO, WARN, ERROR };
|
||||
|
||||
static Logger& instance();
|
||||
|
||||
void setLevel(Level level) { m_level = level; }
|
||||
void setFile(const std::string& path);
|
||||
|
||||
template<typename... Args>
|
||||
void trace(const char* fmt, Args... args) {
|
||||
log(Level::TRACE, fmt, args...);
|
||||
}
|
||||
|
||||
template<typename... Args>
|
||||
void debug(const char* fmt, Args... args) {
|
||||
log(Level::DEBUG, fmt, args...);
|
||||
}
|
||||
|
||||
private:
|
||||
Logger() = default;
|
||||
|
||||
template<typename... Args>
|
||||
void log(Level level, const char* fmt, Args... args);
|
||||
|
||||
Level m_level = Level::INFO;
|
||||
std::ofstream m_file;
|
||||
std::mutex m_mutex;
|
||||
};
|
||||
|
||||
// Usage:
|
||||
#ifdef DEBUG
|
||||
Logger::instance().trace("MenuState::render entry");
|
||||
#endif
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 6. **Const Correctness**
|
||||
|
||||
#### **Problem: Missing const in Some Places**
|
||||
**Location:** `StateContext` and various state methods
|
||||
|
||||
**Recommendation:**
|
||||
```cpp
|
||||
// State.h
|
||||
class State {
|
||||
public:
|
||||
virtual void render(SDL_Renderer* renderer, float logicalScale,
|
||||
SDL_Rect logicalVP) const = 0; // Add const
|
||||
// Render shouldn't modify state
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 7. **Thread Safety**
|
||||
|
||||
#### **Problem: Potential Race Conditions**
|
||||
**Location:** `Audio.cpp` - Background loading
|
||||
|
||||
**Current:**
|
||||
```cpp
|
||||
std::vector<AudioTrack> tracks;
|
||||
std::mutex tracksMutex;
|
||||
```
|
||||
|
||||
**Recommendation:**
|
||||
- Document thread safety guarantees
|
||||
- Use `std::shared_mutex` for read-heavy operations
|
||||
- Consider using lock-free data structures for performance-critical paths
|
||||
|
||||
```cpp
|
||||
// Audio.h
|
||||
class Audio {
|
||||
private:
|
||||
std::vector<AudioTrack> tracks;
|
||||
mutable std::shared_mutex tracksMutex; // Allow concurrent reads
|
||||
|
||||
public:
|
||||
// Read operation - shared lock
|
||||
int getLoadedTrackCount() const {
|
||||
std::shared_lock lock(tracksMutex);
|
||||
return tracks.size();
|
||||
}
|
||||
|
||||
// Write operation - exclusive lock
|
||||
void addTrack(const std::string& path) {
|
||||
std::unique_lock lock(tracksMutex);
|
||||
tracks.push_back(loadTrack(path));
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 8. **Testing Coverage**
|
||||
|
||||
#### **Current State:**
|
||||
Only one test file: `tests/GravityTests.cpp`
|
||||
|
||||
**Recommendation:**
|
||||
Add comprehensive tests:
|
||||
```
|
||||
tests/
|
||||
├── GravityTests.cpp ✅ Exists
|
||||
├── GameLogicTests.cpp ❌ Missing
|
||||
├── ScoreManagerTests.cpp ❌ Missing
|
||||
├── StateTransitionTests.cpp ❌ Missing
|
||||
└── AudioSystemTests.cpp ❌ Missing
|
||||
```
|
||||
|
||||
**Example Test Structure:**
|
||||
```cpp
|
||||
// tests/GameLogicTests.cpp
|
||||
#include <catch2/catch_test_macros.hpp>
|
||||
#include "gameplay/core/Game.h"
|
||||
|
||||
TEST_CASE("Game initialization", "[game]") {
|
||||
Game game(0);
|
||||
|
||||
SECTION("Board starts empty") {
|
||||
const auto& board = game.boardRef();
|
||||
REQUIRE(std::all_of(board.begin(), board.end(),
|
||||
[](int cell) { return cell == 0; }));
|
||||
}
|
||||
|
||||
SECTION("Score starts at zero") {
|
||||
REQUIRE(game.score() == 0);
|
||||
REQUIRE(game.lines() == 0);
|
||||
}
|
||||
}
|
||||
|
||||
TEST_CASE("Piece rotation", "[game]") {
|
||||
Game game(0);
|
||||
|
||||
SECTION("Clockwise rotation") {
|
||||
auto initialRot = game.current().rot;
|
||||
game.rotate(1);
|
||||
REQUIRE(game.current().rot == (initialRot + 1) % 4);
|
||||
}
|
||||
}
|
||||
|
||||
TEST_CASE("Line clearing", "[game]") {
|
||||
Game game(0);
|
||||
|
||||
SECTION("Single line clear awards correct score") {
|
||||
// Setup: Fill bottom row except one cell
|
||||
// ... test implementation
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 9. **Documentation**
|
||||
|
||||
#### **Current State:**
|
||||
- Good inline comments
|
||||
- Config.h has excellent documentation
|
||||
- Missing: API documentation, architecture overview
|
||||
|
||||
**Recommendation:**
|
||||
Add Doxygen-style comments:
|
||||
```cpp
|
||||
/**
|
||||
* @class Game
|
||||
* @brief Core Tetris game logic engine
|
||||
*
|
||||
* Manages the game board, piece spawning, collision detection,
|
||||
* line clearing, and scoring. This class is independent of
|
||||
* rendering and input handling.
|
||||
*
|
||||
* @note Thread-safe for read operations, but write operations
|
||||
* (move, rotate, etc.) should only be called from the
|
||||
* main game thread.
|
||||
*
|
||||
* Example usage:
|
||||
* @code
|
||||
* Game game(5); // Start at level 5
|
||||
* game.tickGravity(16.67); // Update for one frame
|
||||
* if (game.isGameOver()) {
|
||||
* // Handle game over
|
||||
* }
|
||||
* @endcode
|
||||
*/
|
||||
class Game {
|
||||
// ...
|
||||
};
|
||||
```
|
||||
|
||||
Create `docs/ARCHITECTURE.md`:
|
||||
```markdown
|
||||
# Architecture Overview
|
||||
|
||||
## State Machine
|
||||
[Diagram of state transitions]
|
||||
|
||||
## Data Flow
|
||||
[Diagram showing how data flows through the system]
|
||||
|
||||
## Threading Model
|
||||
- Main thread: Rendering, input, game logic
|
||||
- Background thread: Audio loading
|
||||
- Audio callback thread: Audio mixing
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 10. **Performance Considerations**
|
||||
|
||||
#### **Issue: Frequent String Allocations**
|
||||
**Location:** Various places using `std::string` for paths
|
||||
|
||||
**Recommendation:**
|
||||
```cpp
|
||||
// Use string_view for read-only string parameters
|
||||
#include <string_view>
|
||||
|
||||
SDL_Texture* loadTextureFromImage(SDL_Renderer* renderer,
|
||||
std::string_view path, // Changed
|
||||
int* outW = nullptr,
|
||||
int* outH = nullptr);
|
||||
|
||||
// For compile-time strings, use constexpr
|
||||
namespace AssetPaths {
|
||||
constexpr std::string_view LOGO = "assets/images/logo.bmp";
|
||||
constexpr std::string_view BACKGROUND = "assets/images/main_background.bmp";
|
||||
}
|
||||
```
|
||||
|
||||
#### **Issue: Vector Reallocations**
|
||||
**Location:** `fireworks` vector in `main.cpp`
|
||||
|
||||
**Recommendation:**
|
||||
```cpp
|
||||
// Reserve capacity upfront
|
||||
fireworks.reserve(5); // Max 5 fireworks at once
|
||||
|
||||
// Or use a fixed-size container
|
||||
std::array<std::optional<TetrisFirework>, 5> fireworks;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🔧 Specific Recommendations by Priority
|
||||
|
||||
### **High Priority** (Do These First)
|
||||
|
||||
1. **Replace raw SDL pointers with smart pointers**
|
||||
- Impact: Prevents memory leaks
|
||||
- Effort: Medium
|
||||
- Files: `MenuState.h`, `main.cpp`, all state files
|
||||
|
||||
2. **Remove debug file I/O from production code**
|
||||
- Impact: Performance, code cleanliness
|
||||
- Effort: Low
|
||||
- Files: `MenuState.cpp`, `main.cpp`
|
||||
|
||||
3. **Add error handling to asset loading**
|
||||
- Impact: Better debugging, crash prevention
|
||||
- Effort: Medium
|
||||
- Files: `main.cpp`, `AssetManager.cpp`
|
||||
|
||||
### **Medium Priority**
|
||||
|
||||
4. **Extract common patterns into helper classes**
|
||||
- Impact: Code maintainability
|
||||
- Effort: Medium
|
||||
- Files: All state files
|
||||
|
||||
5. **Move remaining magic numbers to Config.h**
|
||||
- Impact: Maintainability
|
||||
- Effort: Low
|
||||
- Files: `MenuState.cpp`, `UIRenderer.cpp`
|
||||
|
||||
6. **Add comprehensive unit tests**
|
||||
- Impact: Code quality, regression prevention
|
||||
- Effort: High
|
||||
- Files: New test files
|
||||
|
||||
### **Low Priority** (Nice to Have)
|
||||
|
||||
7. **Add Doxygen documentation**
|
||||
- Impact: Developer onboarding
|
||||
- Effort: Medium
|
||||
|
||||
8. **Performance profiling and optimization**
|
||||
- Impact: Depends on current performance
|
||||
- Effort: Medium
|
||||
|
||||
9. **Consider using `std::expected` for error handling**
|
||||
- Impact: Better error handling
|
||||
- Effort: High (requires C++23 or external library)
|
||||
|
||||
---
|
||||
|
||||
## 📝 Code Style Observations
|
||||
|
||||
### **Good Practices You're Already Following:**
|
||||
|
||||
✅ **Consistent naming conventions:**
|
||||
- Classes: `PascalCase` (e.g., `MenuState`, `GameRenderer`)
|
||||
- Functions: `camelCase` (e.g., `tickGravity`, `loadTexture`)
|
||||
- Constants: `UPPER_SNAKE_CASE` (e.g., `LOGICAL_W`, `DAS_DELAY`)
|
||||
- Member variables: `camelCase` with `m_` prefix in some places
|
||||
|
||||
✅ **Header guards:** Using `#pragma once`
|
||||
|
||||
✅ **Forward declarations:** Minimizing include dependencies
|
||||
|
||||
✅ **RAII:** Resources tied to object lifetime
|
||||
|
||||
### **Minor Style Inconsistencies:**
|
||||
|
||||
❌ **Inconsistent member variable naming:**
|
||||
```cpp
|
||||
// Some classes use m_ prefix
|
||||
float m_masterVolume = 1.0f;
|
||||
|
||||
// Others don't
|
||||
int selectedButton = 0;
|
||||
```
|
||||
|
||||
**Recommendation:** Pick one style and stick to it. I suggest:
|
||||
```cpp
|
||||
// Private members: m_ prefix
|
||||
float m_masterVolume = 1.0f;
|
||||
int m_selectedButton = 0;
|
||||
|
||||
// Public members: no prefix (rare in good design)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🎨 Architecture Suggestions
|
||||
|
||||
### **Consider Implementing:**
|
||||
|
||||
1. **Event System**
|
||||
Instead of callbacks, use an event bus:
|
||||
```cpp
|
||||
// events/GameEvents.h
|
||||
struct LineClearedEvent {
|
||||
int linesCleared;
|
||||
int newScore;
|
||||
};
|
||||
|
||||
struct LevelUpEvent {
|
||||
int newLevel;
|
||||
};
|
||||
|
||||
// EventBus.h
|
||||
class EventBus {
|
||||
public:
|
||||
template<typename Event>
|
||||
void subscribe(std::function<void(const Event&)> handler);
|
||||
|
||||
template<typename Event>
|
||||
void publish(const Event& event);
|
||||
};
|
||||
|
||||
// Usage in Game.cpp
|
||||
eventBus.publish(LineClearedEvent{linesCleared, _score});
|
||||
|
||||
// Usage in Audio system
|
||||
eventBus.subscribe<LineClearedEvent>([](const auto& e) {
|
||||
playLineClearSound(e.linesCleared);
|
||||
});
|
||||
```
|
||||
|
||||
2. **Component-Based UI**
|
||||
Extract UI components:
|
||||
```cpp
|
||||
class Button {
|
||||
public:
|
||||
void render(SDL_Renderer* renderer);
|
||||
bool isHovered(int mouseX, int mouseY) const;
|
||||
void onClick(std::function<void()> callback);
|
||||
};
|
||||
|
||||
class Panel {
|
||||
std::vector<std::unique_ptr<UIComponent>> children;
|
||||
};
|
||||
```
|
||||
|
||||
3. **Asset Manager**
|
||||
Centralize asset loading:
|
||||
```cpp
|
||||
class AssetManager {
|
||||
public:
|
||||
SDL_TexturePtr getTexture(std::string_view name);
|
||||
FontAtlas* getFont(std::string_view name);
|
||||
|
||||
private:
|
||||
std::unordered_map<std::string, SDL_TexturePtr> textures;
|
||||
std::unordered_map<std::string, std::unique_ptr<FontAtlas>> fonts;
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🔍 Security Considerations
|
||||
|
||||
1. **File Path Validation**
|
||||
```cpp
|
||||
// AssetPath::resolveImagePath should validate paths
|
||||
// to prevent directory traversal attacks
|
||||
std::string resolveImagePath(std::string_view path) {
|
||||
// Reject paths with ".."
|
||||
if (path.find("..") != std::string_view::npos) {
|
||||
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION,
|
||||
"Invalid path: %s", path.data());
|
||||
return "";
|
||||
}
|
||||
// ... rest of implementation
|
||||
}
|
||||
```
|
||||
|
||||
2. **Score File Tampering**
|
||||
Consider adding checksums to score files:
|
||||
```cpp
|
||||
// Scores.cpp
|
||||
void ScoreManager::save() const {
|
||||
nlohmann::json j;
|
||||
j["scores"] = scores;
|
||||
j["checksum"] = computeChecksum(scores);
|
||||
// ... save to file
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 📊 Metrics
|
||||
|
||||
Based on the codebase analysis:
|
||||
|
||||
| Metric | Value | Rating |
|
||||
|--------|-------|--------|
|
||||
| **Code Organization** | Excellent | ⭐⭐⭐⭐⭐ |
|
||||
| **Modern C++ Usage** | Very Good | ⭐⭐⭐⭐ |
|
||||
| **Error Handling** | Fair | ⭐⭐⭐ |
|
||||
| **Memory Safety** | Good | ⭐⭐⭐⭐ |
|
||||
| **Test Coverage** | Poor | ⭐ |
|
||||
| **Documentation** | Good | ⭐⭐⭐⭐ |
|
||||
| **Performance** | Good | ⭐⭐⭐⭐ |
|
||||
| **Maintainability** | Very Good | ⭐⭐⭐⭐ |
|
||||
|
||||
**Overall Score: 4/5 ⭐⭐⭐⭐**
|
||||
|
||||
---
|
||||
|
||||
## 🚀 Quick Wins (Easy Improvements)
|
||||
|
||||
1. **Add `.clang-format` file** for consistent formatting
|
||||
2. **Create `CONTRIBUTING.md`** with coding guidelines
|
||||
3. **Add pre-commit hooks** for formatting and linting
|
||||
4. **Set up GitHub Actions** for CI/CD
|
||||
5. **Add `README.md`** with build instructions and screenshots
|
||||
|
||||
---
|
||||
|
||||
## 📚 Recommended Resources
|
||||
|
||||
- **Modern C++ Best Practices:** https://isocpp.github.io/CppCoreGuidelines/
|
||||
- **SDL3 Migration Guide:** https://wiki.libsdl.org/SDL3/README/migration
|
||||
- **Game Programming Patterns:** https://gameprogrammingpatterns.com/
|
||||
- **C++ Testing with Catch2:** https://github.com/catchorg/Catch2
|
||||
|
||||
---
|
||||
|
||||
## ✅ Conclusion
|
||||
|
||||
Your Tetris project demonstrates **strong software engineering practices** with a clean architecture, modern C++ usage, and good separation of concerns. The main areas for improvement are:
|
||||
|
||||
1. Enhanced error handling
|
||||
2. Increased test coverage
|
||||
3. Elimination of raw pointers
|
||||
4. Removal of debug code from production
|
||||
|
||||
With these improvements, this codebase would be **production-ready** and serve as an excellent example of modern C++ game development.
|
||||
|
||||
**Keep up the excellent work!** 🎮
|
||||
@ -1,4 +1,4 @@
|
||||
# Tetris SDL3 - Production Deployment Guide
|
||||
# Spacetris SDL3 - Production Deployment Guide
|
||||
|
||||
## 🚀 Build Scripts Available
|
||||
|
||||
@ -9,7 +9,7 @@
|
||||
- Uses existing Debug/Release build from build-msvc
|
||||
- Creates distribution package with all dependencies
|
||||
- **Size: ~939 MB** (includes all assets and music)
|
||||
- **Output:** `dist/TetrisGame/` + ZIP file
|
||||
- **Output:** `dist/SpacetrisGame/` + ZIP file
|
||||
|
||||
### 2. Full Production Build
|
||||
```powershell
|
||||
@ -31,8 +31,8 @@ build-production.bat
|
||||
The distribution package includes:
|
||||
|
||||
### Essential Files
|
||||
- ✅ `tetris.exe` - Main game executable
|
||||
- ✅ `Launch-Tetris.bat` - Safe launcher with error handling
|
||||
- ✅ `spacetris.exe` - Main game executable
|
||||
- ✅ `Launch-Spacetris.bat` - Safe launcher with error handling
|
||||
- ✅ `README.txt` - User instructions
|
||||
|
||||
### Dependencies
|
||||
@ -49,9 +49,9 @@ The distribution package includes:
|
||||
## 🎯 Distribution Options
|
||||
|
||||
### Option 1: ZIP Archive (Recommended)
|
||||
- **File:** `TetrisGame-YYYY.MM.DD.zip`
|
||||
- **File:** `SpacetrisGame-YYYY.MM.DD.zip`
|
||||
- **Size:** ~939 MB
|
||||
- **Usage:** Users extract and run `Launch-Tetris.bat`
|
||||
- **Usage:** Users extract and run `Launch-Spacetris.bat`
|
||||
|
||||
### Option 2: Installer (Future)
|
||||
- Use CMake CPack to create NSIS installer
|
||||
@ -60,14 +60,14 @@ The distribution package includes:
|
||||
```
|
||||
|
||||
### Option 3: Portable Folder
|
||||
- Direct distribution of `dist/TetrisGame/` folder
|
||||
Direct distribution of `dist/SpacetrisGame/` folder
|
||||
- Users copy folder and run executable
|
||||
|
||||
[ ] Launch-Spacetris.bat works
|
||||
## 🔧 Build Requirements
|
||||
|
||||
**Solution:** Use `Launch-Spacetris.bat` for better error reporting
|
||||
### Development Environment
|
||||
- **CMake** 3.20+
|
||||
- **Visual Studio 2022** (or compatible C++ compiler)
|
||||
- **Visual Studio 2026 (VS 18)** with Desktop development with C++ workload
|
||||
- **vcpkg** package manager
|
||||
- **SDL3** libraries (installed via vcpkg)
|
||||
|
||||
@ -87,18 +87,21 @@ vcpkg install sdl3 sdl3-ttf --triplet=x64-windows
|
||||
- [ ] Font rendering works (both FreeSans and PressStart2P)
|
||||
- [ ] Game saves high scores
|
||||
|
||||
### Package Validation
|
||||
### Package Validation
|
||||
- [ ] All DLL files present
|
||||
- [ ] Assets folder complete
|
||||
- [ ] Launch-Tetris.bat works
|
||||
- [ ] Launch-Spacetris.bat works
|
||||
- [ ] README.txt is informative
|
||||
- [ ] Package size reasonable (~939 MB)
|
||||
|
||||
### Distribution
|
||||
- [ ] ZIP file created successfully
|
||||
### "spacetris.exe is not recognized"
|
||||
- **Solution:** Ensure all DLL files are in same folder as executable
|
||||
- [ ] Test extraction on clean system
|
||||
- [ ] Verify game runs on target machines
|
||||
- [ ] No missing dependencies
|
||||
### Game won't start
|
||||
- **Solution:** Use `Launch-Spacetris.bat` for better error reporting
|
||||
|
||||
## 📋 User System Requirements
|
||||
|
||||
@ -118,7 +121,7 @@ vcpkg install sdl3 sdl3-ttf --triplet=x64-windows
|
||||
|
||||
## 🐛 Common Issues & Solutions
|
||||
|
||||
### "tetris.exe is not recognized"
|
||||
### "spacetris.exe is not recognized"
|
||||
- **Solution:** Ensure all DLL files are in same folder as executable
|
||||
|
||||
### "Failed to initialize SDL"
|
||||
|
||||
@ -1,363 +0,0 @@
|
||||
# Tetris SDL3 - Improvements Checklist
|
||||
|
||||
Quick reference for implementing the recommendations from CODE_ANALYSIS.md
|
||||
|
||||
---
|
||||
|
||||
## 🔴 High Priority (Critical)
|
||||
|
||||
### 1. Smart Pointer Wrapper for SDL Resources
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 2-3 hours
|
||||
**Impact:** Prevents memory leaks, improves safety
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `src/utils/SDLPointers.h` with smart pointer wrappers
|
||||
- [ ] Replace raw `SDL_Texture*` in `MenuState.h` (lines 17-21)
|
||||
- [ ] Replace raw `SDL_Texture*` in `PlayingState.h`
|
||||
- [ ] Update `main.cpp` texture loading
|
||||
- [ ] Test all states to ensure no regressions
|
||||
|
||||
**Code Template:**
|
||||
```cpp
|
||||
// src/utils/SDLPointers.h
|
||||
#pragma once
|
||||
#include <SDL3/SDL.h>
|
||||
#include <memory>
|
||||
|
||||
struct SDL_TextureDeleter {
|
||||
void operator()(SDL_Texture* tex) const {
|
||||
if (tex) SDL_DestroyTexture(tex);
|
||||
}
|
||||
};
|
||||
|
||||
struct SDL_SurfaceDeleter {
|
||||
void operator()(SDL_Surface* surf) const {
|
||||
if (surf) SDL_DestroySurface(surf);
|
||||
}
|
||||
};
|
||||
|
||||
using SDL_TexturePtr = std::unique_ptr<SDL_Texture, SDL_TextureDeleter>;
|
||||
using SDL_SurfacePtr = std::unique_ptr<SDL_Surface, SDL_SurfaceDeleter>;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 2. Remove Debug File I/O
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 30 minutes
|
||||
**Impact:** Performance, code cleanliness
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Remove or wrap `fopen("tetris_trace.log")` calls in `MenuState.cpp`
|
||||
- [ ] Remove or wrap similar calls in other files
|
||||
- [ ] Replace with SDL_LogTrace or conditional compilation
|
||||
- [ ] Delete `tetris_trace.log` from repository
|
||||
|
||||
**Files to Update:**
|
||||
- `src/states/MenuState.cpp` (lines 182-184, 195-203, 277-278, 335-337)
|
||||
- `src/main.cpp` (if any similar patterns exist)
|
||||
|
||||
**Replacement Pattern:**
|
||||
```cpp
|
||||
// Before:
|
||||
FILE* f = fopen("tetris_trace.log", "a");
|
||||
if (f) { fprintf(f, "MenuState::render entry\n"); fclose(f); }
|
||||
|
||||
// After:
|
||||
SDL_LogTrace(SDL_LOG_CATEGORY_APPLICATION, "MenuState::render entry");
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 3. Improve Error Handling in Asset Loading
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 2 hours
|
||||
**Impact:** Better debugging, prevents crashes
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Update `loadTextureFromImage` to return error information
|
||||
- [ ] Add validation for all asset loads in `main.cpp`
|
||||
- [ ] Create fallback assets for missing resources
|
||||
- [ ] Add startup asset validation
|
||||
|
||||
**Example:**
|
||||
```cpp
|
||||
struct AssetLoadResult {
|
||||
SDL_TexturePtr texture;
|
||||
std::string error;
|
||||
bool success;
|
||||
};
|
||||
|
||||
AssetLoadResult loadTextureFromImage(SDL_Renderer* renderer,
|
||||
const std::string& path);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Medium Priority (Important)
|
||||
|
||||
### 4. Extract Common Patterns
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 3-4 hours
|
||||
**Impact:** Reduces code duplication
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `ExitPopupHelper` class in `StateContext.h`
|
||||
- [ ] Update `MenuState.cpp` to use helper
|
||||
- [ ] Update `PlayingState.cpp` to use helper
|
||||
- [ ] Update `OptionsState.cpp` to use helper
|
||||
|
||||
---
|
||||
|
||||
### 5. Move Magic Numbers to Config.h
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 1 hour
|
||||
**Impact:** Maintainability
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Add menu button constants to `Config::UI`
|
||||
- [ ] Add rendering constants to appropriate namespace
|
||||
- [ ] Update `MenuState.cpp` to use config constants
|
||||
- [ ] Update `UIRenderer.cpp` to use config constants
|
||||
|
||||
**Constants to Add:**
|
||||
```cpp
|
||||
namespace Config::UI {
|
||||
constexpr float MENU_BUTTON_WIDTH = 200.0f;
|
||||
constexpr float MENU_BUTTON_HEIGHT = 70.0f;
|
||||
constexpr float MENU_BUTTON_Y_FRACTION = 0.865f;
|
||||
constexpr float MENU_BUTTON_SPACING = 210.0f;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 6. Add Unit Tests
|
||||
**Status:** ⚠️ Minimal (only GravityTests)
|
||||
**Effort:** 8-10 hours
|
||||
**Impact:** Code quality, regression prevention
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `tests/GameLogicTests.cpp`
|
||||
- [ ] Test piece spawning
|
||||
- [ ] Test rotation
|
||||
- [ ] Test collision detection
|
||||
- [ ] Test line clearing
|
||||
- [ ] Test scoring
|
||||
- [ ] Create `tests/ScoreManagerTests.cpp`
|
||||
- [ ] Test score submission
|
||||
- [ ] Test high score detection
|
||||
- [ ] Test persistence
|
||||
- [ ] Create `tests/StateTransitionTests.cpp`
|
||||
- [ ] Test state transitions
|
||||
- [ ] Test state lifecycle (onEnter/onExit)
|
||||
- [ ] Update CMakeLists.txt to include new tests
|
||||
|
||||
---
|
||||
|
||||
## 🟢 Low Priority (Nice to Have)
|
||||
|
||||
### 7. Add Doxygen Documentation
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 4-6 hours
|
||||
**Impact:** Developer onboarding
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `Doxyfile` configuration
|
||||
- [ ] Add class-level documentation to core classes
|
||||
- [ ] Add function-level documentation to public APIs
|
||||
- [ ] Generate HTML documentation
|
||||
- [ ] Add to build process
|
||||
|
||||
---
|
||||
|
||||
### 8. Performance Profiling
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 4-6 hours
|
||||
**Impact:** Depends on findings
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Profile with Visual Studio Profiler / Instruments
|
||||
- [ ] Identify hotspots
|
||||
- [ ] Optimize critical paths
|
||||
- [ ] Add performance benchmarks
|
||||
|
||||
---
|
||||
|
||||
### 9. Standardize Member Variable Naming
|
||||
**Status:** ⚠️ Inconsistent
|
||||
**Effort:** 2-3 hours
|
||||
**Impact:** Code consistency
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Decide on naming convention (recommend `m_` prefix for private members)
|
||||
- [ ] Update all class member variables
|
||||
- [ ] Update documentation to reflect convention
|
||||
|
||||
**Convention Recommendation:**
|
||||
```cpp
|
||||
class Example {
|
||||
public:
|
||||
int publicValue; // No prefix for public members
|
||||
|
||||
private:
|
||||
int m_privateValue; // m_ prefix for private members
|
||||
float m_memberVariable; // Consistent across all classes
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 📋 Code Quality Improvements
|
||||
|
||||
### 10. Add .clang-format
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 15 minutes
|
||||
**Impact:** Consistent formatting
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `.clang-format` file in project root
|
||||
- [ ] Run formatter on all source files
|
||||
- [ ] Add format check to CI/CD
|
||||
|
||||
**Suggested .clang-format:**
|
||||
```yaml
|
||||
BasedOnStyle: LLVM
|
||||
IndentWidth: 4
|
||||
ColumnLimit: 120
|
||||
PointerAlignment: Left
|
||||
AllowShortFunctionsOnASingleLine: Empty
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 11. Add README.md
|
||||
**Status:** ❌ Missing
|
||||
**Effort:** 1 hour
|
||||
**Impact:** Project documentation
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `README.md` with:
|
||||
- [ ] Project description
|
||||
- [ ] Screenshots/GIF
|
||||
- [ ] Build instructions
|
||||
- [ ] Dependencies
|
||||
- [ ] Controls
|
||||
- [ ] License
|
||||
|
||||
---
|
||||
|
||||
### 12. Set Up CI/CD
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 2-3 hours
|
||||
**Impact:** Automated testing
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `.github/workflows/build.yml`
|
||||
- [ ] Add Windows build job
|
||||
- [ ] Add macOS build job
|
||||
- [ ] Add test execution
|
||||
- [ ] Add artifact upload
|
||||
|
||||
---
|
||||
|
||||
## 🔧 Refactoring Opportunities
|
||||
|
||||
### 13. Create Asset Manager
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 4-5 hours
|
||||
**Impact:** Better resource management
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `src/core/assets/AssetManager.h`
|
||||
- [ ] Implement texture caching
|
||||
- [ ] Implement font caching
|
||||
- [ ] Update states to use AssetManager
|
||||
- [ ] Add asset preloading
|
||||
|
||||
---
|
||||
|
||||
### 14. Implement Event System
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 6-8 hours
|
||||
**Impact:** Decoupling, flexibility
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `src/core/events/EventBus.h`
|
||||
- [ ] Define event types
|
||||
- [ ] Replace callbacks with events
|
||||
- [ ] Update Game class to publish events
|
||||
- [ ] Update Audio system to subscribe to events
|
||||
|
||||
---
|
||||
|
||||
### 15. Component-Based UI
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 8-10 hours
|
||||
**Impact:** UI maintainability
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `src/ui/components/Button.h`
|
||||
- [ ] Create `src/ui/components/Panel.h`
|
||||
- [ ] Create `src/ui/components/Label.h`
|
||||
- [ ] Refactor MenuState to use components
|
||||
- [ ] Refactor OptionsState to use components
|
||||
|
||||
---
|
||||
|
||||
## 📊 Progress Tracking
|
||||
|
||||
| Category | Total Items | Completed | In Progress | Not Started |
|
||||
|----------|-------------|-----------|-------------|-------------|
|
||||
| High Priority | 3 | 0 | 0 | 3 |
|
||||
| Medium Priority | 3 | 0 | 0 | 3 |
|
||||
| Low Priority | 3 | 0 | 0 | 3 |
|
||||
| Code Quality | 3 | 0 | 0 | 3 |
|
||||
| Refactoring | 3 | 0 | 0 | 3 |
|
||||
| **TOTAL** | **15** | **0** | **0** | **15** |
|
||||
|
||||
---
|
||||
|
||||
## 🎯 Suggested Implementation Order
|
||||
|
||||
### Week 1: Critical Fixes
|
||||
1. Remove debug file I/O (30 min)
|
||||
2. Smart pointer wrapper (2-3 hours)
|
||||
3. Improve error handling (2 hours)
|
||||
|
||||
### Week 2: Code Quality
|
||||
4. Move magic numbers to Config.h (1 hour)
|
||||
5. Extract common patterns (3-4 hours)
|
||||
6. Add .clang-format (15 min)
|
||||
7. Add README.md (1 hour)
|
||||
|
||||
### Week 3: Testing
|
||||
8. Add GameLogicTests (4 hours)
|
||||
9. Add ScoreManagerTests (2 hours)
|
||||
10. Add StateTransitionTests (2 hours)
|
||||
|
||||
### Week 4: Documentation & CI
|
||||
11. Set up CI/CD (2-3 hours)
|
||||
12. Add Doxygen documentation (4-6 hours)
|
||||
|
||||
### Future Iterations:
|
||||
13. Performance profiling
|
||||
14. Asset Manager
|
||||
15. Event System
|
||||
16. Component-Based UI
|
||||
|
||||
---
|
||||
|
||||
## 📝 Notes
|
||||
|
||||
- Mark items as completed by changing ❌ to ✅
|
||||
- Update progress table as you complete items
|
||||
- Feel free to reorder based on your priorities
|
||||
- Some items can be done in parallel
|
||||
- Consider creating GitHub issues for tracking
|
||||
|
||||
---
|
||||
|
||||
**Last Updated:** 2025-12-03
|
||||
**Next Review:** After completing High Priority items
|
||||
@ -1,774 +0,0 @@
|
||||
# Quick Start: Implementing Top 3 Improvements
|
||||
|
||||
This guide provides complete, copy-paste ready code for the three most impactful improvements.
|
||||
|
||||
---
|
||||
|
||||
## 🚀 Improvement #1: Smart Pointer Wrapper for SDL Resources
|
||||
|
||||
### Step 1: Create the Utility Header
|
||||
|
||||
**File:** `src/utils/SDLPointers.h`
|
||||
|
||||
```cpp
|
||||
#pragma once
|
||||
#include <SDL3/SDL.h>
|
||||
#include <memory>
|
||||
|
||||
/**
|
||||
* @file SDLPointers.h
|
||||
* @brief Smart pointer wrappers for SDL resources
|
||||
*
|
||||
* Provides RAII wrappers for SDL resources to prevent memory leaks
|
||||
* and ensure proper cleanup in all code paths.
|
||||
*/
|
||||
|
||||
namespace SDL {
|
||||
|
||||
/**
|
||||
* @brief Deleter for SDL_Texture
|
||||
*/
|
||||
struct TextureDeleter {
|
||||
void operator()(SDL_Texture* tex) const {
|
||||
if (tex) {
|
||||
SDL_DestroyTexture(tex);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* @brief Deleter for SDL_Surface
|
||||
*/
|
||||
struct SurfaceDeleter {
|
||||
void operator()(SDL_Surface* surf) const {
|
||||
if (surf) {
|
||||
SDL_DestroySurface(surf);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* @brief Deleter for SDL_Renderer
|
||||
*/
|
||||
struct RendererDeleter {
|
||||
void operator()(SDL_Renderer* renderer) const {
|
||||
if (renderer) {
|
||||
SDL_DestroyRenderer(renderer);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* @brief Deleter for SDL_Window
|
||||
*/
|
||||
struct WindowDeleter {
|
||||
void operator()(SDL_Window* window) const {
|
||||
if (window) {
|
||||
SDL_DestroyWindow(window);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* @brief Smart pointer for SDL_Texture
|
||||
*
|
||||
* Example usage:
|
||||
* @code
|
||||
* SDL::TexturePtr texture(SDL_CreateTexture(...));
|
||||
* if (!texture) {
|
||||
* // Handle error
|
||||
* }
|
||||
* // Automatic cleanup when texture goes out of scope
|
||||
* @endcode
|
||||
*/
|
||||
using TexturePtr = std::unique_ptr<SDL_Texture, TextureDeleter>;
|
||||
|
||||
/**
|
||||
* @brief Smart pointer for SDL_Surface
|
||||
*/
|
||||
using SurfacePtr = std::unique_ptr<SDL_Surface, SurfaceDeleter>;
|
||||
|
||||
/**
|
||||
* @brief Smart pointer for SDL_Renderer
|
||||
*/
|
||||
using RendererPtr = std::unique_ptr<SDL_Renderer, RendererDeleter>;
|
||||
|
||||
/**
|
||||
* @brief Smart pointer for SDL_Window
|
||||
*/
|
||||
using WindowPtr = std::unique_ptr<SDL_Window, WindowDeleter>;
|
||||
|
||||
} // namespace SDL
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 2: Update MenuState.h
|
||||
|
||||
**File:** `src/states/MenuState.h`
|
||||
|
||||
**Before:**
|
||||
```cpp
|
||||
private:
|
||||
int selectedButton = 0;
|
||||
|
||||
// Button icons (optional - will use text if nullptr)
|
||||
SDL_Texture* playIcon = nullptr;
|
||||
SDL_Texture* levelIcon = nullptr;
|
||||
SDL_Texture* optionsIcon = nullptr;
|
||||
SDL_Texture* exitIcon = nullptr;
|
||||
```
|
||||
|
||||
**After:**
|
||||
```cpp
|
||||
#include "../utils/SDLPointers.h" // Add this include
|
||||
|
||||
private:
|
||||
int selectedButton = 0;
|
||||
|
||||
// Button icons (optional - will use text if nullptr)
|
||||
SDL::TexturePtr playIcon;
|
||||
SDL::TexturePtr levelIcon;
|
||||
SDL::TexturePtr optionsIcon;
|
||||
SDL::TexturePtr exitIcon;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 3: Update MenuState.cpp
|
||||
|
||||
**File:** `src/states/MenuState.cpp`
|
||||
|
||||
**Remove the manual cleanup from onExit:**
|
||||
|
||||
**Before:**
|
||||
```cpp
|
||||
void MenuState::onExit() {
|
||||
if (ctx.showExitConfirmPopup) {
|
||||
*ctx.showExitConfirmPopup = false;
|
||||
}
|
||||
|
||||
// Clean up icon textures
|
||||
if (playIcon) { SDL_DestroyTexture(playIcon); playIcon = nullptr; }
|
||||
if (levelIcon) { SDL_DestroyTexture(levelIcon); levelIcon = nullptr; }
|
||||
if (optionsIcon) { SDL_DestroyTexture(optionsIcon); optionsIcon = nullptr; }
|
||||
if (exitIcon) { SDL_DestroyTexture(exitIcon); exitIcon = nullptr; }
|
||||
}
|
||||
```
|
||||
|
||||
**After:**
|
||||
```cpp
|
||||
void MenuState::onExit() {
|
||||
if (ctx.showExitConfirmPopup) {
|
||||
*ctx.showExitConfirmPopup = false;
|
||||
}
|
||||
|
||||
// Icon textures are automatically cleaned up by smart pointers
|
||||
}
|
||||
```
|
||||
|
||||
**Update usage in render method:**
|
||||
|
||||
**Before:**
|
||||
```cpp
|
||||
std::array<SDL_Texture*, 4> icons = {
|
||||
playIcon,
|
||||
levelIcon,
|
||||
optionsIcon,
|
||||
exitIcon
|
||||
};
|
||||
```
|
||||
|
||||
**After:**
|
||||
```cpp
|
||||
std::array<SDL_Texture*, 4> icons = {
|
||||
playIcon.get(),
|
||||
levelIcon.get(),
|
||||
optionsIcon.get(),
|
||||
exitIcon.get()
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 4: Update main.cpp Texture Loading
|
||||
|
||||
**File:** `src/main.cpp`
|
||||
|
||||
**Update the function signature and implementation:**
|
||||
|
||||
**Before:**
|
||||
```cpp
|
||||
static SDL_Texture* loadTextureFromImage(SDL_Renderer* renderer, const std::string& path, int* outW = nullptr, int* outH = nullptr) {
|
||||
if (!renderer) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
const std::string resolvedPath = AssetPath::resolveImagePath(path);
|
||||
SDL_Surface* surface = IMG_Load(resolvedPath.c_str());
|
||||
if (!surface) {
|
||||
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Failed to load image %s (resolved: %s): %s", path.c_str(), resolvedPath.c_str(), SDL_GetError());
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
if (outW) { *outW = surface->w; }
|
||||
if (outH) { *outH = surface->h; }
|
||||
|
||||
SDL_Texture* texture = SDL_CreateTextureFromSurface(renderer, surface);
|
||||
SDL_DestroySurface(surface);
|
||||
|
||||
if (!texture) {
|
||||
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Failed to create texture from %s: %s", resolvedPath.c_str(), SDL_GetError());
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
if (resolvedPath != path) {
|
||||
SDL_LogInfo(SDL_LOG_CATEGORY_APPLICATION, "Loaded %s via %s", path.c_str(), resolvedPath.c_str());
|
||||
}
|
||||
|
||||
return texture;
|
||||
}
|
||||
```
|
||||
|
||||
**After:**
|
||||
```cpp
|
||||
#include "utils/SDLPointers.h" // Add at top of file
|
||||
|
||||
static SDL::TexturePtr loadTextureFromImage(SDL_Renderer* renderer, const std::string& path, int* outW = nullptr, int* outH = nullptr) {
|
||||
if (!renderer) {
|
||||
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Renderer is null");
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
const std::string resolvedPath = AssetPath::resolveImagePath(path);
|
||||
SDL::SurfacePtr surface(IMG_Load(resolvedPath.c_str()));
|
||||
if (!surface) {
|
||||
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Failed to load image %s (resolved: %s): %s",
|
||||
path.c_str(), resolvedPath.c_str(), SDL_GetError());
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
if (outW) { *outW = surface->w; }
|
||||
if (outH) { *outH = surface->h; }
|
||||
|
||||
SDL::TexturePtr texture(SDL_CreateTextureFromSurface(renderer, surface.get()));
|
||||
// surface is automatically destroyed here
|
||||
|
||||
if (!texture) {
|
||||
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Failed to create texture from %s: %s",
|
||||
resolvedPath.c_str(), SDL_GetError());
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
if (resolvedPath != path) {
|
||||
SDL_LogInfo(SDL_LOG_CATEGORY_APPLICATION, "Loaded %s via %s", path.c_str(), resolvedPath.c_str());
|
||||
}
|
||||
|
||||
return texture;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🧹 Improvement #2: Remove Debug File I/O
|
||||
|
||||
### Step 1: Replace with SDL Logging
|
||||
|
||||
**File:** `src/states/MenuState.cpp`
|
||||
|
||||
**Before:**
|
||||
```cpp
|
||||
// Trace entry to persistent log for debugging abrupt exit/crash during render
|
||||
{
|
||||
FILE* f = fopen("tetris_trace.log", "a");
|
||||
if (f) {
|
||||
fprintf(f, "MenuState::render entry\n");
|
||||
fclose(f);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**After:**
|
||||
```cpp
|
||||
// Use SDL's built-in logging (only in debug builds)
|
||||
#ifdef _DEBUG
|
||||
SDL_LogTrace(SDL_LOG_CATEGORY_APPLICATION, "MenuState::render entry");
|
||||
#endif
|
||||
```
|
||||
|
||||
**Or, if you want it always enabled but less verbose:**
|
||||
```cpp
|
||||
SDL_LogVerbose(SDL_LOG_CATEGORY_APPLICATION, "MenuState::render entry");
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 2: Create a Logging Utility (Optional, Better Approach)
|
||||
|
||||
**File:** `src/utils/Logger.h`
|
||||
|
||||
```cpp
|
||||
#pragma once
|
||||
#include <SDL3/SDL.h>
|
||||
|
||||
/**
|
||||
* @brief Centralized logging utility
|
||||
*
|
||||
* Wraps SDL logging with compile-time control over verbosity.
|
||||
*/
|
||||
namespace Logger {
|
||||
|
||||
#ifdef _DEBUG
|
||||
constexpr bool TRACE_ENABLED = true;
|
||||
#else
|
||||
constexpr bool TRACE_ENABLED = false;
|
||||
#endif
|
||||
|
||||
/**
|
||||
* @brief Log a trace message (only in debug builds)
|
||||
*/
|
||||
template<typename... Args>
|
||||
inline void trace(const char* fmt, Args... args) {
|
||||
if constexpr (TRACE_ENABLED) {
|
||||
SDL_LogTrace(SDL_LOG_CATEGORY_APPLICATION, fmt, args...);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Log a debug message
|
||||
*/
|
||||
template<typename... Args>
|
||||
inline void debug(const char* fmt, Args... args) {
|
||||
SDL_LogDebug(SDL_LOG_CATEGORY_APPLICATION, fmt, args...);
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Log an info message
|
||||
*/
|
||||
template<typename... Args>
|
||||
inline void info(const char* fmt, Args... args) {
|
||||
SDL_LogInfo(SDL_LOG_CATEGORY_APPLICATION, fmt, args...);
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Log a warning message
|
||||
*/
|
||||
template<typename... Args>
|
||||
inline void warn(const char* fmt, Args... args) {
|
||||
SDL_LogWarn(SDL_LOG_CATEGORY_APPLICATION, fmt, args...);
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Log an error message
|
||||
*/
|
||||
template<typename... Args>
|
||||
inline void error(const char* fmt, Args... args) {
|
||||
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, fmt, args...);
|
||||
}
|
||||
|
||||
} // namespace Logger
|
||||
```
|
||||
|
||||
**Usage in MenuState.cpp:**
|
||||
```cpp
|
||||
#include "../utils/Logger.h"
|
||||
|
||||
void MenuState::render(SDL_Renderer* renderer, float logicalScale, SDL_Rect logicalVP) {
|
||||
Logger::trace("MenuState::render entry");
|
||||
|
||||
// ... rest of render code
|
||||
|
||||
Logger::trace("MenuState::render exit");
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 3: Update All Files
|
||||
|
||||
**Files to update:**
|
||||
- `src/states/MenuState.cpp` (multiple locations)
|
||||
- `src/main.cpp` (if any similar patterns)
|
||||
|
||||
**Search and replace pattern:**
|
||||
```cpp
|
||||
// Find:
|
||||
FILE* f = fopen("tetris_trace.log", "a");
|
||||
if (f) {
|
||||
fprintf(f, ".*");
|
||||
fclose(f);
|
||||
}
|
||||
|
||||
// Replace with:
|
||||
Logger::trace("...");
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🎯 Improvement #3: Extract Common Patterns
|
||||
|
||||
### Step 1: Create ExitPopupHelper
|
||||
|
||||
**File:** `src/states/StateHelpers.h` (new file)
|
||||
|
||||
```cpp
|
||||
#pragma once
|
||||
|
||||
/**
|
||||
* @file StateHelpers.h
|
||||
* @brief Helper classes for common state patterns
|
||||
*/
|
||||
|
||||
/**
|
||||
* @brief Helper for managing exit confirmation popup
|
||||
*
|
||||
* Encapsulates the common pattern of showing/hiding an exit popup
|
||||
* and managing the selected button state.
|
||||
*
|
||||
* Example usage:
|
||||
* @code
|
||||
* ExitPopupHelper exitPopup(ctx.exitPopupSelectedButton, ctx.showExitConfirmPopup);
|
||||
*
|
||||
* if (exitPopup.isVisible()) {
|
||||
* exitPopup.setSelection(0); // Select YES
|
||||
* }
|
||||
*
|
||||
* if (exitPopup.isYesSelected()) {
|
||||
* // Handle exit
|
||||
* }
|
||||
* @endcode
|
||||
*/
|
||||
class ExitPopupHelper {
|
||||
public:
|
||||
/**
|
||||
* @brief Construct helper with pointers to state variables
|
||||
* @param selectedButton Pointer to selected button index (0=YES, 1=NO)
|
||||
* @param showPopup Pointer to popup visibility flag
|
||||
*/
|
||||
ExitPopupHelper(int* selectedButton, bool* showPopup)
|
||||
: m_selectedButton(selectedButton)
|
||||
, m_showPopup(showPopup)
|
||||
{}
|
||||
|
||||
/**
|
||||
* @brief Set the selected button
|
||||
* @param value 0 for YES, 1 for NO
|
||||
*/
|
||||
void setSelection(int value) {
|
||||
if (m_selectedButton) {
|
||||
*m_selectedButton = value;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Get the currently selected button
|
||||
* @return 0 for YES, 1 for NO, defaults to 1 (NO) if pointer is null
|
||||
*/
|
||||
int getSelection() const {
|
||||
return m_selectedButton ? *m_selectedButton : 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Select YES button
|
||||
*/
|
||||
void selectYes() {
|
||||
setSelection(0);
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Select NO button
|
||||
*/
|
||||
void selectNo() {
|
||||
setSelection(1);
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Check if YES is selected
|
||||
*/
|
||||
bool isYesSelected() const {
|
||||
return getSelection() == 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Check if NO is selected
|
||||
*/
|
||||
bool isNoSelected() const {
|
||||
return getSelection() == 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Show the popup
|
||||
*/
|
||||
void show() {
|
||||
if (m_showPopup) {
|
||||
*m_showPopup = true;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Hide the popup
|
||||
*/
|
||||
void hide() {
|
||||
if (m_showPopup) {
|
||||
*m_showPopup = false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Check if popup is visible
|
||||
*/
|
||||
bool isVisible() const {
|
||||
return m_showPopup && *m_showPopup;
|
||||
}
|
||||
|
||||
/**
|
||||
* @brief Toggle between YES and NO
|
||||
*/
|
||||
void toggleSelection() {
|
||||
setSelection(isYesSelected() ? 1 : 0);
|
||||
}
|
||||
|
||||
private:
|
||||
int* m_selectedButton;
|
||||
bool* m_showPopup;
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 2: Update MenuState.cpp
|
||||
|
||||
**File:** `src/states/MenuState.cpp`
|
||||
|
||||
**Add include:**
|
||||
```cpp
|
||||
#include "StateHelpers.h"
|
||||
```
|
||||
|
||||
**Before:**
|
||||
```cpp
|
||||
void MenuState::handleEvent(const SDL_Event& e) {
|
||||
if (e.type == SDL_EVENT_KEY_DOWN && !e.key.repeat) {
|
||||
auto setExitSelection = [&](int value) {
|
||||
if (ctx.exitPopupSelectedButton) {
|
||||
*ctx.exitPopupSelectedButton = value;
|
||||
}
|
||||
};
|
||||
auto getExitSelection = [&]() -> int {
|
||||
return ctx.exitPopupSelectedButton ? *ctx.exitPopupSelectedButton : 1;
|
||||
};
|
||||
auto isExitPromptVisible = [&]() -> bool {
|
||||
return ctx.showExitConfirmPopup && *ctx.showExitConfirmPopup;
|
||||
};
|
||||
auto setExitPrompt = [&](bool visible) {
|
||||
if (ctx.showExitConfirmPopup) {
|
||||
*ctx.showExitConfirmPopup = visible;
|
||||
}
|
||||
};
|
||||
|
||||
if (isExitPromptVisible()) {
|
||||
switch (e.key.scancode) {
|
||||
case SDL_SCANCODE_LEFT:
|
||||
case SDL_SCANCODE_UP:
|
||||
setExitSelection(0);
|
||||
return;
|
||||
case SDL_SCANCODE_RIGHT:
|
||||
case SDL_SCANCODE_DOWN:
|
||||
setExitSelection(1);
|
||||
return;
|
||||
case SDL_SCANCODE_RETURN:
|
||||
case SDL_SCANCODE_KP_ENTER:
|
||||
case SDL_SCANCODE_SPACE:
|
||||
if (getExitSelection() == 0) {
|
||||
setExitPrompt(false);
|
||||
if (ctx.requestQuit) {
|
||||
ctx.requestQuit();
|
||||
}
|
||||
} else {
|
||||
setExitPrompt(false);
|
||||
}
|
||||
return;
|
||||
case SDL_SCANCODE_ESCAPE:
|
||||
setExitPrompt(false);
|
||||
setExitSelection(1);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// ... rest of code
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**After:**
|
||||
```cpp
|
||||
void MenuState::handleEvent(const SDL_Event& e) {
|
||||
if (e.type == SDL_EVENT_KEY_DOWN && !e.key.repeat) {
|
||||
ExitPopupHelper exitPopup(ctx.exitPopupSelectedButton, ctx.showExitConfirmPopup);
|
||||
|
||||
auto triggerPlay = [&]() {
|
||||
if (ctx.startPlayTransition) {
|
||||
ctx.startPlayTransition();
|
||||
} else if (ctx.stateManager) {
|
||||
ctx.stateManager->setState(AppState::Playing);
|
||||
}
|
||||
};
|
||||
|
||||
if (exitPopup.isVisible()) {
|
||||
switch (e.key.scancode) {
|
||||
case SDL_SCANCODE_LEFT:
|
||||
case SDL_SCANCODE_UP:
|
||||
exitPopup.selectYes();
|
||||
return;
|
||||
case SDL_SCANCODE_RIGHT:
|
||||
case SDL_SCANCODE_DOWN:
|
||||
exitPopup.selectNo();
|
||||
return;
|
||||
case SDL_SCANCODE_RETURN:
|
||||
case SDL_SCANCODE_KP_ENTER:
|
||||
case SDL_SCANCODE_SPACE:
|
||||
if (exitPopup.isYesSelected()) {
|
||||
exitPopup.hide();
|
||||
if (ctx.requestQuit) {
|
||||
ctx.requestQuit();
|
||||
} else {
|
||||
SDL_Event quit{};
|
||||
quit.type = SDL_EVENT_QUIT;
|
||||
SDL_PushEvent(&quit);
|
||||
}
|
||||
} else {
|
||||
exitPopup.hide();
|
||||
}
|
||||
return;
|
||||
case SDL_SCANCODE_ESCAPE:
|
||||
exitPopup.hide();
|
||||
exitPopup.selectNo();
|
||||
return;
|
||||
default:
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
switch (e.key.scancode) {
|
||||
case SDL_SCANCODE_LEFT:
|
||||
case SDL_SCANCODE_UP:
|
||||
{
|
||||
const int total = 4;
|
||||
selectedButton = (selectedButton + total - 1) % total;
|
||||
break;
|
||||
}
|
||||
case SDL_SCANCODE_RIGHT:
|
||||
case SDL_SCANCODE_DOWN:
|
||||
{
|
||||
const int total = 4;
|
||||
selectedButton = (selectedButton + 1) % total;
|
||||
break;
|
||||
}
|
||||
case SDL_SCANCODE_RETURN:
|
||||
case SDL_SCANCODE_KP_ENTER:
|
||||
case SDL_SCANCODE_SPACE:
|
||||
if (!ctx.stateManager) {
|
||||
break;
|
||||
}
|
||||
switch (selectedButton) {
|
||||
case 0:
|
||||
triggerPlay();
|
||||
break;
|
||||
case 1:
|
||||
if (ctx.requestFadeTransition) {
|
||||
ctx.requestFadeTransition(AppState::LevelSelector);
|
||||
} else if (ctx.stateManager) {
|
||||
ctx.stateManager->setState(AppState::LevelSelector);
|
||||
}
|
||||
break;
|
||||
case 2:
|
||||
if (ctx.requestFadeTransition) {
|
||||
ctx.requestFadeTransition(AppState::Options);
|
||||
} else if (ctx.stateManager) {
|
||||
ctx.stateManager->setState(AppState::Options);
|
||||
}
|
||||
break;
|
||||
case 3:
|
||||
exitPopup.show();
|
||||
exitPopup.selectNo();
|
||||
break;
|
||||
}
|
||||
break;
|
||||
case SDL_SCANCODE_ESCAPE:
|
||||
exitPopup.show();
|
||||
exitPopup.selectNo();
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Step 3: Apply to Other States
|
||||
|
||||
Apply the same pattern to:
|
||||
- `src/states/PlayingState.cpp`
|
||||
- `src/states/OptionsState.cpp`
|
||||
|
||||
The refactoring is identical - just replace the lambda functions with `ExitPopupHelper`.
|
||||
|
||||
---
|
||||
|
||||
## ✅ Testing Your Changes
|
||||
|
||||
After implementing these improvements:
|
||||
|
||||
1. **Build the project:**
|
||||
```powershell
|
||||
cd d:\Sites\Work\tetris
|
||||
cmake -B build -S . -DCMAKE_BUILD_TYPE=Debug
|
||||
cmake --build build
|
||||
```
|
||||
|
||||
2. **Run the game:**
|
||||
```powershell
|
||||
.\build\Debug\tetris.exe
|
||||
```
|
||||
|
||||
3. **Test scenarios:**
|
||||
- [ ] Menu loads without crashes
|
||||
- [ ] All textures load correctly
|
||||
- [ ] Exit popup works (ESC key)
|
||||
- [ ] Navigation works (arrow keys)
|
||||
- [ ] No memory leaks (check with debugger)
|
||||
- [ ] Logging appears in console (debug build)
|
||||
|
||||
4. **Check for memory leaks:**
|
||||
- Run with Visual Studio debugger
|
||||
- Check Output window for memory leak reports
|
||||
- Should see no leaks from SDL textures
|
||||
|
||||
---
|
||||
|
||||
## 📊 Expected Impact
|
||||
|
||||
After implementing these three improvements:
|
||||
|
||||
| Metric | Before | After | Improvement |
|
||||
|--------|--------|-------|-------------|
|
||||
| **Memory Safety** | ⭐⭐⭐ | ⭐⭐⭐⭐⭐ | +67% |
|
||||
| **Code Clarity** | ⭐⭐⭐⭐ | ⭐⭐⭐⭐⭐ | +25% |
|
||||
| **Maintainability** | ⭐⭐⭐⭐ | ⭐⭐⭐⭐⭐ | +25% |
|
||||
| **Lines of Code** | 100% | ~95% | -5% |
|
||||
| **Potential Bugs** | Medium | Low | -50% |
|
||||
|
||||
---
|
||||
|
||||
## 🎉 Next Steps
|
||||
|
||||
After successfully implementing these improvements:
|
||||
|
||||
1. Review the full `CODE_ANALYSIS.md` for more recommendations
|
||||
2. Check `IMPROVEMENTS_CHECKLIST.md` for the complete task list
|
||||
3. Consider implementing the medium-priority items next
|
||||
4. Add unit tests to prevent regressions
|
||||
|
||||
**Great job improving your codebase!** 🚀
|
||||
34
README.md
Normal file
@ -0,0 +1,34 @@
|
||||
Spacetris SDL3
|
||||
=============
|
||||
|
||||
A native C++20 SDL3-based Tetris-style game.
|
||||
|
||||
Quick Start (Windows)
|
||||
- Install vcpkg and packages: `vcpkg install sdl3 sdl3-ttf --triplet=x64-windows`
|
||||
- Configure: `cmake -S . -B build-msvc -DCMAKE_BUILD_TYPE=Debug`
|
||||
- Build: `cmake --build build-msvc --config Debug`
|
||||
- Run (helper): `.
|
||||
build-debug-and-run.ps1` or run `build-msvc\Debug\spacetris.exe`
|
||||
|
||||
Production Packaging
|
||||
- Quick package (uses existing build): `.
|
||||
package-quick.ps1`
|
||||
- Full production (clean Release build + package): `.
|
||||
build-production.ps1 -Clean`
|
||||
|
||||
Tests
|
||||
- Unit tests target: `spacetris_tests` → exe `spacetris_tests.exe`
|
||||
- Run tests: configure+build then `ctest -C Debug` or run the test exe directly
|
||||
|
||||
Where to look
|
||||
- Main app sources: `src/` (entry `src/main.cpp`, app `src/app`)
|
||||
- Build control: `CMakeLists.txt` and `cmake/ProductionBuild.cmake`
|
||||
- Packaging helpers: `build-production.ps1`, `package-quick.ps1`
|
||||
|
||||
Notes
|
||||
- The canonical executable name is `spacetris` (`spacetris.exe` on Windows).
|
||||
- Assets live in `assets/` and are copied into the distribution package.
|
||||
|
||||
If you want, I can:
|
||||
- Run a Debug build and confirm the test executable name,
|
||||
- Replace remaining legacy "tetris" tokens across generated files and docs.
|
||||
@ -1,4 +1,4 @@
|
||||
# Tetris — Upgrade Roadmap
|
||||
# Spacetris — Upgrade Roadmap
|
||||
|
||||
This document lists recommended code, architecture, tooling, and runtime upgrades for the native SDL3 Tetris project. Items are grouped, prioritized, and mapped to target files and effort estimates so you can plan incremental work.
|
||||
|
||||
@ -20,7 +20,7 @@ This document lists recommended code, architecture, tooling, and runtime upgrade
|
||||
- [x] Replace ad-hoc printf with SDL_Log or injected Logger service
|
||||
- Note: majority of printf/fprintf debug prints were replaced with `SDL_Log*` calls; a quick grep audit is recommended to find any remaining ad-hoc prints.
|
||||
- [x] Add unit tests (gravity conversion, level progression, line clear behavior)
|
||||
- Note: a small test runner (`tests/GravityTests.cpp`) and the `tetris_tests` CMake target were added and run locally; gravity tests pass (see build-msvc test run). Converting to broader Catch2 suites is optional.
|
||||
- Note: a small test runner (`tests/GravityTests.cpp`) and the `spacetris_tests` CMake target were added and run locally; gravity tests pass (see build-msvc test run). Converting to broader Catch2 suites is optional.
|
||||
- [ ] Add CI (build + tests) and code style checks
|
||||
- [ ] Improve input hit-testing for level popup and scalable UI
|
||||
- [ ] Add defensive guards (clamps, null checks) and const-correctness
|
||||
|
||||
|
Before Width: | Height: | Size: 15 KiB After Width: | Height: | Size: 1.0 MiB |
BIN
assets/fonts/Exo2.ttf
Normal file
BIN
assets/fonts/Orbitron.ttf
Normal file
BIN
assets/images/asteroids_001.png
Normal file
|
After Width: | Height: | Size: 196 KiB |
BIN
assets/images/blocks90px_002.png
Normal file
|
After Width: | Height: | Size: 87 KiB |
BIN
assets/images/blocks90px_003.png
Normal file
|
After Width: | Height: | Size: 112 KiB |
BIN
assets/images/cooperate_info.png
Normal file
|
After Width: | Height: | Size: 416 KiB |
BIN
assets/images/hold_panel.png
Normal file
|
After Width: | Height: | Size: 103 KiB |
BIN
assets/images/levels/level0.jpg
Normal file
|
After Width: | Height: | Size: 257 KiB |
BIN
assets/images/levels/level1.jpg
Normal file
|
After Width: | Height: | Size: 226 KiB |
BIN
assets/images/levels/level10.jpg
Normal file
|
After Width: | Height: | Size: 188 KiB |
BIN
assets/images/levels/level11.jpg
Normal file
|
After Width: | Height: | Size: 204 KiB |
BIN
assets/images/levels/level12.jpg
Normal file
|
After Width: | Height: | Size: 157 KiB |
BIN
assets/images/levels/level13.jpg
Normal file
|
After Width: | Height: | Size: 333 KiB |
BIN
assets/images/levels/level14.jpg
Normal file
|
After Width: | Height: | Size: 265 KiB |
BIN
assets/images/levels/level15.jpg
Normal file
|
After Width: | Height: | Size: 298 KiB |
BIN
assets/images/levels/level16.jpg
Normal file
|
After Width: | Height: | Size: 144 KiB |
BIN
assets/images/levels/level17.jpg
Normal file
|
After Width: | Height: | Size: 276 KiB |
BIN
assets/images/levels/level18.jpg
Normal file
|
After Width: | Height: | Size: 223 KiB |
BIN
assets/images/levels/level19.jpg
Normal file
|
After Width: | Height: | Size: 281 KiB |
BIN
assets/images/levels/level2.jpg
Normal file
|
After Width: | Height: | Size: 450 KiB |
BIN
assets/images/levels/level20.jpg
Normal file
|
After Width: | Height: | Size: 315 KiB |
BIN
assets/images/levels/level21.jpg
Normal file
|
After Width: | Height: | Size: 322 KiB |
BIN
assets/images/levels/level22.jpg
Normal file
|
After Width: | Height: | Size: 323 KiB |
BIN
assets/images/levels/level23.jpg
Normal file
|
After Width: | Height: | Size: 242 KiB |
BIN
assets/images/levels/level24.jpg
Normal file
|
After Width: | Height: | Size: 349 KiB |
BIN
assets/images/levels/level25.jpg
Normal file
|
After Width: | Height: | Size: 387 KiB |
BIN
assets/images/levels/level26.jpg
Normal file
|
After Width: | Height: | Size: 226 KiB |
BIN
assets/images/levels/level27.jpg
Normal file
|
After Width: | Height: | Size: 203 KiB |
BIN
assets/images/levels/level28.jpg
Normal file
|
After Width: | Height: | Size: 158 KiB |
BIN
assets/images/levels/level29.jpg
Normal file
|
After Width: | Height: | Size: 297 KiB |
BIN
assets/images/levels/level3.jpg
Normal file
|
After Width: | Height: | Size: 328 KiB |
BIN
assets/images/levels/level30.jpg
Normal file
|
After Width: | Height: | Size: 245 KiB |
BIN
assets/images/levels/level31.jpg
Normal file
|
After Width: | Height: | Size: 108 KiB |
BIN
assets/images/levels/level32.jpg
Normal file
|
After Width: | Height: | Size: 295 KiB |
BIN
assets/images/levels/level4.jpg
Normal file
|
After Width: | Height: | Size: 145 KiB |
BIN
assets/images/levels/level5.jpg
Normal file
|
After Width: | Height: | Size: 199 KiB |
BIN
assets/images/levels/level6.jpg
Normal file
|
After Width: | Height: | Size: 142 KiB |
BIN
assets/images/levels/level7.jpg
Normal file
|
After Width: | Height: | Size: 117 KiB |
BIN
assets/images/levels/level8.jpg
Normal file
|
After Width: | Height: | Size: 335 KiB |
BIN
assets/images/levels/level9.jpg
Normal file
|
After Width: | Height: | Size: 420 KiB |
|
Before Width: | Height: | Size: 930 KiB |
|
Before Width: | Height: | Size: 195 KiB |
|
Before Width: | Height: | Size: 110 KiB |
|
Before Width: | Height: | Size: 24 KiB |
|
Before Width: | Height: | Size: 5.9 MiB |
BIN
assets/images/main_screen.png
Normal file
|
After Width: | Height: | Size: 1.2 MiB |
|
Before Width: | Height: | Size: 1.4 MiB |
|
Before Width: | Height: | Size: 1.4 MiB |
|
Before Width: | Height: | Size: 2.1 MiB |
|
Before Width: | Height: | Size: 2.2 MiB |
BIN
assets/images/main_screen_old.png
Normal file
|
After Width: | Height: | Size: 2.0 MiB |
BIN
assets/images/next_panel.png
Normal file
|
After Width: | Height: | Size: 127 KiB |
|
Before Width: | Height: | Size: 1.9 MiB After Width: | Height: | Size: 1.9 MiB |
|
Before Width: | Height: | Size: 35 KiB After Width: | Height: | Size: 35 KiB |
|
Before Width: | Height: | Size: 21 KiB After Width: | Height: | Size: 21 KiB |
|
Before Width: | Height: | Size: 13 KiB After Width: | Height: | Size: 13 KiB |
|
Before Width: | Height: | Size: 21 KiB After Width: | Height: | Size: 21 KiB |
|
Before Width: | Height: | Size: 3.6 KiB After Width: | Height: | Size: 3.6 KiB |
|
Before Width: | Height: | Size: 253 KiB After Width: | Height: | Size: 253 KiB |
|
Before Width: | Height: | Size: 66 KiB After Width: | Height: | Size: 66 KiB |
|
Before Width: | Height: | Size: 55 KiB After Width: | Height: | Size: 55 KiB |
|
Before Width: | Height: | Size: 76 KiB After Width: | Height: | Size: 76 KiB |
BIN
assets/images/panel_score.png
Normal file
|
After Width: | Height: | Size: 275 KiB |
BIN
assets/images/statistics_panel.png
Normal file
|
After Width: | Height: | Size: 440 KiB |
|
Before Width: | Height: | Size: 1.6 MiB |
|
Before Width: | Height: | Size: 1.1 MiB |
|
Before Width: | Height: | Size: 756 KiB |
|
Before Width: | Height: | Size: 1007 KiB |
|
Before Width: | Height: | Size: 1.0 MiB |
|
Before Width: | Height: | Size: 957 KiB |
|
Before Width: | Height: | Size: 805 KiB |
|
Before Width: | Height: | Size: 1.1 MiB |
|
Before Width: | Height: | Size: 947 KiB |
|
Before Width: | Height: | Size: 1.6 MiB |
|
Before Width: | Height: | Size: 1.3 MiB |
|
Before Width: | Height: | Size: 1.4 MiB |
|
Before Width: | Height: | Size: 825 KiB |
|
Before Width: | Height: | Size: 1.7 MiB |
|
Before Width: | Height: | Size: 1.2 MiB |
|
Before Width: | Height: | Size: 1.3 MiB |
|
Before Width: | Height: | Size: 970 KiB |
|
Before Width: | Height: | Size: 1.1 MiB |
|
Before Width: | Height: | Size: 1.6 MiB |
|
Before Width: | Height: | Size: 2.3 MiB |
|
Before Width: | Height: | Size: 1007 KiB |
|
Before Width: | Height: | Size: 638 KiB |
|
Before Width: | Height: | Size: 1.2 MiB |
|
Before Width: | Height: | Size: 1.1 MiB |