How should the code be organized?

[ILostMyAccount]

I know that there are lots of ways but what do you do usually?
What's inside your main? Where do you initialize your sprites?
I've always used engines so I didn't really have to worry about it too much but now...

[Raincode]

Well you can keep your main nice and small by putting everything in a Game/Application class, creating an instance of that in main and calling e.g. its run function.

You should use some sort of ResourceHolder class for the SFML resources. Check on the internet or other threads, you should find something.

Generally you should be able to initialize your sprites in Constructors. Constructors are there to put your Objects into a valid state when created.

Overall, your question is very unspecific. I'm guessing that is the reason why the replies aren't exactely flying in.

[cob59]

My usual game classes:

GameEngine:
* created and run inside the main()
* handles my scenes under a state pattern so I know which one to run, and whether I should switch to an other one
* handles the sf::RenderWindow and all the global SFML stuff
* the run() method contains the classic SFML pollEvent() loop

Scene:
* created/updated/displayed/destroyed by the GameEngine
* implements sf::Drawable according to a composite pattern (i.e. contains sub-sprites and other drawable elements)
* contains an OnUpdate(sf::Time elapsedTime) method to update the physics engine (if any) or any entity of the scene (charac animation, menu cursor blink, etc).
* Concrete implementations could be: TitleScene, SettingsMenuScene, WorldmapScene, Level1Scene, etc

GameEvent:
* Similar to sf::Event but with my own events
* Used along an observer pattern
* Example: TitleScene uses a GameEvent to notify its subscribers that the "New game" button has been pressed. GameEngine is notified and updates its state diagram accordingly.

[ILostMyAccount]

Thank you   

[ILostMyAccount]

One last thing, It's ok if I make a .cpp+.h that just contains a namespace where i declare and inizialize my sprites and sounds?

[Ixrec]

If you think that takes up enough code that putting it in separate files makes your code base easier to work with, then go for it.

Though I would think "initializing sprites and sounds" involves loading some resources from files, which can't really be done in a C++ initialization, so you may need an actual class with a constructor.

[ILostMyAccount]

If you think that takes up enough code that putting it in separate files makes your code base easier to work with, then go for it.

Though I would think "initializing sprites and sounds" involves loading some resources from files, which can't really be done in a C++ initialization, so you may need an actual class with a constructor.

I would add a "LoadSprites()" function to initialize them

[ILostMyAccount]

Let's say something like:

#pragma once
#include <SFML/Graphics.hpp>
#include <string>

namespace Resources
{
   const std::string GAME_SPRITES_PATH = "Assets/Sprites/Game/";
   const std::string GUI_SPRITES_PATH = "Assets/Sprites/GUI/";


   sf::Texture mouseover_texture;
   sf::Sprite mouseover_sprite;
 

   bool LoadSprites();

}

[Ixrec]

Now that I've seen a bit more code, yeah, a class would definitely be better.

These may sound like nitpicks, but the problems with structuring your code that way are:
1) You may try to use one of the filepaths or the texture from some other code, when it probably has no purpose outside these two files.
2) It's possible to try and use mouseover_sprite before LoadSprites() has been called.
3) A sprite isn't really a "resource", so I would typically let somebody else make the sprites and get textures from this file.  For a game where you expect to be adding and removing lots of sprites at runtime, you definitely want someone else handling the sprites, but maybe it's a non-issue for you.

To solve 1 and 2, I would recommend using a class here.  In your main(), you already need to have

Resources::LoadSprites();

so we might as well change that to:

Resources resources; // constructor handles loading all the textures

This class could then mark as private the things that should be private, and the use of a constructor ensures it's impossible to try using mouseover_sprite before the texture loading has been done.


P.S. Use code tags to make your post more readable.  When you write a post there's a little "Code" box in the top right.  Select C++ from it to get the C++ code tags I was just using.

[ILostMyAccount]

Now that I've seen a bit more code, yeah, a class would definitely be better.

These may sound like nitpicks, but the problems with structuring your code that way are:
1) You may try to use one of the filepaths or the texture from some other code, when it probably has no purpose outside these two files.
2) It's possible to try and use mouseover_sprite before LoadSprites() has been called.
3) A sprite isn't really a "resource", so I would typically let somebody else make the sprites and get textures from this file.  For a game where you expect to be adding and removing lots of sprites at runtime, you definitely want someone else handling the sprites, but maybe it's a non-issue for you.

To solve 1 and 2, I would recommend using a class here.  In your main(), you already need to have

Resources::LoadSprites();

so we might as well change that to:

Resources resources; // constructor handles loading all the textures

This class could then mark as private the things that should be private, and the use of a constructor ensures it's impossible to try using mouseover_sprite before the texture loading has been done.


P.S. Use code tags to make your post more readable.  When you write a post there's a little "Code" box in the top right.  Select C++ from it to get the C++ code tags I was just using.

Thank you for the help! I'll use a class then   
I guess i was blind I couldn't find the "Code" anywhere in the page so i used quote instead 

[Ixrec]

One last thing, since we were talking about a Resources class:

I have no way of knowing if this will ever apply to what you're trying to make, but if you get to the point where you want more advanced resource management that loads each resource only when needed and unloads them when they're no longer needed, thor::ResourceCache can do a lot of the work for you.

[ILostMyAccount]

Hey guys sorry if I keep posting dumb questions but it's basically the first time in game development without an engine that takes care of everything, so...

Based on your suggestions I wrote this:

#include "Engine.h"


Engine::Engine(sf::RenderWindow *window)
{
        window_ = window;
}

Engine::Engine(sf::RenderWindow *window, bool vsync)
{
        window_ = window;
        window_->setVerticalSyncEnabled(vsync);
        vsync_status_ = vsync;
}

Engine::~Engine()
{
}

void Engine::Update(sf::Time elapsed_time)
{
        sf::Event e;
        while (window_->pollEvent(e))
        {
                if (e.type == sf::Event::Closed)
                {
                        window_->close();
                        this->~Engine();
                }
        }
        //Do something like:
        //map.Draw();
        //player.Update(e,elapsed_time);
        //enemy1.Update(elapsed_time);
        //enemy2.Update(elapsed_time);
        //using global instances maybe?
        window_->clear();
        //Engine related draws...
        window_->display();
}


void Engine::ChangeRenderWindow(sf::RenderWindow *window)
{
        window_ = window;
}

void Engine::ToggleVsync()
{
        vsync_status_ = !vsync_status_;
        window_->setVerticalSyncEnabled(vsync_status_);
}

bool Engine::GetVsyncStatus()
{
        return vsync_status_;
}
 

I'm quite sure the Update it's completely inefficient so... suggestions?

[Ixrec]

Based solely on the code in that post, I don't see any efficiency problems with the Update method.  Don't worry about it until you have a real performance problem, and you've used a profiler to pin down exactly what needs optimizing.

There are a few non-efficiency problems I do see though:

- Don't define a destructor if you aren't going to put anything in it.
- Never manually call a destructor (this->~Engine(); ) unless you're doing the kind of very low-level memory management stuff where things like placement new/delete are commonplace.  Which you aren't.
- You need a better way to handle the "game's over now, stop updating" logic than trying to delete yourself.  It would be better to either put the actual game loop in main(), and have it call Engine::isRunning() to tell if the game has ended, or put the whole loop in an Engine method that main() simply calls once.
- Engine::ChangeRenderWindow seems like a strange feature.  Do you expect to have an actual use for this?
- Generally speaking, a setter is better than a "toggler" (in this case, ToggleVsync), because whatever code is using the "toggler" has to remember what the current state is to know whether to call the toggler or not, so you end up duplicating the state that's used to track the state...which is not good.
- Your comments imply you're calling draw methods before window.clear().  Don't do that.

it's basically the first time in game development without an engine that takes care of everything, so...

In that case, I would recommend reading http://gameprogrammingpatterns.com/contents.html.  Most engines you've seen use several of these patterns, and you'll likely need to reimplement simpler versions of a few of them yourself.

[cob59]

Why did you keep the sf::RenderWindow outside of your Engine?
You should make it a private attribute, initialized in the default constructor.
You basically need an extra method:

void Engine::run()
{
  sf::Clock clock;
  while (this->window->isOpen())
  {
    sf::Time elapsedTime = clock.restart();
    this->Update(elapsedTime);
  }
}
 

So you main can look like this :

int main()
{
  Engine engine;
  engine.run();
  return 0;
}
 

EDIT
I came across this (quite recent) tutorial:
https://www.binpress.com/tutorial/creating-a-city-building-game-with-sfml/137

It's pretty good IMO.
His Game class and the way he manages his states with a GameState stack is most certainly what you're looking for.

[Ixrec]

Why did you keep the sf::RenderWindow outside of your Engine?
You should make it a private attribute, initialized in the default constructor.
You basically need an extra method:

There's more than one valid way to structure these programs.  Yours is valid, but you haven't given any real reason why it's wrong for him to put the RenderWindow outside the Engine class, or not use the Engine::run() approach.

I do think it's a bit odd to have it outside the Engine class, but personally, I think it's odd to have an "Engine" class at all.  imo a three-line main() like that simply makes main() a piece of meaningless boilerplate, when it should contain some "real code".  I'd rather have main() contain the actual game loop, instantiate all the entities and systems I need, then call handleInput(), update() and draw() on them every frame.  But that's just me.  It's more a matter of personal taste than anything else.  I suggested two possibilities in my post (one of which is yours).

Those tutorials seem interesting, but I'm already seeing questionable stuff in them like manual new/delete, lots of this->member instead of just member, and a stack of pointers that should probably be a stack of objects (or at least smart pointers), so I would be cautious about basing your code on it.  We also don't know enough about ILostMyAccount's program to tell if he needs a top-level GameState stack, so we shouldn't be asserting that either.


Update: His second tutorial shows multiple draw functions with a clear(), a draw() and no display().  I think that's a severe enough mistake to warrant saying do not use these tutorials.