SFML community forums

Help => General => Topic started by: ILostMyAccount on December 22, 2014, 02:33:43 pm

Title: How should the code be organized?
Post by: ILostMyAccount on December 22, 2014, 02:33:43 pm
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...

Title: Re: How should the code be organized?
Post by: Raincode on December 22, 2014, 03:28:06 pm
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.
Title: Re: How should the code be organized?
Post by: cob59 on December 22, 2014, 04:37:13 pm
My usual game classes:

GameEngine:
* created and run inside the main()
* handles my scenes under a state pattern (http://gameprogrammingpatterns.com/state.html) 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.
Title: Re: How should the code be organized?
Post by: ILostMyAccount on December 22, 2014, 06:56:13 pm
Thank you   :D
Title: Re: How should the code be organized?
Post by: ILostMyAccount on December 23, 2014, 12:45:08 pm
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?
Title: Re: How should the code be organized?
Post by: Ixrec on December 23, 2014, 12:48:56 pm
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.
Title: Re: How should the code be organized?
Post by: ILostMyAccount on December 23, 2014, 12:52:47 pm
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
Title: Re: How should the code be organized?
Post by: ILostMyAccount on December 23, 2014, 01:01:37 pm
Let's say something like:
Quote
#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();

}
Title: Re: How should the code be organized?
Post by: Ixrec on December 23, 2014, 01:07:45 pm
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.
Title: Re: How should the code be organized?
Post by: ILostMyAccount on December 23, 2014, 01:16:22 pm
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   :D
I guess i was blind I couldn't find the "Code" anywhere in the page so i used quote instead  :-[
Title: Re: How should the code be organized?
Post by: Ixrec on December 23, 2014, 01:22:37 pm
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 (http://www.bromeon.ch/libraries/thor/v2.0/tutorial-resources.html).
Title: Re: How should the code be organized?
Post by: ILostMyAccount on December 23, 2014, 08:08:40 pm
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?
Title: Re: How should the code be organized?
Post by: Ixrec on December 23, 2014, 08:22:00 pm
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.

Quote
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.
Title: Re: How should the code be organized?
Post by: cob59 on December 24, 2014, 11:01:27 am
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. :)
Title: Re: How should the code be organized?
Post by: Ixrec on December 24, 2014, 12:08:47 pm
Quote
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.
Title: Re: How should the code be organized?
Post by: grok on December 24, 2014, 12:41:35 pm
ILostMyAccount,
based on your example above I'd like to point out about the tricky (well, not so much, but still) moment about managing the allocated resources. In your version the Engine doesn't *own* the window, therefore it must not delete it in the current scenario. It is OK, but you have to deal with deallocation somewhere *outside* in case it was allocated on the heap.
This adds an additional burden to remember that.
The problem is that if you allocate window on the stack like below:
sf::RenderWindow window(sf::VideoMode(800, 600), "SFML window");
Engine engine(&window);
 
you must not delete it from anywhere.
But, keeping in mind that your Engine takes a window by pointer, it makes things worse (it is not clear whether or not it should perform deallocation).

Therefore, I'd suggest that your Engine handle all the work related to window internally, i.e.: allocation and deallocation. In this case you won't need to remember about that moment, thus it'd lead to fewer possible mistakes.

Of course, it is personal and your approach works as long as you remember what you're doing.

Just my 5 cents.
Title: Re: How should the code be organized?
Post by: Ixrec on December 24, 2014, 12:48:26 pm
Well, you should never have a raw *owning* pointer in modern C++ anyway.  You should only ever use raw pointers to *point* to something that somebody else owns (and only when references won't work for whatever reason).  So in principle that "should I delete it?" ambiguity should never even be an issue, but if nobody else in main() ever uses the window (an assumption I tried not to make earlier) then it's true that having it fully owned by the Engine would be simpler and clearer.
Title: Re: How should the code be organized?
Post by: cob59 on December 24, 2014, 01:53:06 pm
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.
That's why I asked him first.
Variables should be declared as locally as possible, so if there's not a second Engine instance in his main that needs a reference to the renderer window, there's no point scattering his code across Engine.cpp and main.cpp.

imo a three-line main() like that simply makes main() a piece of meaningless boilerplate
That's the point : main.cpp becomes just a bootstrap and all your program logic is contained inside classes like Engine.
That's the Qt approach.

Those tutorials seem interesting, but I'm already seeing questionable stuff in them like manual new/delete
That's just a tutorial.
No need to add complexity by adding smart pointers everywhere.

lots of this->member instead of just member
I personally prefer the this->myAttribute notation over the m_myAttribute one.
Just a matter of taste.

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.
He probably doesn't know either.
Trying to find the perfect-fitting code structure is pointless.
He just started SFML, he may be a bit lost, so he probably needs a quick HOW-TO with some basic game structure more than an exhaustive analysis on the subject.

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.
What are you talking about?
Link? Quote?
Title: Re: How should the code be organized?
Post by: Ixrec on December 24, 2014, 02:11:44 pm
If you really want to argue this...

Quote
That's why I asked him first.
It doesn't count as asking if you don't wait for the answer.  But if there is no second instance, then yes it's simpler to put it in Engine.

Quote
That's just a tutorial.
No need to add complexity by adding smart pointers everywhere.
...
He probably doesn't know either.
Trying to find the perfect-fitting code structure is pointless.
He just started SFML, he may be a bit lost, so he probably needs a quick HOW-TO with some basic game structure more than an exhaustive analysis on the subject.

I was trying to point out that you were recommending a specific program structure when we have nowhere near enough information to tell if it's appropriate for whatever he's trying to do (and we still don't know what that is!).  For some reason you now appear to be accusing me of doing exactly that.  And I never said that he needs smart pointers; I said the state stack in those tutorials should *at least* be using smart pointers if not ordinary objects, but after further reading I don't think that stack needs to exist at all.

If anything, those tutorials are the antithesis of a quick how-to: Some of them end without a runnable program, the state stack is completely unnecessary since there are only two states (one of which is trivial), the texture manager doesn't do any managing and is also unnecessary, even if the stack was necessary the implementation is extremely circuitous (the items on the stack shouldn't be adding each other to the stack or calling functions on the stack's owner), and so on.  The same result could be achieved with a lot less code.

Quote
What are you talking about?
Link? Quote?
You already linked them, and I said "second", but...

https://www.binpress.com/tutorial/creating-a-city-building-game-with-sfml-part-2-the-first-state/124 contains these two:
void GameStateStart::draw(const float dt)
{
    this->game->window.setView(this->view);

    this->game->window.clear(sf::Color::Black);
    this->game->window.draw(this->game->background);

    return;
}
void GameStateEditor::draw(const float dt)
{
    this->game->window.clear(sf::Color::Black);
    this->game->window.draw(this->game->background);

    return;
}

Quote
I personally prefer the this->myAttribute notation over the m_myAttribute one.
Just a matter of taste.

To me the huge number of this->'s make the code a lot harder to read compared to a m_ prefix, and to be honest I was worried he didn't know the this-> was unnecessary in C++, but if that's actually a style some people prefer then I guess it's fine.
Title: Re: How should the code be organized?
Post by: SpeCter on December 24, 2014, 02:17:19 pm
Quote
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:

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.

No offense, but you didn't read his code carefully enough.He does use display(). He clears the screen twice(which is probably not what the author intended) but if you look at the game.cpp you will see that he calls display(), after he draws his stuff from whatever state he is in.

Not liking his code style is another thing though.The thing which I don't like the most is the fact that he writes return in void functions.
Title: Re: How should the code be organized?
Post by: Ixrec on December 24, 2014, 02:22:00 pm
Quote
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:

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.

No offense, but you didn't read his code carefully enough.He does use display(). He clears the screen twice(which is probably not what the author intended) but if you look at the game.cpp you will see that he calls display(), after he draws his stuff from whatever state he is in.

Ah, you're right, he does.  I guess it's not quite as bad then, though if even the author forgot he had a clear() call up in Game::gameLoop() when writing those draw() functions (and made me forget the display() was up there), that's still a strong sign that he's overcomplicated the basic game loop.
Title: Re: How should the code be organized?
Post by: SpeCter on December 24, 2014, 02:54:16 pm
I would say, that you can't forget the clear and display in the gamestates(since you only need them once in the mainloop), which makes the code more failsafe. Not sure if that was the authors intent(he could have dropped the clear in his gamestates), but there is something positive about it ;)

You are right that it is rather complicated for the result, but keep in mind, that the author intended the reader to extend it with the aquired knowledge, so it could very well be that you add more states like pause,options,game over, game won(however that would be achieved) and then it is not so complicated anymore :D

Title: Re: How should the code be organized?
Post by: Ixrec on December 24, 2014, 03:08:02 pm
Well, in this case my real issue with the complexity is that the states are constructing each other and adding each other to the stack and accessing various things on the object that owns the stack.  So as implemented, adding additional states is likely to create a lot of bugs or subtle dependencies that newbies wouldn't even know to look out for.

If it was a *good* state manager, and there were one or two more states or transitions so that a newbie could get the idea of what this is actually for, and it was developed in a more iterative fashion (eg, start out with a simple switch() statement that will work and compile now, then make it complicated later), then I'd be all for it.

But we should probably wait for ILostMyAccount to return with an actual question.  Odds are none of this matters to him at all yet.
Title: Re: How should the code be organized?
Post by: ILostMyAccount on December 24, 2014, 10:44:42 pm
Thank you for the answers.
Yes, a tutorial/guide or a good project to refer to would be nice, I'm just starting with SFML, I've read these: http://www.sfml-dev.org/tutorials/2.2/ but I really have no idea how to organize everything, I hate when I have too much stuff in my main if it's not necessary.

Title: Re: How should the code be organized?
Post by: SpeCter on December 24, 2014, 10:51:35 pm
The way the code is structured in the tutorial is not that bad, if you take it with a grain of salt, but there is a book made from sfml members which is really good ;)
And there is no optimal solution to how to structure your code. Some code structures are better then others for specific projects.
Title: Re: How should the code be organized?
Post by: Nexus on December 25, 2014, 10:43:02 am
http://en.sfml-dev.org/forums/index.php?topic=5187
...and many more :)