C++

Forum

 
Strange program behaviour

adam2016 (1529)
Hey guys I am messing around with SDL I am just getting used to some very simple animation with 2d graphics,

anyway for some odd reason I would say about 50% of the time I run the program no characters appear on the screen and on the other executions of the program the players will appear on the screen,

anybody possibly have any idea why sometimes the players are not being rendered to the screen?

thanks

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
 
  #include <iostream>
#include <vector>
#include <SDL2/SDL.h>
#include <SDL2/SDL_image.h>

using namespace std;

vector<SDL_Texture*> players;
vector<SDL_Rect*> destRecs;
SDL_Window *window;
SDL_Renderer *renderer;
SDL_Texture *firstPic;
SDL_Surface *AdamTmp;
SDL_Texture *Adam;
SDL_Texture *Jim;
SDL_Surface *JimTemp;
SDL_Rect* srcRec;
SDL_Rect* destRec;
SDL_Rect* JimRecSrc;
SDL_Rect* JimRecDst;
SDL_Event event;
const int SCREEN_WIDTH = 670;
const int SCREEN_HEIGHT = 600;
bool up,down;
int PlayerCount = 0;

bool gameInit(){

   if(SDL_Init(SDL_INIT_EVERYTHING) < 0){

     cout << "error" << endl;
     return false;
   }
   if(IMG_Init(IMG_INIT_PNG) < 0){

     cout << "error" << endl;
     return false;
   }

    window = SDL_CreateWindow("The GTG game",SDL_WINDOWPOS_CENTERED,SDL_WINDOWPOS_CENTERED,SCREEN_WIDTH,SCREEN_HEIGHT,SDL_WINDOW_RESIZABLE);
    renderer = SDL_CreateRenderer(window,-1,0);
    AdamTmp = IMG_Load("S.png");
    JimTemp = IMG_Load("M.png");
    Adam = SDL_CreateTextureFromSurface(renderer,AdamTmp);
    Jim = SDL_CreateTextureFromSurface(renderer,JimTemp);
    SDL_SetRenderDrawColor(renderer,255,255,255,255);
    destRec = new SDL_Rect;
    JimRecDst = new SDL_Rect;

    players.push_back(Adam);
    destRecs.push_back(destRec);
    destRecs.at(0)->h = 120;
    destRecs.at(0)->w = 120;


    bool down = true;
    bool up = false;


    return true;
}

void cleanUp(){

   SDL_DestroyRenderer(renderer);
   SDL_DestroyWindow(window);
   IMG_Quit();
   SDL_Quit();
}


void render(){

  SDL_RenderClear(renderer);

  for(int i = 0; i < players.size(); i++){
  SDL_RenderCopy(renderer,players.at(i),NULL,destRecs.at(i));
  }

  SDL_RenderPresent(renderer);

}

void updateScreen(){

   if(destRecs.at(0)->y > SCREEN_HEIGHT-destRecs.at(0)->h){

       up = true;
       down = false;
   }



   if(destRecs.at(0)->y < 1){

       up = false;
       down = true;
   }

   if(down){

   destRecs.at(0)->y += 5;
   SDL_Delay(100);


   }
   if(up){

     destRecs.at(0)->y -= 5;
     SDL_Delay(100);
  }
}

// add a new player to game and vector players

void createPlayer(){

   if(PlayerCount == 1){

      players.push_back(Jim);
      destRecs.push_back(JimRecDst);
      destRecs.at(1)->h = 120;
      destRecs.at(1)->w = 120;
      destRecs.at(1)->x = 125;
   }
}

int SDL_main(int argc,char* argv[])
{

   bool quit = false;

   cout << "to add a new character press the down key" << endl;

   gameInit();

   while(!quit){

    SDL_PollEvent(&event);
    if(event.type == SDL_QUIT){

        quit = true;
    }
    if(event.type == SDL_KEYDOWN){

        PlayerCount++;
        createPlayer();
    }

    updateScreen();
    render();
   }

}
Ganado (6814)
Not sure about your actual issue of characters not appearing, but note that line 56 and 57 are making local variables, they're not modifying your global variables.

For your actual issue, my guess (emphasis on guess) is that you have uninitialized structs or something. For example, where is the y coordinate set for your player or destRec?
adam2016 (1529)
thanks Ganado very good point,not sure how I made that mistake even the newer code I fixed that without even realising,

also it may just be that.I will try explicitly setting the y values.
jlb (4973)
You should also strive to get rid of all those global variables and try to normal non-pointer instances when possible.
mbozzi (3934)
SDL's resources are all opaque pointers. This means you can use std::unique_ptr to manage them. Here's coarsely what I use for this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
 
/** \brief Generic deleter functor for SDL resources.
 * For use with std::unique_ptr.
 */
struct sdl_deleter {
  // Generate "free" calls.
# define FREE_ME(type, fn_name) inline void operator()(type* const thing)  \
    const noexcept { if (thing) SDL_##fn_name(thing); }
  FREE_ME(SDL_RWops, FreeRW)             FREE_ME(SDL_cond, DestroyCond)
  FREE_ME(SDL_Cursor, FreeCursor)        FREE_ME(SDL_PixelFormat, FreeFormat)
  FREE_ME(SDL_mutex, DestroyMutex)       FREE_ME(SDL_Palette, FreePalette)
  FREE_ME(SDL_Renderer, DestroyRenderer) FREE_ME(SDL_sem, DestroySemaphore)
  FREE_ME(SDL_Surface, FreeSurface)      FREE_ME(SDL_Texture, DestroyTexture)
  FREE_ME(Uint8, FreeWAV)                FREE_ME(SDL_Window, DestroyWindow)
#undef FREE_ME
};

template <typename Resource>
using sdl_unique_ptr = std::unique_ptr<Resource, sdl_deleter>;

using rwops_ptr         = sdl_unique_ptr<SDL_RWops>;
using cursor_ptr        = sdl_unique_ptr<SDL_Cursor>;
using mutex_ptr         = sdl_unique_ptr<SDL_mutex>;
using renderer_ptr      = sdl_unique_ptr<SDL_Renderer>;
using surface_ptr       = sdl_unique_ptr<SDL_Surface>;
using wav_ptr           = sdl_unique_ptr<Uint8>;
using cond_ptr          = sdl_unique_ptr<SDL_cond>;
using pixel_format_ptr  = sdl_unique_ptr<SDL_PixelFormat>;
using palette_ptr       = sdl_unique_ptr<SDL_Palette>;
using sem_ptr           = sdl_unique_ptr<SDL_sem>;
using texture_ptr       = sdl_unique_ptr<SDL_Texture>;
using window_ptr        = sdl_unique_ptr<SDL_Window>;
adam2016 (1529)
Thanks Mbozzi,just a follow up

I hope this doesn't sound mundane,but why should you use std:unique_ptr over regular pointers? is there any benefit or performance benefits?

also in what way are SDL resources opaque? does it mean that you can not call any of their members functions or variables?

thanks :)
mbozzi (3934)
I hope this doesn't sound mundane,but why should you use std:unique_ptr over regular pointers? is there any benefit or performance benefits?


No, it's not mundane. It's a great question, but I don't think I can write a complete explanation right now.

A resource is something that there's a finite number of. When you allocate a resource, it's (often) yours exclusively until you give it back. Some examples of resources include dynamic memory or open files. You have to release these things (say delete, call fclose()) when you're done with them. SDL provides resources too - Windows, Renderers, Surfaces, Textures, etc., all have their own SDL_CreateX() and SDL_ReleaseX() pairs.

The problem is that it is often difficult to keep track of where and when to give a resource back. Each resource must be released exactly once, in the right order, and only when the program is finished with it.

This is a plentiful source of errors, but the problem can be mitigated by introducing a concept of ownership. Usually there's exactly one system that knows when everyone is done with a resource and can safely release it - this system is the owner of a resource, and this is where unique_ptr steps in: it represents this unique ownership, and is responsible for releasing the resource automatically and in the proper order, and at the right time.

There are two primary benefits over raw pointers:
1. unique_ptr makes it clear who owns the resource. This makes it challenging to release stuff twice or to otherwise lose track of it.
2. unique_ptr will automatically release the resource it is managing, in the correct order, saving you from the burden of doing it yourself, which is otherwise difficult and exceptionally error-prone.

A correctly-implemented unique_ptr has no overhead over a normal pointer, neither in time nor space.
This is true as long as the deleter is not stateful. Most deleters are not, the one above isn't.

Also in what way are SDL resources opaque? Does it mean that you can not call any of their members functions or variables?

Partially. Consider SDL_Renderer. You'll only ever get pointers back from the library, and SDL_Renderer itself is (most likely) an incomplete type. You can only do stuff with the renderer by passing the pointer to it to library functions. This is done so that your code will not depend on the insides of SDL.

This idea promotes flexibility on the library side - the guts of the renderer can change without affecting your code at all, even if your code has already been compiled (it promotes ABI compatibility). A related C++ pattern is called the pointer-to-implementation idiom, PIMPL:
https://en.cppreference.com/w/cpp/language/pimpl

Last edited on
Repeater (3046)
Since we're on the specific subject of unique_ptr and SDL, this is an opportunity for an excellent discussion...

We can see in the naive implementation above from adam2016 (where here 'naive' is not a comment about adam2016 ; it's a common work used by programmers to describe a very simple implementation of something that doesn't really take advantage of the language) that all the SDL resources get initialised with a specific function call (for example, SDL_CreateRenderer ) and are thence accessible with raw pointers.

Many of these resources should be released not with a simple delete, but with a call to a specific SDL function which presumably does some tidying up (for example, SDL_DestroyRenderer ).

We can see in the code above that there is a cleanUp() function doing this; it calls the right deinitialisation function on the right pointers in the right order. If this code becomes more complicated, this will get more complicated. What if sometimes, some resources aren't always initialised? What if we make a mistake here where there are lots of resources and forget some, or do one twice, or get them in the wrong order? What if the order isn't fixed? What if the code throws an exception, and we skip over calling the cleanup? What if we remember to call the cleanup, but then how do we then know which of these resources even got initialised? Welcome to wide word of C-style problems with manual cleanup!

This sounds awfully familiar, doesn't it? A function used to initialise an object, and another function to be called when its time to tidy away that object? Goodness me - we're talking about a constructor and a destructor !

Now, we don't need to talk about what he pitfalls of having to manually initialise and manually deinitialise objects are; it's basically the reason why constructors and destructors exist. RAII is a big advantage of C++.

So here's one way that C++ programmers have traditionally dealt with having this sort of C-style manual construction and destruction of something - just wrapping it inside a C++ class that will handle that for us.

1
2
3
4
5
6
7
8
9
10
 
class SDLRendererWrapped
{
    public:
    SDL_Renderer* pRenderer;
    SDLRendererWrapped(SDL_Window* window, int index, Uint32 flags)
    {
       pRenderer = SDL_CreateRenderer (window, index, flags);
    }
    ~SDLRendererWrapped() {SDL_DestroyRenderer(pRenderer};
}


This now allows us to create a SDLRendererWrapped object just like we would have created a SDL_Renderer, and in this simple implementation we can easily reach the actual pointer to the actual renderer that we want. We could also overload various operators to make it behave more like an exact drop-in replacement. We have achieved memory safety, and we are guaranteed that when this object goes out of scope the rendered will be properly tided up, and so on. All the problems of manual memory management and manual deinitialisation of objects have been dealt with.

However, in C++11, we got smart pointers. A unique_ptr will, when it goes out of scope, call delete on the pointer it holds for us. If all this renderer object required was a call to delete when it was time to destroy it, this would be simple and easy. However, this rendered object requires more than that. It requires a call to SDL_DestroyRenderer when it's being deleted.

C++11 smart pointers allow us to do this; they allow us to tell the unique_ptr that calling delete isn't what's required. That instead, it has to call this other function. This is what mbozzi is doing in his code example above.

mbozzi has done a quick and simple example using some preprocessor magic. Here's a page discussing exactly this example with SDL, without using any preprocessor magic, and covering shared_ptr as well; https://swarminglogic.com/jotting/2015_05_smartwrappers

I won't go into the details. The important thing is to understand, and to add to your "how to think and design in C++" knowledge is thus two-fold:

1) When objects require specific initialisation and destruction, such as specific function calls, they are just begging to be wrapped inside a C++ object so that we get the benefits of RAII (i.e. memory safety, correct order of destruction, simpler code, etc.)

2) If that init and destruction is simple enough (as it is in this SDL case), that C++ object can be a simple smart pointer that we inform of the function that needs to be called upon destruction.

We can always look up the syntax for how to do this; what we can't look up is this extra knowledge that this is possible and desirable. The knowledge that this is possible and a good thing needs to go inside the programmer's head now so that the programmer designs code to use it where appropriate. We can always look up the syntax when you need it; this knowledge that it's possible has to go inside heads so it will be used in the future. Don't focus on the syntax here; focus on internalising what's possible, and you'll be able to look up the syntax later (and one day you'll realise you've memorised it).

When people say "think in C++" this is the kind of thing they mean.
Last edited on
adam2016 (1529)
thanks guys,

really interesting reads,true I have actually learned about RAII resource acquisition is initialization, many programmers don't like it's name hard to remember lol,

but yes when reading Bjarnes practices and principles he said always free resources in the destructor and initialise resources in the constructor something that always seems to slip my mind.

and that is a great idea about creating a wrapper class for the SDL resources but this would create a lot more code isn't that a bad thing? (but to be honest more code is always better than your program throwing an exception)

should I choose to use unique_ptrs over RAII in general?

Repeater (3046)
and that is a great idea about creating a wrapper class for the SDL resources but this would create a lot more code isn't that a bad thing?


More code isn't inherently a bad thing. More complexity is a bad thing. More opportunity to make mistakes is a bad thing.

All else being equal, less code is better than more code, but this isn't equal; this is an extra class consisting of four simple lines plus some spacing, in exchange for which you get so much back. So much that makes your code simpler and safer.
adam2016 (1529)
1
2
3
4
5
6
7
8
9
10
11
12
13
 

struct sdl_deleter {
  // Generate "free" calls.
# define FREE_ME(type, fn_name) inline void operator()(type* const thing)  \
    const noexcept { if (thing) SDL_##fn_name(thing); }
  FREE_ME(SDL_RWops, FreeRW)             FREE_ME(SDL_cond, DestroyCond)
  FREE_ME(SDL_Cursor, FreeCursor)        FREE_ME(SDL_PixelFormat, FreeFormat)
  FREE_ME(SDL_mutex, DestroyMutex)       FREE_ME(SDL_Palette, FreePalette)
  FREE_ME(SDL_Renderer, DestroyRenderer) FREE_ME(SDL_sem, DestroySemaphore)
  FREE_ME(SDL_Surface, FreeSurface)      FREE_ME(SDL_Texture, DestroyTexture)
  FREE_ME(Uint8, FreeWAV)                FREE_ME(SDL_Window, DestroyWindow)
#undef FREE_ME
};




this trick is also pretty neat,using the pre-processor to sum up the code,

a few questions about the define directive while I'm here,I've used it in the past but not much. I know it's more of a C thing(as far as I know could be horribly wrong) here Mbozzi is creating a struct sdl_deleter

#define FREE_ME(type, fn_name) inline void operator()(type* const thing) \

also just wondering SDL_##fn_name(thing); why the ## ater SDL_ and before fn_name(thing); ?

first I never knew you could for example say type declaring any object,I thought had to be a known object or pre defined type such as int,char,double etc, obviously it is legal so it's pretty similar to a template in a way?

also why the need for the \ slash after the function definition?

and the second thing which takes me by surprise is 

1
2
3
4
5
6
7
8
 

FREE_ME(SDL_RWops, FreeRW)             FREE_ME(SDL_cond, DestroyCond)
  FREE_ME(SDL_Cursor, FreeCursor)        FREE_ME(SDL_PixelFormat, FreeFormat)
  FREE_ME(SDL_mutex, DestroyMutex)       FREE_ME(SDL_Palette, FreePalette)
  FREE_ME(SDL_Renderer, DestroyRenderer) FREE_ME(SDL_sem, DestroySemaphore)
  FREE_ME(SDL_Surface, FreeSurface)      FREE_ME(SDL_Texture, DestroyTexture)
  FREE_ME(Uint8, FreeWAV)                FREE_ME(SDL_Window, DestroyWindow)


why is there no semicolon at the end of any of the definitions of FREE_ME()?

thanks :)

Last edited on
Repeater (3046)
They're all class functions, inside the class sdl_deleter. They come out something like this:

1
2
3
4
5
6
7
 
struct sdl_deleter 
{
 inline void operator()(SDL_RWops* const thing) const noexcept { if (thing) SDL_FreeRW(thing); }  
 inline void operator()(SDL_cond* const thing) const noexcept { if (thing) SDL_DestroyCond(thing); }

 // etc etc
}


There's no semi-colon at the end because the end of the class function is marked with a }, like in every class.
mbozzi (3934)
FREE_ME above is a function-like macro. That's something to search for if you're curious.
Last edited on
mbozzi (3934)
I found some discussion of this snippet from the beginning of 2017:
http://www.cplusplus.com/forum/general/205799/#msg974265
adam2016 (1529)
thanks guys
Topic archived. No new replies allowed.