rip-off

I concur, keep writing programs. You will only know when to apply which software practises when you build up a working knowledge of how larger and larger programs are written.

An important thing is not to get too bogged down in producing "perfect" software, to the point where your ability to complete projects is degraded. It is something that I, and I would guess many, hobby developers struggle with. There is a delicate balance between being a productive programmer (in terms of the project end goals, not mere lines of code!) while simultaneously keeping the code manageable. It is only by experiencing the problems caused by being unproductive due to spending too much time making your code "perfect". and the problems caused by being unable to be productive due to unaddressed issues in the code, that you will learn when and how to strike this balance.

After several years working as a programmer, and many more as a hobbyist, I am still learning.

What about a good game with good implementation where every element implemented was carefully considered by someone who is very passionate about the project and is a talented game designer?

For a good game, this is usually the case. But I think your unstated assumption is that this "someone" is/should be one individual. For smaller games, maybe. For larger games, this is impractical.

There is nothing wrong with tinkering with a formula when it is necessary, but every step away from the original idea is likely going to detract somewhat from its personality. Of course it can improve the end product but it seems obvious  that starting out with an idea that needs a minimal amount of tinkering and having said tinkering done by the person who originally came up with the idea will result in a game with more soul.

It depends. The "soul" and "personality" include how open to collaboration you are, how you handle other people's creative inputs and willing to compromise when you might be wrong or misguided. A game designed by a tyrannical dictator may certainly have a strong soul, and you probably would feel it through the end product.

This is a very idealistic view of game design, I just think that there is nothing wrong with the ideal itself. And that it could be strived for a bit more in the gaming industry, especially the indie community.

Perhaps. I don't think it is the most pressing game design problem in the game industry, indie or not.

Very confusing. SDL's documentation for SDL_RWFromFP says:

This is not available under Win32, since files opened in an application on that platform cannot be used by a dynamically linked library

They should make it so that on Win32, this function either doesn't appear to be available, or at the very least have an alternate code path that returns an error.

This page, under "Reliance on friend name injection", claims that this behaviour is not standard C++.

Caveat: This is not an area of C++ I am very familiar with, but this is my interpretation of what the book says. My compiler happens to agree with me.

Statement 1: What does it mean it's a member of the namespace?!

It means that the function that is friended is Me::you() - that is how you would call it from main(). To friend a global function, then you'd have to write:

void globalFunction();
 
namespace SomeNamespace
{
 
    class Example
    {
        friend void ::globalFunction();
     };
}

Statement 2: So if I add a friend function in a class that is in the global namespace,because I want it to,ALL the other classes that I added in the global namespace will take that class as a friend too?

No. They are saying that when you use the friend keyword to introduce a function, this is implicitly declaring the function (if it is not already declared). In the case the class offering friendship is not in a namespace, the function implicitly declared is also part of the global namespace:

class Example
{
    friend void someFunction();
};

In this example, the friend statement doubles as a declaration of someFunction in the global namespace.

Perhaps the following complete example might be illustrative:

 
class SomeClass {
    // Implicitly declaring "someFunction" in the global namespace
    friend void someFunction();
};
 
namespace SomeNamespace
{
    class SomeNamespaceClass
    {
        // This line wouldn't compile without the (implicit) declaration above
        friend void ::someFunction();
 
        // Implicitly declaring "someNamespaceFunction" in SomeNamespace
        friend void someNamespaceFunction();
    };
}
 
// We can call these functions in main now, despite not having declared them outside the friend declarations
int main() {
    someFunction();
    SomeNamespace::someNamespaceFunction();
}
 

@rip-off: Wow. I was hoping for somebody to dissect my code like that. I really appreciate the time you took to look at my game. Thanks a lot.

You are most welcome.

I have made this change, but can you tell me the downside of using preprocessor defines and what are the benefits of using compile time constants, as for this program I don't apparently see the benefit of using constants.

The preprocessor does not understand C++. It only has a rather basic concept of token replacement. For example, a preprocessor define inside a namespace will not be restricted to replacing symbols inside that namespace.

To me the only difference I see is the program would have to allocate memory for the constants, and no memory would be allocated of the "define".

C++ does not map directly to machine code. There is no particular reason why a modern compiler, in Release mode, would allocate memory for a const value unless you do something such as take it's address.

The function setTextures() and the member m_Window could have been made private, but I'm passing the brick object to another class which would then need to call some of its functions, if I set it to private the class to which the brick was passed to will not be able to call its functions and that is why I have made this public.

That isn't quite how private works. A class may pass it's private members to any other function or class. If it does so by reference, then the class or function may write directly to the "private" value. Private does not prohibit a class designer from making the decision to share it's member with code it "trusts". It could also publish a member function pointer to a private function.

What private is intended for is to ensure that members and functions the class designer does want to keep "secret" cannot legally be accessed by other portions of the code. One reason for this is that most classes establish invariants.

There are strong invariants such as the standard containers, where if some code decided to change on of the members it could break the class (and usually the program!). A slightly "weaker" form of invariant might be covering the "business logic" or "game mechanics". That is, you might want to enforce a rule such as the player can only have so many items in their inventory, or that to obtain a new item from a shopkeeper the player must lose the corresponding number of coins from their pouch. If these kinds of code can be hidden behind a class interface that does not allow the logic to be misused, then the chances of a bug in the game logic is greatly reduced - and when one does occur it is most likely in that classes implementation.

Yes I could have made them members of the game class, but I didn't want my game class to be littered with a hundred members. So, instead I decided to make these variables local to the functions that needed them and made them static so they persist even when the function ends. These variables are not needed anywhere except the functions of they are a part of.

I believe the static variables in Game::setTextures() and Game::run() could be made non-static local variables instead.

For example:

 
switch (m_GameState)
{
case title:
    {
        Input_Manager::titleResult result = m_inpMgr.titleEvents(m_Event);        
        if (result == Input_Manager::exit)
            m_GameState = exiting;
        else if (result == Input_Manager::about)
            m_GameState = about;
        else if (result == Input_Manager::newGame)
            m_GameState = running;
    }
    break;
case about:
    {
        Input_Manager::aboutResult result = m_inpMgr.aboutEvents(m_Event);
        if (result == Input_Manager::back)
            m_GameState = title;
    }
    break;
case running:
    {
        Input_Manager::gameResult result = m_inpMgr.gameEvents(m_Event, m_Paddle, m_Ball);
        if (result == Input_Manager::prev)
            m_GameState = title;
        else if (result == Input_Manager::paused)
            m_GameState = paused;
        else if (result == Input_Manager::idle && m_Paddle.livesLeft() > 0 && isLevelWon() == false)
            /* nothing to do... */;
        else if (result == Input_Manager::idle && m_Paddle.livesLeft() > 0 && isLevelWon() == true)
            m_GameState = gamewon;
 
        else if (result == Input_Manager::idle && m_Paddle.livesLeft() < 0)
            m_GameState = gamelost;
    }
    break;
case gamelost:
    {
        Input_Manager::gamelostResult result = m_inpMgr.lostEvents(m_Event);
        if (result == Input_Manager::nada)
            /* nothing to do... */;
        else if (result == Input_Manager::mainMenu)
            m_GameState = title;
    }
    break;
case gamewon:
    {
        Input_Manager::gamewonResult result = m_inpMgr.wonEvents(m_Event);
        if (result == Input_Manager::idling)
            /* nothing to do... */;
        else if (result == Input_Manager::menu)
            m_GameState = title;
    }
    break;
case paused:
    {
        Input_Manager::pauseResult result = m_inpMgr.pauseEvents(m_Event);
        if (result == Input_Manager::doNothing)
            /* nothing to do... */;
        else if (result == Input_Manager::theMainMenu)
            m_GameState = title;
        else if (result == Input_Manager::resume)
            m_GameState = running;
    }
    break;
default:
    break;
}
 

Adding a block inside each case, I've created a separate scope for each result variable of the appropriate type. At this point, it becomes quite clear that each case can be written as a helper function which can change the current gamestate:

 
void Game::handleTitleEvents() {
    Input_Manager::titleResult result = m_inpMgr.titleEvents(m_Event);        
    if (result == Input_Manager::exit)
        m_GameState = exiting;
    else if (result == Input_Manager::about)
        m_GameState = about;
    else if (result == Input_Manager::newGame)
        m_GameState = running;
}
 
void handleAboutEvents() {
    Input_Manager::aboutResult result = m_inpMgr.aboutEvents(m_Event);
    if (result == Input_Manager::back)
        m_GameState = title;
}
 
viod Game::handleRunningEvents() {
    Input_Manager::gameResult result = m_inpMgr.gameEvents(m_Event, m_Paddle, m_Ball);
    if (result == Input_Manager::prev)
        m_GameState = title;
    else if (result == Input_Manager::paused)
        m_GameState = paused;
    else if (result == Input_Manager::idle && m_Paddle.livesLeft() > 0 && isLevelWon() == false)
        /* nothing to do... */;
    else if (result == Input_Manager::idle && m_Paddle.livesLeft() > 0 && isLevelWon() == true)
        m_GameState = gamewon;
    else if (result == Input_Manager::idle && m_Paddle.livesLeft() < 0)
        m_GameState = gamelost;
}
 
void Game::handleGameLostEvents() {
    Input_Manager::gamelostResult result = m_inpMgr.lostEvents(m_Event);
    if (result == Input_Manager::nada)
        /* nothing to do... */;
    else if (result == Input_Manager::mainMenu)
        m_GameState = title;
}
 
void Game::handleGameWonEvents() {
    Input_Manager::gamewonResult result = m_inpMgr.wonEvents(m_Event);
    if (result == Input_Manager::idling)
        /* nothing to do... */;
    else if (result == Input_Manager::menu)
        m_GameState = title;
}
 
void Game::handlePausedEvents() {
    Input_Manager::pauseResult result = m_inpMgr.pauseEvents(m_Event);
    if (result == Input_Manager::doNothing)
        /* nothing to do... */;
    else if (result == Input_Manager::theMainMenu)
        m_GameState = title;
    else if (result == Input_Manager::resume)
        m_GameState = running;
}
[code]
 
[/code]
switch (m_GameState)
{
case title: handleTitleEvents(); break;
case about: handleAboutEvents(); break;
case running: handleRunningEvents() break;
case gamelost: handleGameLostEvents(); break;
case gamewon: handleGameWonEvents(); break;
case paused: handlePausedEvents(); break;
default:
    assert(false && "Unknown gamestate");
    break;
}
 

In a much bigger project what would be a disadvantage or danger of using these static variables?

If you ever start using multiple threads then the static variables become a hazard, as they are effectively globals shared by any threads which happen to call that function. Aside from that, static variable suffer many of the downsides of global variables. It is harder to understand and reason about a function which makes use of hidden, global state. The function becomes hard to unit test in isolation because you cannot control the value of the static variable explicitly during each test.

Noted. But the reason I have it there was that it was possible for the lresult variable to get a value of "nada". So I explicitly included it there and told the compiler to not do anything if that happens.

It is unnecessary to instruct the compiler to do nothing in this case. The compiler will only do what you tell it to do. Now, there is a reasonable argument to be made that this is for the benefit of the code maintainers, you're showing that you are aware of this case but do not need to take any special steps. I would use a comment to make this crystal clear, as the first instinct of most programmers reading if(condition); is that the author has omitted some code from the branch body.

There are a couple of ways of making the intent clearer, for example:

 
 case gamelost:
    lresult = m_inpMgr.lostEvents(m_Event);
     if (lresult == Input_Manager::mainMenu) {
        m_GameState = title;
    } else { 
        // lresult == Input_Manager::nada, nothing to do...
    }
    break; 
 

You can see I used such a mechanism earlier in this post.

There is a reason why I did that. My brick object has a property called "alive". I only want to draw bricks that are marked alive = true. Whenever a ball hits a brick that is breakable the bricks value is changed to alive = false. So during my for loop I have to check each brick's status. Now if you have a better solution to this problem please let me know how I can decouple the brick drawing from the brick type.

One way is to have two classes. One is just the Brick data, and another knows how to draw bricks:

class Brick {
public:
     bool isAlive() const;
     BrickType getType() const;
// ...
};
 
class BrickRenderer {
public:
    BrickRenderer(TextureHandle red, TextureHandle blue);
 
    // Note: Bricks are value objects!
    void render(const std::vector<Brick> &bricks, Window &window);
private:
    Sprite sprite;
    Texture red;
    Texture blue;
};
 
void BrickRenderer::render(const std::vector<Brick> &bricks) {
    for(auto i = bricks.cbegin() ; i != bricks.cend() ; ++i) {
        Brick &brick = bricks[i];
        if(brick.isAlive()) {
             switch(brick.getType()) {
             case BrickType_Red:
                 sprite.setTexture(red);
                 break;
             case BrickType_Blue:
                 sprite.setTexture(blue);
                 break;
             default:
                 assert(false && "No such brick type!");
                 break;
             }
             sprite.setPosition(brick.getPosition());
             window.draw(sprite);
        }
    }
}

It might be possible to render a texture directly to the screen at a given co-ordinate, but I haven't used SFML so the above is psuedo-code.

Of course, the other convenience functionality that Sprite provides might make it worthwhile to keep the per-Brick sprite. It isn't a huge deal here, but the basic idea can have a lot of value when the number of objects gets very high (e.g. particle systems).

 

This function is probably the ugliest part of my game. I had a lot of problems with how I should go about handling the collision with the bricks, and my limited math knowledge only made it worse. The majority of the if else blocks are determining the various directions of the ball hitting the brick and reacting differently to where the ball hits the brick. I just couldn't come up with a better solution due to my limited math knowledge. You're right it could have been easier on the eyes if I broke it down into different functions but this was one of the last things I had to do to finish my game and I just hacked together this code, as I was losing motivation on this project fast (I wanted to move on to another project), but I forced myself to finish this game.

I spent at least three or four days on just this function alone. I am considering using Box2D to handle the physics and collision for my next projects, or I'll have to take the time and learn the math. Coming into gamedev I had no idea the amount of stuff that goes into making even the smallest of games, there is so much more that I still need to learn that it sometimes feels too overwhelming, which will probably make me go the easier route and use Box2D .

That is fair enough.

What would be a better way to handle the input?

There are a number of solutions. It is nice to decouple input detection from the input "response". A simple structure which has served me well for small games, create a separate class for each game state with a basic interface for interacting with a given state:

class State {
     virtual ~State() {}
 
     virtual void render(/* ... */) = 0;
     virtual void update(/* ... */) = 0;
     virtual void handleEvent(const Event &event) = 0;
};

With such a structure, your top level object will have a pointer to the active state, and can poll for events, passing them to the active state. You probably no longer need an "Input Manager" with such an approach.

The loop is separate. It appears to be inside the previous loop because the indentation is messed up. ...

My bad! I was confused that it worked at all, but I didn't think about it too much, I figured that your data made it appear to work.

ok so I did a little research on my own and checked my game of how the objects were being destroyed and it is fine. However, you were 100% correct, I should be using a virtual destructor in this case nonetheless.

Yes, it is just a common C++ rule to say that any class which has a virtual function must also have a virtual destructor. It is a safety net that catches any cases where an object is destroyed through a pointer to a parent type.

Congratulations on completing your game! Completing any game is no small task.

Here are some things I noticed when reviewing your code;

• By placing the projects own header files in stdafx.h, you are causing the entire game to be recompiled when any header changes. This might be manageable in smaller projects, but it will not scale as your project grows.
  

   

   

   



• As the automatically generated comments in stdafx.h state, the file should generally be used for external headers (e.g. SFML). There are "internal" headers that it may make sense to include here, such as commonly included utility headers (random.h might be a good candidate).
  

   

   

   



• You should not use preprocessor defines for compile time constants. Use the language feature:
  

   

   

  const int SCREENWIDTH = 832;
  const int SCREENHEIGHT = 600;
  const int PADDLEYPOS = 550;
  const int MAXPADDLESPEED = 400; 

   

   



• Your game class has a a number helper functions, for example setTextures(). These should be "private", not "public". Likewise, try to design your program so you do not have public fields. The bricks and window members should be private. The event member can probably be a local variable to your event handling code.
  

   

   

   



• Consider using an enumeration for the brick types, rather than strings. A typo in a string literal will not be caught at compile time.
  

   

   

   



• You seem to have a lot of static variables. I don't think you really need any of them. It should be simple to convert them to members of the Game class. There may be better solutions.
  

   

   

   



• Boolean functions such as Game::isExiting can be written to return the expression, rather than write a full if/else statement:
  

   

   

   bool Game::isExiting(){
      return m_GameState == exiting;
  } 

   

   



• If statements ending in a semicolon is highly unusual and normally sets alarm bells ringing for most programmers. Consider at least moving the semicolon to the next line and adding a comment saying "intentionally blank" or something. Better still is to avoid writing if conditions that require no logic.
  

   

   

  Instead of:

   

   case gamelost:
      lresult = m_inpMgr.lostEvents(m_Event);
      if (lresult == Input_Manager::nada); 
      else if (lresult == Input_Manager::mainMenu)
          m_GameState = title;
      break; 

   

  Consider omitting the "nada" branch:

   

   case gamelost:
      lresult = m_inpMgr.lostEvents(m_Event);
      if (lresult == Input_Manager::mainMenu)
          m_GameState = title;
      break; 

   

   



• Some parts of your Game class are complicated by trying to handle each case in one large if/else block. Moving sub-conditions into sub-blocks makes it much easier to read and understand:
  

   

   

   if (gresult == Input_Manager::prev)
      m_GameState = title;
  else if (gresult == Input_Manager::paused)
      m_GameState = paused;
  else if (gresult == Input_Manager::idle) {
      if(m_Paddle.livesLeft() < 0) {
          m_GameState = gamelost;
      } else if(isLevelWon()) {
          m_GameState = gamewon;
      }
  } 

   

   



• If you decouple the brick drawing from the brick type, you could probably have a vector of values rather than smart pointers.
  

   

   

   



• Your Game_Object class has a virtual member function, it's destructor should also be virtual. Alternatively, consider removing the virtual initialise function, instead require all game objects to be correctly initialised during construction. The base class should require the texture be passed through it's constructor, it should not be necessary to then allow clients to change the texture afterwards.
  

   

   

   



• Sometimes code can be easier to read if you pull a common expression into a local variable. Compare:
  

   

   

  // Original
  sf::Vector2f Game_Object::getCenter(){
      return sf::Vector2f(m_Sprite.getGlobalBounds().left + m_Sprite.getGlobalBounds().width / 2, m_Sprite.getGlobalBounds().top + m_Sprite.getGlobalBounds().height / 2);
  }
  
  // Local variable 
  sf::Vector2f Game_Object::getCenter(){
      sf::FloatRect bounds = m_Sprite.getGlobalBounds();
      return sf::Vector2f(bounds.left + bounds.width / 2, bounds.top + bounds.height / 2);
  } 

   

   



• Your coding convention is inconsistently applied. For example, Brick::alive and Ball::isBallMoving should start with "m_" to match the other members.
  

   

   

   



• The logic in Ball::handleCollisions is very complex and appears to contain much duplicate code. Consider splitting this into multiple functions, one of which determines if any collision occurred, another which calculates which way to bounce, and another one which handles the red/blue brick distinctions.
  

   

   

   



• Your input manager is tightly coupled to the specifics of the game states. In one instance, it requires detailed knowledge of where the buttons are and their dimensions. This is exacerbated by the fact that magic numbers are used here, rather than named constants.
  

   

   

   



• Your level loading code computes the positions for each brick as it loads individual bricks. This could probably be moved into a separate loop that is run after the individual bricks are instantiated. In fact, your current code contains a bug because the "count" variable in initialised at the start of the function, but it gets incremented many times as the two loops run. "yPos", and to a lesser extend "xPos", 
  

   

   

   



• Your level loading code does not handle errors very well. Ideally, your code should be able to report to the user if the file is missing, unreadable or corrupt. At the moment, your code prints a log message in the first case, but it appears that the program would still try to run without any bricks. If the file itself is corrupt, then you'll get very strange results. 
  

   

   

   



• There is no need to declare and define empty destructors, unless you wish to mark them as "virtual", or perhaps to show that the class obeys the rule of three but for some reason the destructor ends up being empty (in that case I would also consider adding a comment to this effect).
  

   

   

   



• Speaking of the rule of three, in larger projects consider marking classes as noncopyable where they cannot or should not have value semantics.
  

   

   

   

One solution is for the objects to mark themselves as "dead", perhaps using a flag or adding themselves (e.g. index or pointer) to a separate "dead list". The master loop can clear dead objects at a convenient time.

Looks like a good start.

The main thing I would start with is making some of the operations exception safe. For example, instead of deleting the existing elements first during the assignment operator, create the copy first. This way an exception during allocation won't destroy the existing object. To avoid leaking, an exception during copying the elements should deallocate this array. Similar care would need to be taken when reserving, etc.

Your Reserve() has the condition newCapacity < size twice, one of which is an early return. The other case can never occur.

I would consider using assertions instead of/in addition to exceptions for operator[].

Calling the assignment operator from the copy constructor is worrying. While it appears to be correct at the moment, you aren't initialising all the members in the copy constructor, so some alternative implementations could cause bugs. One consideration is the common idiom of implementing a swap() function (which you are considering doing anyway), and implementing the assignment operator in terms of a copy and a swap:

 
DynamicArray &operator=(DynamicArray copy) {
    Swap(*this, copy);
    return *this;
}

This implementation can achieve the same exception safety as I described in the first paragraph, provided care is taken in the copy constructor not to leak memory.

Another thing to consider would be not requiring that elements between the size and capacity are initialised. For heavier objects combined with doubling the capacity, you can end up initialising quite a number of elements that may never be accessed. If the object's constructor/destructor do things such as count the number of live objects, the client code might experience unexpected results.

If you haven't already, consider writing a suite of tests. In particular, try to test some of the edge cases mentioned, such as an element type with a constructor/copy constructor that throws, or constantly inserting until there is a std::bad_alloc, ensuring that the original object is intact after such circumstances.

Well, I cannot see your current code.

At the very least, add logging when an error occurs using an external API. You should not need to use a debugger to diagnose such errors. When a connection fails on another user's machine, they may not have a debugger or the skills required to effectively use one. Something not unlike this:

int result = SomeLib_DoStuff(/* ... */);
if(result != SOME_LIB_STUFF_DONE) {
     std::stringstream error;
     error << "DoStuff returned failure code: " << result << ", error message: " << SomeLib_GetError();
     log(error.str());
     return false;
}

Functors? Is that some C++11 construct?

No, it is not new, but in C++11, there is a new syntactic sugar available for functors:

 
std::stable_sort(m_vInstances.begin(), m_vInstances.end(), [](const RenderInstance* pLeft, const RenderInstance* pRight) {
    return pLeft->sortKey.bits > pRight->sortKey.bits; 
});

Upon adding that error checking method to my other networking application using similar code, it also caused my server to no longer receive updates on the client's position, so I have removed that. Does anyone know why this isn't working, its really starting to bug me :/

Step 1: error checking. Step 2: error handling/logging. For example, the documentation for WSAStartup says:

 

Return value

If successful, the WSAStartup function returns zero. Otherwise, it returns one of the error codes listed below.

The WSAStartup function directly returns the extended error code in the return value for this function. A call to theWSAGetLastError function is not needed and should not be used.

 

<list of error codes omitted>

So WSAStartup is not returning false, it is returning its "success" value, which you are misinterpreting as failure. It is important to thoroughly read the documentation for the functions you are using about what kind of error codes can be returned.

You should at the very least write a log message somewhere when a fatal error occurs, and preferably provide a notification to the user rather than crashing to desktop. Most APIs have a way to get a human (or at least, geek) readable error description, which will help enormously in diagnosing such failures when they happen to a random user on the internet, who might want to play your game but might not be particularly technical.

A few things still need some work:

• You aren't checking for errors creating the socket in Application::Init_Winsock(). In addition, you only perform partial cleanup were something to go wrong in the middle of this function.

   

• Your recvfrom() call should check that the number of bytes received was (at least) the number that is expected. It might not be particularly likely, but you could collide with an existing port number, and another application may attempt to send a message to the port you are listening on (for UDP in particular, you can easily pick up broadcasted discovery messages on a local network).

   

• You still have not addressed the thread synchronisation problems in your code. A simple fix would be to ensure that all accesses to Application::Player are controlled by a Critical section. There are more sophisticated ways of handling this, particularly if your "Player" variable is currently being accessed all over the place.

   

• Your background thread's error handling isn't very useful. If a receive fails, you silently (i.e. without even printing a log message) return "false". Where does this return value go? Well, the operating system will store the return value, but your code does not appear to have anything listening for the thread unexpectedly dying.

A relatively simple alternative to multi-threading is to use "non-blocking" sockets. This means you can try to receive in your main loop without stalling the game logic until new data arrives. I would recommend this to someone who is still getting to grips with socket programming. Trying to learn and understand multi-threaded programming is significantly harder, and trying to do both at the same time is a recipe for confusion and error.

When you have your code working, you might also consider writing some code to detect and handle the case where the client or server dies or is shut down. Something as simple as checking if no messages have been received for a reasonable period of time, and then assuming the remote peer is "gone" and informing the user.

What are you trying to achieve? In what language? Is this an introspective program, or are you analysing another process?

Hello, thanks a lot for your time and your review what I do bad.

 

I rewrite some part of code, but I cann't rewrite everything.

If I create my own dynamic memory allocation, I should use it or I should use Stack?

Foo *foo = new Foo(/* ... */);
// Use *foo
delete foo;

Foo foo(/* ... */);
// Use foo

That is a great idea, but in some function(when I render text) I must free Surface every frame.

// "Wrapped" font, like the Surface idea... 
class Font : noncopyable
{
public:
    // ...
 
    SDL_Surface *renderText(const std::string &text, /* ... colour ? */);
 
};
 
class RenderedText : noncopyable
{
public:
    // N.b. Font must outlive this instance
    RenderedText(Font &font)
    :
        font(&font),
        surface(nullptr)
    {
    }
 
    ~RenderedText()
    {
        // Note: safe to pass nullptr to SDL_FreeSurface
        // http://sdl.beuc.net/sdl.wiki/SDL_FreeSurface
        SDL_FreeSurface(surface);
    }
 
    void render(SDL_Surface *destination, int x, int y)
    {
        if(surface)
        {
            SDL_BlitSurface(/* ... */);
        }
    }
 
    void setMessage(std::string newMessage)
    {
        if(newMessage != currentMessage)
        {
            SDL_Surface *temp = font->renderText(newMessage);
            if(!temp)
            {
                // TODO: handle error?
            }
            std::swap(surface, temp);
            std::swap(newMessage, currentMessage);
            SDL_FreeSurface(temp);
        }
    }
 
private:
    Font *font;
    std::string currentMessage;
    SDL_Surface *surface;
};

What I forget clean up in BackGround?

Do you mean that I should use there vector?

I'm loading this data from file as you said. But I must have 10 Files with 5 values. And it is cumbersome. If I create one file with all values I don't know, how I can move in this file. I know functions seekg but I don't know how I can go on line 2, or on line 5..

struct Level
{
    float speedx;
    float speedy;
    // ...
};
 
std::istream &operator>>(std::istream &stream, Level &level)
{
    return stream >> level.speedx >> level.speedy /* ... */;
}
 
std::vector<Level> loadLevels(std::istream &stream)
{
    // Optional:
    // Early out if stream isn't good to read from initially
 
    std::vector<Level> result;
 
    while(stream >> std::ws)
    {
        Level level;
        if(stream >> level)
        {
            result.push_back(level);
        }
    }
 
    if(!stream.eof())
    {
        // Level file might be corrupt?
        // Might be worth logging.
       std::clog << "Level data corrupt after " << result.size() << " levels!\n";
    }
 
    if(result.empty())
    {
        // Maybe complain and exit if no levels have been loaded...
        // Could be caller's reponsibility
        std::cerr << "No levels?\n";
    }
    return result;
}

1 2.3
2 3.4
3 4.5
4 5.6

I completely change GameRun and Saves class and I split long functions into smaller helper functions. I hope that it is better.

I will not go change Failure handling and loading font once, because game is complete(I hope).

Second game will better 

The errors that SiCrane and I mentioned are rather elementary C++ errors. If you cannot solve them yet, I would recommend you step back from trying to integrate Physfs and Python into a complex game for the time being. Instead, start with a simpler program, for example make some console programs until you are thoroughly familiar with the basic language.