From 4277f4c818194b7ddaeeacc728f3181d1b5e3633 Mon Sep 17 00:00:00 2001 From: binaryDiv Date: Fri, 21 Nov 2025 01:09:05 +0100 Subject: [PATCH] Use unique_ptr to manage SDL pointers --- src/app.cppm | 10 ------ src/core/engine.cppm | 8 ++--- src/core/renderer.cppm | 45 +++++++-------------------- src/core/window.cppm | 34 ++++---------------- src/game/game.cppm | 12 ++++--- src/game/sprite.cppm | 64 +++++++++----------------------------- src/resources/texture.cppm | 63 +++++++++++++++++++++++++++++++++++++ src/utils/memory.cppm | 21 +++++++++++++ 8 files changed, 128 insertions(+), 129 deletions(-) create mode 100644 src/resources/texture.cppm create mode 100644 src/utils/memory.cppm diff --git a/src/app.cppm b/src/app.cppm index 86c3f51..37c5483 100644 --- a/src/app.cppm +++ b/src/app.cppm @@ -22,16 +22,6 @@ export public: App() = default; - // No copy operations - App(const App&) = delete; - App& operator=(const App&) = delete; - - // Default move operations - App(App&&) = default; - App& operator=(App&&) = default; - - ~App() = default; - SDL_AppResult initialize() { try { diff --git a/src/core/engine.cppm b/src/core/engine.cppm index dda117d..2af5b7b 100644 --- a/src/core/engine.cppm +++ b/src/core/engine.cppm @@ -31,13 +31,11 @@ export namespace core instantiated_ = true; } - // No copy operations + // No copy or move operations Engine(const Engine&) = delete; Engine& operator=(const Engine&) = delete; - - // Default move operations - Engine(Engine&&) = default; - Engine& operator=(Engine&&) = default; + Engine(Engine&&) = delete; + Engine& operator=(Engine&&) = delete; ~Engine() { diff --git a/src/core/renderer.cppm b/src/core/renderer.cppm index 2a8ef7b..337c6f5 100644 --- a/src/core/renderer.cppm +++ b/src/core/renderer.cppm @@ -1,15 +1,20 @@ module; +#include #include export module core.renderer; +import utils.memory; + export namespace core { class Renderer { - // TODO: Use unique_ptr with custom deleters - SDL_Renderer* sdl_renderer_ = nullptr; + std::unique_ptr< + SDL_Renderer, + utils::FuncDeleter + > sdl_renderer_ = nullptr; public: Renderer() = default; @@ -18,47 +23,21 @@ export namespace core : sdl_renderer_(sdl_renderer) {} - // No copy operations - Renderer(const Renderer&) = delete; - Renderer& operator=(const Renderer&) = delete; - - // Move constructor - Renderer(Renderer&& other) noexcept - : sdl_renderer_(other.sdl_renderer_) - { - other.sdl_renderer_ = nullptr; - } - - // Move assignment - Renderer& operator=(Renderer&& other) noexcept - { - sdl_renderer_ = other.sdl_renderer_; - other.sdl_renderer_ = nullptr; - return *this; - } - - ~Renderer() - { - if (sdl_renderer_ != nullptr) { - SDL_DestroyRenderer(sdl_renderer_); - } - } - // TODO: Remove this when not needed anymore - SDL_Renderer* get_sdl_renderer() const + constexpr SDL_Renderer* get_sdl_renderer() const { - return sdl_renderer_; + return sdl_renderer_.get(); } // TODO: Rename clear/present to start_render/finish_render or similar? void clear() const { - SDL_RenderClear(sdl_renderer_); + SDL_RenderClear(sdl_renderer_.get()); } void present() const { - SDL_RenderPresent(sdl_renderer_); + SDL_RenderPresent(sdl_renderer_.get()); } // TODO: Replace SDL_Texture pointer with Texture class @@ -66,7 +45,7 @@ export namespace core // to just type-alias it?) void render_texture(SDL_Texture* texture, const SDL_FRect* src_rect, const SDL_FRect* dest_rect) const { - SDL_RenderTexture(sdl_renderer_, texture, src_rect, dest_rect); + SDL_RenderTexture(sdl_renderer_.get(), texture, src_rect, dest_rect); } }; } diff --git a/src/core/window.cppm b/src/core/window.cppm index 9bccb78..85bcc95 100644 --- a/src/core/window.cppm +++ b/src/core/window.cppm @@ -1,5 +1,6 @@ module; +#include #include #include @@ -7,13 +8,16 @@ export module core.window; import core.exceptions; import core.renderer; +import utils.memory; export namespace core { class Window { - // TODO: Use unique_ptr with custom deleters - SDL_Window* sdl_window_ = nullptr; + std::unique_ptr< + SDL_Window, + utils::FuncDeleter + > sdl_window_ = nullptr; public: Window() = default; @@ -22,32 +26,6 @@ export namespace core : sdl_window_(sdl_window) {} - // No copy operations - Window(const Window&) = delete; - Window& operator=(const Window&) = delete; - - // Move constructor - Window(Window&& other) noexcept - : sdl_window_(other.sdl_window_) - { - other.sdl_window_ = nullptr; - } - - // Move assignment - Window& operator=(Window&& other) noexcept - { - sdl_window_ = other.sdl_window_; - other.sdl_window_ = nullptr; - return *this; - } - - ~Window() - { - if (sdl_window_ != nullptr) { - SDL_DestroyWindow(sdl_window_); - } - } - static std::pair create_window_and_renderer( const std::string& title, const int width, diff --git a/src/game/game.cppm b/src/game/game.cppm index 4b7a6a1..8896bf2 100644 --- a/src/game/game.cppm +++ b/src/game/game.cppm @@ -8,6 +8,7 @@ export module game.game; import core.engine; import core.renderer; import game.sprite; +import resources.texture; export namespace game { @@ -25,11 +26,9 @@ export namespace game : engine_(engine) {} - // No copy operations + // No copy or move operations because we have a reference to the engine Game(const Game&) = delete; Game& operator=(const Game&) = delete; - - // No move operations - TODO? Game(Game&&) = delete; Game& operator=(Game&&) = delete; @@ -37,7 +36,12 @@ export namespace game void initialize() { - sprite_ = std::make_unique(engine_.get_renderer(), "assets/neocat.png", 100, 100); + auto texture = resources::Texture::load_from_file( + engine_.get_renderer().get_sdl_renderer(), + "assets/neocat.png" + ); + + sprite_ = std::make_unique(std::move(texture), 100, 100); } // Handles an SDL event. Returns true if the event has been handled. diff --git a/src/game/sprite.cppm b/src/game/sprite.cppm index 9c4d9b7..089656c 100644 --- a/src/game/sprite.cppm +++ b/src/game/sprite.cppm @@ -2,87 +2,53 @@ module; #include #include -#include export module game.sprite; import core.exceptions; import core.renderer; +import resources.texture; // TODO: Move this to a different namespace (core, drawing, ...?) export namespace game { class Sprite { - // TODO: Move texture to separate class - SDL_Texture* sdl_texture; - SDL_FRect dest_rect{0, 0, 0, 0}; + // TODO: Texture should be a reference/pointer to an object managed by a ResourceManager or similar. + resources::Texture texture_; + SDL_FRect dest_rect_{0, 0, 0, 0}; public: explicit Sprite( - core::Renderer& renderer, - const std::string& filename, + resources::Texture&& texture, const int width, const int height ) + : texture_{std::move(texture)} { - SDL_Surface* texture_surface = IMG_Load(filename.c_str()); - if (texture_surface == nullptr) { - throw core::SDLException("IMG_Load"); - } - - sdl_texture = SDL_CreateTextureFromSurface(renderer.get_sdl_renderer(), texture_surface); - SDL_DestroySurface(texture_surface); - - if (sdl_texture == nullptr) { - throw core::SDLException("SDL_CreateTextureFromSurface"); - } - - dest_rect.w = static_cast(width); - dest_rect.h = static_cast(height); + dest_rect_.w = static_cast(width); + dest_rect_.h = static_cast(height); } // Don't allow copy operations Sprite(const Sprite&) = delete; Sprite& operator=(const Sprite&) = delete; - // Move constructor - Sprite(Sprite&& other) noexcept - : sdl_texture(other.sdl_texture), - dest_rect(other.dest_rect) - { - other.sdl_texture = nullptr; - } + // Default move operations + Sprite(Sprite&& other) = default; + Sprite& operator=(Sprite&& other) = default; - // Move assignment - Sprite& operator=(Sprite&& other) noexcept - { - // Move inner resources from other - sdl_texture = other.sdl_texture; - dest_rect = other.dest_rect; - - // Reset other to make it safe for deletion - other.sdl_texture = nullptr; - - return *this; - } - - ~Sprite() - { - if (sdl_texture != nullptr) { - SDL_DestroyTexture(sdl_texture); - } - } + ~Sprite() = default; void move(const float x, const float y) { - dest_rect.x = x; - dest_rect.y = y; + dest_rect_.x = x; + dest_rect_.y = y; } void draw(const core::Renderer& renderer) const { - renderer.render_texture(sdl_texture, nullptr, &dest_rect); + renderer.render_texture(texture_.get_sdl_texture(), nullptr, &dest_rect_); } }; } diff --git a/src/resources/texture.cppm b/src/resources/texture.cppm new file mode 100644 index 0000000..bb724fa --- /dev/null +++ b/src/resources/texture.cppm @@ -0,0 +1,63 @@ +module; + +#include +#include +#include +#include + +export module resources.texture; + +import core.exceptions; +import utils.memory; + +export namespace resources +{ + class Texture + { + std::unique_ptr< + SDL_Texture, + utils::FuncDeleter + > sdl_texture_ = nullptr; + + public: + Texture() = default; + + explicit Texture(SDL_Texture* sdl_texture) + : sdl_texture_(sdl_texture) + {} + + static Texture create_from_surface( + SDL_Renderer* sdl_renderer, + SDL_Surface* sdl_surface + ) + { + SDL_Texture* sdl_texture = SDL_CreateTextureFromSurface(sdl_renderer, sdl_surface); + + if (sdl_texture == nullptr) { + throw core::SDLException("SDL_CreateTextureFromSurface"); + } + + return Texture{sdl_texture}; + } + + static Texture load_from_file( + SDL_Renderer* sdl_renderer, + const std::string& filename + ) + { + SDL_Texture* sdl_texture = IMG_LoadTexture(sdl_renderer, filename.c_str()); + + if (sdl_texture == nullptr) { + throw core::SDLException("IMG_LoadTexture"); + } + + return Texture{sdl_texture}; + } + + // TODO: Do we need this? + constexpr SDL_Texture* get_sdl_texture() const + { + return sdl_texture_.get(); + } + }; +} diff --git a/src/utils/memory.cppm b/src/utils/memory.cppm new file mode 100644 index 0000000..d1e16ad --- /dev/null +++ b/src/utils/memory.cppm @@ -0,0 +1,21 @@ +module; + +export module utils.memory; + +export namespace utils +{ + /** + * Template to generate deleters for `std::unique_ptr` from functions, e.g. to free SDL resources. + * + * @tparam delete_func Function that takes a pointer to a resource and deletes the resource. + */ + template + struct FuncDeleter + { + template + constexpr void operator()(T* ptr) const noexcept + { + delete_func(ptr); + } + }; +}