A C++ Game Engine

[misty vessel]

Hello! This is my first time building such a big project, and I've relized that my code base is getting messy. Please review my code and let me know if there is some better way that I can structure my code. Any criticism is appreciated!
https://github.com/cheyao/opengl

[onyx tendon]

You could try to combine overlapping features of related classes (like the ones in include/systems) into a template class instead of everything being a flat landscape of classes

[proven igloo]

You have memory Leak in destructor: In the destructor the components in mComponents are not being deleted correctly, The loop should delete the components and then remove them from the list to prevent that shit

opengl/src/ui
/UIScreen.cpp

#include "ui/UIScreen.hpp"

#include "game.hpp"
#include "ui/UIComponent.hpp"

#include <string>
#include <algorithm> // for std::find
#include <memory>    // for std::unique_ptr

#ifdef IMGUI
#include <imgui.h>
#endif

UIScreen::UIScreen(Game* game, const std::string name)
    : mGame(game), mState(UIScreen::ACTIVE), mName(name) {
    game->addUI(this);
}

UIScreen::~UIScreen() {
    mGame->removeUI(this);

    // Properly delete components to prevent memory leaks
    while (!mComponents.empty()) {
        delete mComponents.back();
        mComponents.pop_back();
    }
}

void UIScreen::update(const float delta) {
    updateScreen(delta);

#ifdef IMGUI
    ImGui::Begin("UI Components");
#endif
    for (const auto& component : mComponents) {
#ifdef IMGUI
        ImGui::Text("Component: %s position: %f %f",
                    component->getName().c_str(),
                    component->getPosition().x(),
                    component->getPosition().y());
#endif
        component->update(delta);
    }
#ifdef IMGUI
    ImGui::End();
#endif
}

void UIScreen::updateScreen([[maybe_unused]] const float delta) {}

void UIScreen::draw(Shader* shader) {
    for (const auto& component : mComponents) {
        component->draw(shader);
    }
}

void UIScreen::drawText(Shader* shader) {
    for (const auto& component : mComponents) {
        component->drawText(shader);
    }
}

void UIScreen::processInput(const bool* keys) {
    for (const auto& component : mComponents) {
        component->input(keys);
    }
}

void UIScreen::touch(const SDL_FingerID& finger, const float x, const float y, const bool lift) {
    for (const auto& component : mComponents) {
        component->touch(finger, x, y, lift);
    }
}

void UIScreen::removeComponent(UIComponent* component) {
    auto iter = std::find(mComponents.begin(), mComponents.end(), component);

    if (iter != mComponents.end()) {
        delete *iter;
        mComponents.erase(iter);
    }
}

smth like this should be good

[misty vessel]

Thanks for the review! But in fact I am removing the component with the destructor of the components, so there won't be any problems.

[terse violet]

why not just

for (auto &component : mComponents) {
  delete component;
}

[sharp hamlet]

Hey, can you give some comments about Game Programming in C++ Code book?

What made you recommend that it is not worth to read it?

[onyx tendon]

That's maybe a little general or vague of a statement

[proven igloo]

Slow

[terse violet]

and checking the vector isnt empty, deleting the element at the back then popping it, for every element, isnt?

[misty vessel]

sorry for the late reply, the book goes too much into unimportant details, but it doesn't explain some more essential concepts (mostly after it started teaching opengl)