Relation between an object and its renderer

Question

Recently I decided to start optimizing code in the game development field.

I found this pacman project on github a good starting point: https://github.com/LucaFeggi/PacMan_SDL/tree/main

I found a lot of things that deserve an optimization. One of the most things that caught my attention is that a lot of classes have their Draw (render) method inside them, such as Pac, Ghost, Fruit classes...

So I decided to make a Renderer class, and move to it the Draw method and the appropriate fields that the latter works on.

Let us take the Pac class inside the PacRenderer.hpp as an example, to clarify things:

class Pac : public Entity{
    public:
        Pac();
        ~Pac();
        void UpdatePos(std::vector<unsigned char> &mover, unsigned char ActualMap[]);
        unsigned char FoodCollision(unsigned char ActualMap[]);
        bool IsEnergized();
        void ChangeEnergyStatus(bool NewEnergyStatus);
        void SetFacing(unsigned char mover);
        bool IsDeadAnimationEnded();
        void ModDeadAnimationStatement(bool NewDeadAnimationStatement);
        void UpdateCurrentLivingPacFrame();
        void ResetCurrentLivingFrame();
        void WallCollisionFrame();
        void Draw();
    private:
        LTexture LivingPac;
        LTexture DeathPac;
        SDL_Rect LivingPacSpriteClips[LivingPacFrames];
        SDL_Rect DeathPacSpriteClips[DeathPacFrames];
        unsigned char CurrLivingPacFrame;
        unsigned char CurrDeathPacFrame;
        bool EnergyStatus;
        bool DeadAnimationStatement;
};

So according to what I described above, the PacRenderer class will be:

class PacRenderer {
public:
        PacRenderer(std::shared_ptr<Pac> pac);
        void Draw();
private:
        LTexture LivingPac;
        LTexture DeathPac;
        SDL_Rect LivingPacSpriteClips[LivingPacFrames];
        SDL_Rect DeathPacSpriteClips[DeathPacFrames];
}

PacRenderer::PacRenderer(std::shared_ptr<Pac> pac)
{
    this->pac = pac;

    //loading textures here instead of in the Fruit class
    LivingPac.loadFromFile("Textures/PacMan32.png");
    DeathPac.loadFromFile("Textures/GameOver32.png");
    InitFrames(LivingPacFrames, LivingPacSpriteClips);
    InitFrames(DeathPacFrames, DeathPacSpriteClips);
}

After moving Draw() and 4 fields from the Pac class.

At the beginning, I thought that it is a very good optimization, and separation between game logic and graphics rendering.

Until I read this question and its right answer: How should renderer class relate to the class it renders?

I found that what the right answer describes as

you're just taking a baby step away from "objects render themselves" rather than moving all the way to an approach where something else renders the objects

applies exactly to my solution.

So I want to verify if this description really apply to my solution.

Is my solution good, did I actually separate game logic from graphics rendering?

If not, what is the best thing to do in this case?

Answer 1

did I actually separate game logic from graphics rendering?

No, at some point the character must be drawn, adding an additional class per object does nothing to improve separation.

SDL is the abstraction layer.

As far as code size, CPU performance, or memory optimization is concerned, the proposed split will perform slightly worse than the original code.

---

Adding a DrawPac() method, along with the other objects' draw calls to a single Render class would separate render calls from objects.

This solution is equivalent to the original code, draw call in object, in most aspects including performance. Convention(tradition) alone groups the code in the object being drawn.

---

Remember that huge monolithic code generates the fastest and smallest code, but it is also a pain to maintain. This attribute in code should not be feared or corrected without cause.

Note, I am not advocating for writing new code this way. There are many dangers to over optimization.

Do what works best for you and optimize when needed.

---

My version of Pac-man kept the Ghosts, Pac-man, Transporters(screen wrap) and Fruit as objects, and everything else(dot, power-dot, walls) was done in a single "world" object from one array drawn in a single loop.

---

Just glancing at the Board code, the Ghost checks have no purpose here. So uncoupling objects would be a better place to start refactoring the code.

Answer 2

Strictly speaking you did not optimize the code, you restructured it. You have performed a refactoring (but probably without tests).

There is a principle of locality inherent in object oriented programming. Meaning, functionality that belongs to an object should be kept at the object. Having a render method in each class is nothing to be condemned per se. There is even a design pattern for it, the command pattern.

In the presented case you collected all render stuff in one extra layer or package/namespace, but the control flow dependency is still there between each Pac and PacRenderer, Fruit and FruitRenderer etc. Note the reference you have to pipe to each renderer to make this approach work. If you truly had abstracted/separated things, such a dependency would not be present anymore.

The underlying idea is that changes on the object probably make changes of the rendering necessary. So the best/simplest case is, object logic and render code are held together. Similar parts could be moved to a common parent class (or static helper classes), if this makes things easier to handle.

Don't worry tho, these are all best practices coming with more experience. Just keep going. You did the best thing here: Trying to change code and learn something from it is the best teacher you can have!