Game structure help

[Canvas]

I'm in the early stages of a game I would love to create and i'm trying to work out how the game program should be structured (classes, headers etc). I'm using C++ and SFML for this project.
Let me first by showing you my basic image of the structure. Don't pay attention to MainMenuState.cpp and NewGameState.cpp



Game.cpp will have an instance of all those services, it will also have the MainMenuState, NewGameState and LevelState. Now here is my problem.
Game.cpp will have a method that will call the RenderService.RenderWindow, I'm trying to work out exactly how I should code this method? Should I pass in an arrayList of sf::sprites? If I do this, the LevelState.cpp will need an ArrayList of sf::Sprites. which will be updated on each game step. But lets say the Hero.cpp has 1 sf::Sprite. the Map.cpp has 20 sf::sprites and Monster.cpp has 1 sf::Sprite. this would mean I have to pass 22 sf::Sprites back to the RenderService every game step (60fps). so that is 1320 sf::Sprites per second back to the RenderService. I understand game structure is a very hard thing to do and hard to implement correctly, because there isn't a "set" way. I have created a Vector which holds sf::Sprites and every time I call my Render function I clear the vector before so everything is removed for memory. This works fine. Would that be the answer? But as I stated before is passing a lot of sf::Sprites to be rendered ok?
 
Here is my simple SFML program to test this

#include <SFML/Graphics.hpp>
#include "ImageService.h"
#include "DisplayObject.h"
#include "Enums.h"
 
ImageService is;
DisplayObject displayObject;
 
int main()
{
        sf::RenderWindow window(sf::VideoMode(1024, 768), "SFML works!");
        sf::CircleShape shape(100.f);
        shape.setFillColor(sf::Color::Green);
 
        is.Images[image1].loadFromFile("ship1.tga");
        displayObject.sprite.setTexture(is.Images[displayObject.spriteType]);
        std::vector<sf::Sprite> displayObjects;
 
        while (window.isOpen())
        {
                displayObjects.clear();
                displayObjects.push_back(displayObject.sprite);
 
                sf::Event event;
                while (window.pollEvent(event))
                {
                        if (event.type == sf::Event::Closed)
                                window.close();
                }
 
                window.clear();
 
                window.draw(shape);
                //Draw out the selected sprites
                for (int i = 0; i < displayObjects.size(); i++)
                {
                        window.draw(displayObjects.at(i));
                }
 
                window.display();
        }
 
        return 0;
}

Let me know if you would like me to explain any part of my program

[Ixrec]

If the main concern is simply the act of passing large numbers of sprites around, that's not an issue at all.  Put them in a container and pass a pointer or reference to that container.

Since you mistakenly referred to an "arrayList", I'm going to assume you're more familiar with languages like Java that try to hide things like copy and reference semantics from you.  In C++, these things are very explicit, so assuming you know the syntax for them you don't have to worry about expensive copies being made without your knowledge.

The part you do need to think about is ownership semantics.  Typically, one of your objects will actually contain the real vector, rather than a mere pointer or reference to it, and the vector will be automatically cleaned up when that object is destroyed, because of RAII (go read about that, it's important).  Presumably, whatever object is calling the render function should be the one owning this vector, and it will simply pass a reference to everyone else.  It doesn't sound like you need anything complicated like shared pointers.

[Canvas]

Well the vector that would store the sf::Sprites would live inside of the RenderService.cpp, so every time the Game.cpp Update function is called, it will call the RenderService.cpp Update function (RenderService object will live inside game.cpp) which then the RenderService will gather the sf::Sprites store them inside of a vector and then draw them out, that vector would then be cleared at the end of the render state, to release the memory from that vector, then on each Game Update call that vector is once again populated and then cleared again.

So are you saying populating a vector and clearing it every frame call is ok?

[Ixrec]

Okay, now it sounds like the issue is populating and clearing a container, not passing one around.

This is fine if the objects are very lightweight.  An sf::Sprite is pretty lightweight, since it just contains some numbers and a pointer to an sf::Texture.  The texture is the heavyweight part.  So from an efficiency standpoint, if it's just the sprites you're clearing and recreating every frame, that's probably okay, as long as you aren't doing the same thing with your textures.

From a design standpoint, this is probably a bad thing, since each sprite is probably linked to a conceptual entity on the screen, and it's unlikely you're displaying a completely different set of entities every single frame.

[Canvas]

Ok sweet, so I think for the moment a vector of sf::Sprites is fine. Well also Ixrec about the

"From a design standpoint, this is probably a bad thing, since each sprite is probably linked to a conceptual entity on the screen, and it's unlikely you're displaying a completely different set of entities every single frame."

I could create a basic object called DisplayObject which holds a sf::Sprite and a bool, the bool would be to indicate if the sprite needed to be updated e.g. size, rotation, colour, position etc. But if I were to do that, then do the extra "IF" checks on that bool would that save create more of a heavy process or would it overall save time due to the sprite never being passed again until it needs to be?

[Ixrec]

Better would be to simply update the size/rotation/color/position every frame, whether you need to or not.  It's much simpler, the operations involved are all completely trivial (we're only talking a half-dozen floating point assignments per sprite), and will probably be easier to understand and--more importantly--change in the future.  There's definitely no need to wrap sf::Sprites in your own class (at least not yet).

For the record, checking a bool and assigning a float are trivial actions that should only take nanoseconds to complete, and you should never be afraid of doing them several times a frame.  Expensive actions that are usually worth minimizing include draw() calls, system calls, file I/O and network operations.  Most other stuff you shouldn't worry about until you can prove with a profiler that it's a real problem.

An example of where this sort of if(bool) optimization is worthwhile would be sf::Text.  That class will not go to the trouble of rendering all the glyphs it needs and computing their positions until and unless it actually needs to, because these operations are non-trivial.

[Canvas]

Ixrec that has really cleared up a lot for me and for the future of my project , thank you very much, but is it ok if I ask if this functionally is ok,

Basically I will have a vector of sf::Sprite. when the game update is called, the render service will then work out what sprites needs to be drawn. and the vector will then be populated, once populated it will then update the screen, and at the end of that draw method, the vector will be cleared, and the same process will happen every frame. the sprites themselves will be updated by the objects they are from. the render service just cares about taking sprites and then drawing them. Does that seem like a way to resolve this situation?

[Ixrec]

That sounds exactly the same as before, so I'm not really sure how it resolves anything.  Still, my objection to that is solely about design, not efficiency, so feel free to go ahead with it.  You can always refactor later.

[Canvas]

Ok so lets try this example

We have 5 objects that have a sf::Sprite.
When the renderService is called it will gather those 5 sprites and draw them.
It will then render those 5 sprites.
Now lets say object 1, 2 and 4 have been updated
so now we need to update sprite 1,2 and 4, but we don't need to update sprite 3 or 5.
How would I go about doing this with a vector of sf::Sprite?

[Ixrec]

Not really sure where we're going with this, but I guess I'll give a direct answer to that one.

That depends on how you know about the need to update, but assuming the sequence of events in code corresponds to what you just wrote, you would just call whichever set* methods you need on sprites[0], sprites[1] and sprites[3].

I feel like in order to answer the question you're actually trying to ask, I would have to see some actual code around the parts where you're unsure.  And it may be that I misunderstood something and reconstructing the vector every frame actually is the better design decision.  That would be strange, but without code it's hard to judge.

[dwarfman78]

but we don't need to update sprite 3 or 5.

just do it, even if the containing object hasn't changed, you are most likely to kill your game performance with the collision detection (for instance) than with the updating of each sprite during each loop turn.

[Canvas]

Ok so, here is a snippet of code I just created.

#include <SFML/Graphics.hpp>
#include "ImageService.h"
#include "DisplayObject.h"
#include "Enums.h"

ImageService is;
DisplayObject obj1;
DisplayObject obj2;
DisplayObject obj3;
DisplayObject obj4;
DisplayObject obj5;
int timer = 0;

int main()
{
        sf::RenderWindow window(sf::VideoMode(1024, 768), "SFML works!");

        is.Images[image1].loadFromFile("Block.png");
        obj1.sprite.setTexture(is.Images[obj1.spriteType]);
        obj1.sprite.setColor(sf::Color(255, 0, 0));
        obj1.sprite.setPosition(100,100);
        obj2.sprite.setTexture(is.Images[obj2.spriteType]);
        obj2.sprite.setColor(sf::Color(0, 255, 0));
        obj3.sprite.setPosition(200, 100);
        obj3.sprite.setTexture(is.Images[obj3.spriteType]);
        obj3.sprite.setColor(sf::Color(0, 0, 255));
        obj4.sprite.setPosition(300,100);
        obj4.sprite.setTexture(is.Images[obj4.spriteType]);
        obj4.sprite.setColor(sf::Color(127, 127, 127));
        obj5.sprite.setPosition(400,100);
        obj5.sprite.setTexture(is.Images[obj5.spriteType]);
        std::vector<sf::Sprite> displayObjects;

        displayObjects.push_back(obj1.sprite);
        displayObjects.push_back(obj2.sprite);
        displayObjects.push_back(obj3.sprite);
        displayObjects.push_back(obj4.sprite);
        displayObjects.push_back(obj5.sprite);

        window.setFramerateLimit(60);

        while (window.isOpen())
        {
                timer++;

                if (timer == 60)
                {
                        obj1.sprite.setPosition(100, 200);
                        obj2.sprite.setPosition(200, 200);
                        obj4.sprite.setPosition(400, 200);
                }

                for (int i = 0; i < displayObjects.size(); i++)
                {
                        window.draw(displayObjects.at(i));
                }

                sf::Event event;
                while (window.pollEvent(event))
                {
                        if (event.type == sf::Event::Closed)
                                window.close();
                }

                window.clear();
                //Draw out the selected sprites
                for (int i = 0; i < displayObjects.size(); i++)
                {
                        window.draw(displayObjects.at(i));
                }

                window.display();
        }

        return 0;
}

a DisplayObject at the moment just holds a Sprite and has a Sprites, Sprites is an enum which is used for loading the correct sprite for that object.

So as you can see we have 5 display objects, these are then pushed onto the vector, now that I have updated obj1, obj2 and obj4, how would I then tell the vector to update these? I can just say at position 0, 1 and 3, but if I were to do that where would I get the numbers 0, 1 and 3 from? Would i have to do something on the lines of this?

#include <SFML/Graphics.hpp>
#include "ImageService.h"
#include "DisplayObject.h"
#include "Enums.h"

ImageService is;
DisplayObject obj1;
DisplayObject obj2;
DisplayObject obj3;
DisplayObject obj4;
DisplayObject obj5;
int timer = 0;

int main()
{
        sf::RenderWindow window(sf::VideoMode(1024, 768), "SFML works!");

        is.Images[image1].loadFromFile("Block.png");
        obj1.sprite.setTexture(is.Images[obj1.spriteType]);
        obj1.sprite.setColor(sf::Color(255, 0, 0));
        obj1.sprite.setPosition(100,100);
        obj2.sprite.setTexture(is.Images[obj2.spriteType]);
        obj2.sprite.setColor(sf::Color(0, 255, 0));
        obj2.sprite.setPosition(200, 100);
        obj3.sprite.setTexture(is.Images[obj3.spriteType]);
        obj3.sprite.setColor(sf::Color(0, 0, 255));
        obj3.sprite.setPosition(300,100);
        obj4.sprite.setTexture(is.Images[obj4.spriteType]);
        obj4.sprite.setColor(sf::Color(127, 127, 127));
        obj4.sprite.setPosition(400,100);
        obj5.sprite.setTexture(is.Images[obj5.spriteType]);
        std::vector<sf::Sprite> displayObjects;

        displayObjects.push_back(obj1.sprite);
        obj1.positionInVector = displayObjects.size() - 1;
        displayObjects.push_back(obj2.sprite);
        obj2.positionInVector = displayObjects.size() - 1;
        displayObjects.push_back(obj3.sprite);
        obj3.positionInVector = displayObjects.size() - 1;
        displayObjects.push_back(obj4.sprite);
        obj4.positionInVector = displayObjects.size() - 1;
        displayObjects.push_back(obj5.sprite);
        obj5.positionInVector = displayObjects.size() - 1;

        window.setFramerateLimit(60);

        while (window.isOpen())
        {
                timer++;

                if (timer == 60)
                {
                        obj1.sprite.setPosition(100, 200);
                        displayObjects.at(obj1.positionInVector) = obj1.sprite;
                        obj2.sprite.setPosition(200, 200);
                        displayObjects.at(obj2.positionInVector) = obj2.sprite;
                        obj4.sprite.setPosition(400, 200);
                        displayObjects.at(obj4.positionInVector) = obj4.sprite;
                }

                for (int i = 0; i < displayObjects.size(); i++)
                {
                        window.draw(displayObjects.at(i));
                }

                sf::Event event;
                while (window.pollEvent(event))
                {
                        if (event.type == sf::Event::Closed)
                                window.close();
                }

                window.clear();
                //Draw out the selected sprites
                for (int i = 0; i < displayObjects.size(); i++)
                {
                        window.draw(displayObjects.at(i));
                }

                window.display();
        }

        return 0;
}

It does seem a little long winded, but it does work. Is that what you were thinking?

Update

dwarfman78 I do agree with you, but I don't want to get into a habit that is seen as "wrong" or "not great for performance" code.

[Ixrec]

Premature optimization is just as bad a habit to get into.  Don't waste time optimizing things that only take a handful of machine instructions to begin with.  Calls to trivial methods like setPosition() will NEVER be your bottleneck.

Creating wrapper classes that don't actually do anything is also a bad habit.  Just use sf::Sprites directly.

I'm not sure I understand the problem anymore.  Why are you updating the state of objects not in your vector, then trying to put them in your vector?  Why not simply update the ones in the vector directly?  If you're seriously asking how to do that, then you need to go learn how vectors work; simply writing displayObjects[0].setPosition(100, 100) will work just fine.

[Canvas]

My vector is taking sf::Sprite only. Well in my game a level could have a number of objects, such as mosnters, blocks, coins etc. So for each object it has it's own sf::Sprite. which it handles when moving, updating, changing etc. So this sprite is passed to the Vector so the RenderService can then use that vector with that sprite in to display it. The sf::Sprite (s) do not start in the vector, the vector is populated with sprites from objects that already exist.

[Ixrec]

I've lost track of what the question is now.

By the way, why do you even need a RenderService?  Why not simply give each entity (monster, block, coin, etc) a draw() method that calls draw() on its internal sprite, and call that method on every entity each frame?


Edit: My instinct is that at this point you should go off and start implementing everything.  It seems like you'd learn more that way than playing twenty questions with us.