-
Notifications
You must be signed in to change notification settings - Fork 0
Separate platform api, major refactor #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
0ff0979
ad9132e
9e8bc76
0a8eae6
ef73eff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Generated by CLion for LLVM | ||
| BasedOnStyle: LLVM | ||
| IndentWidth: 4 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| /cmake-build-debug/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 15/08/2025. | ||
| // | ||
| #pragma once | ||
|
|
||
| #include "physics/Vec2.h" | ||
| #include <filesystem> | ||
| #include <string_view> | ||
|
|
||
| namespace config { | ||
|
|
||
| inline constexpr std::string_view GAME_NAME {"Sandbox 2D"}; | ||
|
|
||
| namespace resources { | ||
| // resources (should move in namespace or a more relevant file later) | ||
| inline const std::filesystem::path ASSETS_PATH = "assets"; | ||
| inline const std::filesystem::path TEX_PATH = ASSETS_PATH / "textures"; | ||
| inline const std::filesystem::path FONT_PATH = ASSETS_PATH / "fonts"; | ||
| inline constexpr std::string_view FONT {"Hack-Regular.ttf"}; | ||
| inline constexpr std::string_view PLAYER_TEX {"bacon.png"}; | ||
| inline constexpr std::string_view WALL_TEX {"wall.png"}; | ||
| } | ||
|
|
||
| namespace graphics { | ||
| // graphics (should move in namespace or a more relevant file later) | ||
| inline constexpr phys::Vec2i WINDOW_SIZE {1280, 720}; | ||
| inline constexpr int TILE_SIZE = 16; | ||
| inline constexpr float DRAW_SCALE = 4.0; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 15/08/2025. | ||
| // | ||
| #pragma once | ||
|
|
||
| #include "game/GameState.h" | ||
| #include "graphics/Renderer.h" | ||
| #include "input/InputManager.h" | ||
|
|
||
| class Game { | ||
| public: | ||
| bool init(); | ||
| void loop(); | ||
|
|
||
| private: | ||
| GameState gameState_ {}; | ||
| Renderer renderer_ {}; | ||
| InputManager inputManager_ {}; | ||
|
|
||
| void step(double deltaSeconds); | ||
|
|
||
| public: | ||
| Game() = default; | ||
| ~Game() = default; | ||
| // Delete copy operations | ||
| Game(const Game&) = delete; | ||
| Game& operator=(const Game&) = delete; | ||
| // Delete move operations | ||
| Game(Game&&) = delete; | ||
| Game& operator=(Game&&) = delete; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 20/08/2025. | ||
| // | ||
| #pragma once | ||
| #include "Player.h" | ||
| #include "world/World.h" | ||
|
|
||
| struct GameState { | ||
| uint64_t gameTicks = 0; | ||
| bool running = true; | ||
| bool paused = true; | ||
| bool debugMode = true; | ||
| Player player {}; | ||
| World world {}; | ||
|
|
||
| bool unpaused() const { return !paused; } | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 20/08/2025. | ||
| // | ||
| #pragma once | ||
| #include "physics/Vec2.h" | ||
| #include "world/World.h" | ||
|
|
||
| struct Player { | ||
| constexpr static float SPEED = 10.0f; | ||
| phys::Vec2f pos {}; | ||
| phys::Vec2f vel {}; | ||
| Sprite sprite {std::string(config::resources::PLAYER_TEX)}; | ||
|
|
||
| void set_velocity(phys::Vec2f v); | ||
| void move(double deltaSeconds); | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 21/08/2025. | ||
| // | ||
| #pragma once | ||
| #include "game/GameState.h" | ||
| #include "platform/sdl/GraphicsDevice.h" | ||
|
|
||
| class Renderer { | ||
| public: | ||
| bool init(int winW, int winH, std::string_view windowTitle); | ||
| void render(const GameState& gameState, double deltaSeconds); | ||
|
|
||
| private: | ||
| void drawSprite(const Sprite &sprite, phys::Vec2f pos) const; | ||
| void drawWorld(const World& world) const; | ||
| void cameraSnap(phys::Vec2f pos); | ||
| void cameraFollow(phys::Vec2f pos, double deltaSeconds); | ||
| phys::Vec2f toScreenCoords(const phys::Vec2f& pos) const; | ||
| void drawDebugInfo(const GameState &gameState, double deltaSeconds) const; | ||
|
|
||
| GraphicsDevice graphicsDevice_ {}; | ||
| struct Camera { phys::Vec2f pos {0, 0}; } camera_; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 21/08/2025. | ||
| // | ||
|
|
||
| #pragma once | ||
| #include "platform/sdl/InputSource.h" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was initially using abstract interfaces but it seemed overkill, the current refactor already positions the code with much better separation that would make further steps easier. Since the code is still developing, going the full way might be premature. Do you have suggestions to ease into that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jacopouggeri That's a very pragmatic approach! You're right that premature abstraction can be more hindrance than help during active development. Let me suggest some incremental steps that ease toward better separation without requiring a major overhaul: Phase 1: Minimize the include footprint (immediate, low-risk) // In InputManager.h - just move the include to the .cpp file
#pragma once
struct GameState; // forward declaration
class InputSource; // forward declaration
class InputManager {
public:
bool init() const;
void handleEvents(GameState& state) const;
private:
InputSource* inputSource_; // pointer instead of value
};This keeps the same concrete type but removes the header dependency. The implementation stays in the .cpp file where platform headers are fine. Phase 2: Simple factory approach (when you add another platform) // platform/InputSourceFactory.h
std::unique_ptr<InputSource> createInputSource();Phase 3: Interface when you actually need it The key insight is that your current directory structure already provides the right boundaries - you just need to be more careful about include dependencies crossing those boundaries. Would this incremental approach work better for your current development pace? |
||
|
|
||
| class InputManager { | ||
| public: | ||
| bool init(); | ||
| void handleEvents(GameState& state); | ||
|
|
||
| private: | ||
| InputSource inputSource_; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 21/08/2025. | ||
| // | ||
|
|
||
| #pragma once | ||
| #include <string> | ||
|
|
||
| struct Sprite { | ||
| std::string textureName; | ||
| int width {16}; | ||
| int height {16}; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 19/08/2025. | ||
| // | ||
| #pragma once | ||
|
|
||
| #include "game/Config.h" | ||
| #include "game/resources/Sprite.h" | ||
| #include <array> | ||
|
|
||
| namespace world { | ||
| constexpr int CHUNK_SIZE = 16; | ||
| constexpr int WORLD_WIDTH_CHUNKS = 8; | ||
| constexpr int WORLD_HEIGHT_CHUNKS = 4; | ||
| constexpr phys::Vec2i WORLD_SIZE {WORLD_WIDTH_CHUNKS * CHUNK_SIZE, WORLD_HEIGHT_CHUNKS * CHUNK_SIZE}; | ||
| constexpr int SEA_LEVEL = 0; | ||
| constexpr int CHUNK_COUNT = WORLD_WIDTH_CHUNKS * WORLD_HEIGHT_CHUNKS; | ||
|
|
||
| } | ||
|
|
||
|
|
||
| struct Tile { | ||
| Sprite sprite; | ||
| }; | ||
|
|
||
| struct Chunk { | ||
| std::array<Tile, world::CHUNK_SIZE* world::CHUNK_SIZE> tiles; | ||
|
|
||
| Tile& getTile(int tileX, int tileY); | ||
| }; | ||
|
|
||
| struct World { | ||
| std::array<Chunk, world::CHUNK_COUNT> chunks; | ||
|
|
||
| void init(); | ||
|
|
||
| Tile& getTileGlobal(int worldX, int worldY); | ||
|
|
||
| Chunk& getChunk(int chunkX, int chunkY); | ||
|
|
||
| [[nodiscard]] static constexpr phys::Vec2i chunkCoords(int chunkIdx) noexcept { | ||
| return { chunkIdx % world::WORLD_WIDTH_CHUNKS, chunkIdx / world::WORLD_WIDTH_CHUNKS }; | ||
| } | ||
|
|
||
| [[nodiscard]] static constexpr phys::Vec2i tileCoords(int tileIdx) noexcept { | ||
| return { tileIdx % world::CHUNK_SIZE, tileIdx / world::CHUNK_SIZE }; | ||
| } | ||
|
|
||
| [[nodiscard]] static constexpr phys::Vec2i worldCoords(int chunkIdx, int tileIdx) noexcept { | ||
| const phys::Vec2i chunkPos = chunkCoords(chunkIdx); | ||
| const phys::Vec2i tilePos = tileCoords(tileIdx); | ||
| return chunkPos * world::CHUNK_SIZE + tilePos - world::WORLD_SIZE / 2; // Center the world | ||
| } | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 19/08/2025. | ||
| // | ||
| #pragma once | ||
|
|
||
| #include "platform/sdl/Resource.h" | ||
| #include <string> | ||
| #include <string_view> | ||
| #include <unordered_map> | ||
|
|
||
| class AssetLoader { | ||
| public: | ||
| void loadTextures(SDL_Renderer* renderer); | ||
| void loadFonts(); | ||
| [[nodiscard]] const Texture& getTexture(std::string_view textureName) const noexcept; | ||
| [[nodiscard]] const Font& getFont(std::string_view fontName) const noexcept; | ||
| void clear() noexcept { | ||
| textures.clear(); | ||
| fonts.clear(); | ||
| } | ||
|
|
||
| private: | ||
| void loadTexture(std::string_view textureName, SDL_Renderer* renderer); | ||
| void loadFont(std::string_view fontName, int size); | ||
|
|
||
| std::unordered_map<std::string, Texture> textures; | ||
| std::unordered_map<std::string, Font> fonts; | ||
|
|
||
| public: | ||
| AssetLoader() = default; | ||
| ~AssetLoader() = default; | ||
| // Prevent copying | ||
| AssetLoader(const AssetLoader&) = delete; | ||
| AssetLoader& operator=(const AssetLoader&) = delete; | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,32 @@ | ||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||
| // Created by Jacopo Uggeri on 20/08/2025. | ||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||
| #pragma once | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| #include "game/resources/Sprite.h" | ||||||||||||||||||||||||||||||||||||||||
| #include "platform/sdl/AssetLoader.h" | ||||||||||||||||||||||||||||||||||||||||
| #include "platform/sdl/SDLContext.h" | ||||||||||||||||||||||||||||||||||||||||
| #include <SDL_render.h> | ||||||||||||||||||||||||||||||||||||||||
| #include <SDL_video.h> | ||||||||||||||||||||||||||||||||||||||||
| #include <string_view> | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| class GraphicsDevice final { | ||||||||||||||||||||||||||||||||||||||||
| public: | ||||||||||||||||||||||||||||||||||||||||
| bool init(int winW, int winH, std::string_view windowTitle); | ||||||||||||||||||||||||||||||||||||||||
| void beginFrame() const; | ||||||||||||||||||||||||||||||||||||||||
| void endFrame() const; | ||||||||||||||||||||||||||||||||||||||||
| void drawTexture(const Sprite &sprite, float screenX, float screenY) const; | ||||||||||||||||||||||||||||||||||||||||
| void drawText(std::string_view text, float x, float y) const; | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug in implementation: srcRect uses scaled size (in .cpp) In GraphicsDevice.cpp, srcRect width/height are computed after applying DRAW_SCALE. srcRect should use the unscaled sprite pixel size; only destRect should be scaled. This causes incorrect sampling/cropping. Apply in src/platform/sdl/GraphicsDevice.cpp: - const float width = sprite.width * config::graphics::DRAW_SCALE;
- const float height = sprite.height * config::graphics::DRAW_SCALE;
- const SDL_Rect srcRect {0, 0, static_cast<int>(width), static_cast<int>(height)};
- screenX -= width / 2.0f;
- screenY -= height / 2.0f;
- const SDL_FRect destRect {screenX, screenY, width, height};
+ const int srcW = sprite.width;
+ const int srcH = sprite.height;
+ const float dstW = srcW * config::graphics::DRAW_SCALE;
+ const float dstH = srcH * config::graphics::DRAW_SCALE;
+ const SDL_Rect srcRect {0, 0, srcW, srcH};
+ screenX -= dstW / 2.0f;
+ screenY -= dstH / 2.0f;
+ const SDL_FRect destRect {screenX, screenY, dstW, dstH};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
| void drawOverlay(uint8_t r, uint8_t g, uint8_t b, uint8_t a) const; | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle missing font in drawText (in .cpp) drawText dereferences a potentially null font. Guard against it to avoid crashes if asset loading failed. Apply in src/platform/sdl/GraphicsDevice.cpp: - TTF_Font* font = assetLoader_.getFont(config::resources::FONT).get();
- if (const auto textSurface = TTF_RenderText_Blended(font, text.data(), color)) {
+ TTF_Font* font = assetLoader_.getFont(config::resources::FONT).get();
+ if (!font) return;
+ if (const auto textSurface = TTF_RenderText_Blended(font, text.data(), color)) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
| std::pair<int, int> getWindowSize() const; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| private: | ||||||||||||||||||||||||||||||||||||||||
| SDLContext sdlContext_ {}; | ||||||||||||||||||||||||||||||||||||||||
| SDL_Window* window_ = nullptr; | ||||||||||||||||||||||||||||||||||||||||
| SDL_Renderer* renderer_ = nullptr; | ||||||||||||||||||||||||||||||||||||||||
| AssetLoader assetLoader_ {}; | ||||||||||||||||||||||||||||||||||||||||
| bool vsyncEnabled_ = true; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| public: | ||||||||||||||||||||||||||||||||||||||||
| ~GraphicsDevice(); | ||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 20/08/2025. | ||
| // | ||
| #pragma once | ||
|
|
||
| struct GameState; | ||
|
|
||
| class InputSource final { | ||
| public: | ||
| bool init(); | ||
| void handleEvents(GameState& state); | ||
| void handlePlayerInput(GameState& state); | ||
|
|
||
| ~InputSource(); | ||
|
Comment on lines
+6
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Platform-to-game coupling: avoid exposing GameState in the platform API. Having platform headers reference Example interface sketch (outside this file for illustration): // include/platform/interfaces/IInputSource.h
struct InputEvent {
enum class Type { Quit, KeyDown, MouseDown, MouseMove };
Type type;
// key/button/mouse payload...
};
class IInputSource {
public:
virtual ~IInputSource() = default;
virtual bool init() noexcept = 0;
virtual bool pollEvent(InputEvent& out) noexcept = 0; // returns false when queue empty
};Then 🤖 Prompt for AI Agents |
||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| // | ||
| // Created by Jacopo Uggeri on 21/08/2025. | ||
| // | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <SDL_render.h> | ||
| #include <SDL_ttf.h> | ||
| #include <memory> | ||
|
|
||
| template<typename T, auto Deleter> | ||
| class Resource { | ||
| using ResourcePtr = std::unique_ptr<T, decltype(Deleter)>; | ||
| public: | ||
| explicit Resource(T* resource = nullptr) | ||
| : resource_(resource, Deleter) {} | ||
|
|
||
| explicit operator bool() const { return resource_ != nullptr; } | ||
|
|
||
| [[nodiscard]] T* get() const { return resource_.get(); } | ||
|
|
||
| T* operator->() const { return resource_.get(); } | ||
| T& operator*() const { return *resource_; } | ||
|
|
||
| private: | ||
| ResourcePtr resource_; | ||
| }; | ||
|
|
||
| using Texture = Resource<SDL_Texture, &SDL_DestroyTexture>; | ||
| using Font = Resource<TTF_Font, &TTF_CloseFont>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Windows entry-point linkage: add SDL2::SDL2main or define SDL_MAIN_HANDLED.
With
#include <SDL.h>inmain.cpp, themainsymbol is usually remapped toSDL_mainand requires linkingSDL2::SDL2mainon Windows. Without it, MSVC builds often fail at link time. Two options:SDL2::SDL2main(recommended when you include<SDL.h>in your main TU):target_link_libraries(${PROJECT_NAME} PRIVATE - SDL2::SDL2 + SDL2::SDL2 + SDL2::SDL2main SDL2_image::SDL2_image SDL2_ttf::SDL2_ttf )SDL_MAIN_HANDLEDand include<SDL.h>for APIs only (then you typically don’t linkSDL2main). If you go this route, add:and call
SDL_SetMainReady()beforeSDL_Init()(desktop often works without it, but it’s the documented sequence).🏁 Script executed:
Length of output: 383
Link Windows entry-point library or define SDL_MAIN_HANDLED
On Windows, SDL remaps
main()toSDL_main, which requires either linking againstSDL2::SDL2mainor definingSDL_MAIN_HANDLEDand callingSDL_SetMainReady()beforeSDL_Init(). Our verification shows you’re currently linking onlySDL2::SDL2,SDL2_image, andSDL2_ttf, and there’s no evidence ofSDL2::SDL2mainor theSDL_MAIN_HANDLEDmacro in the codebase.Please address one of the following in your
CMakeLists.txtand source:• Add the
SDL2mainlibrary so that the default remapping works:target_link_libraries(${PROJECT_NAME} PRIVATE - SDL2::SDL2 + SDL2::SDL2main + SDL2::SDL2 SDL2_image::SDL2_image SDL2_ttf::SDL2_ttf )• Or keep your custom
main()signature by definingSDL_MAIN_HANDLEDand initializing correctly:CMakeLists.txt:main.cpp, before any SDL calls:Adjusting one of these options will resolve Windows link errors related to the SDL entry-point.
🤖 Prompt for AI Agents