Allegro.cc - Online Community

Allegro.cc Forums » Programming Questions » Accessing a bool of an instance variable gives the wrong value

This thread is locked; no one can reply to it. rss feed Print
Accessing a bool of an instance variable gives the wrong value
cmasupra
Member #12,969
June 2011

I am working on a level editor for my game, but I have run into a problem that I just can't figure out. I can get the correct value of a variable from within the same class, but not from my main loop (which is where an instance of the class is created).

In the SaveMapButton class, the Draw function will draw the correct animation depending on the 'clicked' boolean variable, which tells me that 'clicked' is getting set correctly by the Update function.

However, when I check if 'clicked' is true in my main game loop, it always returns false, even if the button is clicked.

My main game loop in main.cpp:

#SelectExpand
1while (!quit) 2{ 3 ALLEGRO_EVENT ev; 4 al_wait_for_event(event_queue, &ev); 5 if (ev.type == ALLEGRO_EVENT_TIMER) 6 { 7 redraw = true; 8 } 9 else if (ev.type == ALLEGRO_EVENT_DISPLAY_CLOSE) 10 { 11 quit = true; 12 } 13 14 if(redraw && al_is_event_queue_empty(event_queue)) 15 { 16 redraw = false; 17 al_clear_to_color(al_map_rgb(100,149,237)); 18 al_get_mouse_state(&mouse); 19 20 // If a toolbar button is clicked 21 if (saveMapB.clicked) // *** This always returns false, even if the button was clicked this 'frame' *** 22 { 23 cout << "SaveMapB clicked!" << endl; // *** This never shows up, so I assume the if statement that this is inside is false *** 24 map.saveLayer(); 25 } 26 27 // Call any Update functions 28 for (unsigned int i = 0; i < buttonList.size(); i++) 29 { 30 buttonList.at(i).Update(mouse); 31 } 32 for (unsigned int i = 0; i < tileListButton.size(); i++) 33 { 34 tileListButton.at(i).Update(mouse, selectedTileIndex); 35 } 36 map.setTiles(selectedTileIndex, mouse); 37 38 // Call any Draw functions 39 for (int i = 0; i <= monitorW; i++) // draw top toolbar background 40 { 41 for (int j = 0; j <= 45; j++) 42 { 43 al_draw_pixel(i, j, al_map_rgb(128, 128, 128)); 44 } 45 } 46 for (unsigned int i = 0; i < buttonList.size(); i++) 47 { 48 buttonList.at(i).Draw(); 49 } 50 for (unsigned int i = 0; i < tileListButton.size(); i++) 51 { 52 tileListButton.at(i).Draw(); 53 } 54 al_set_target_bitmap(mapBMP); 55 al_clear_to_color(al_map_rgb(0, 0, 0)); 56 map.Draw(tileListBMP); 57 al_set_target_backbuffer(display); 58 al_draw_bitmap(mapBMP, 4.5, 50.5, 0); 59 60 al_flip_display(); 61 } 62}

SaveMapButton.cpp:

#SelectExpand
1#include "SaveMapButton.h" 2 3SaveMapButton::SaveMapButton() : Button() 4{ 5 6} 7 8SaveMapButton::SaveMapButton(ALLEGRO_BITMAP *tex, Recta rect) : Button(tex, rect) 9{ 10 //ctor 11} 12 13SaveMapButton::~SaveMapButton() 14{ 15 //dtor 16} 17 18void SaveMapButton::Effect() 19{ 20 21}

Button.cpp's Update function:

#SelectExpand
1void Button::Update(ALLEGRO_MOUSE_STATE mouse) 2{ 3 Recta mouseRect (mouse.x, mouse.y, 1, 1); 4 if (mouseRect.intersects(collisionRect)) 5 { 6 hover = true; 7 } 8 else 9 { 10 hover = false; 11 } 12 13 if (hover && al_mouse_button_down(&mouse, 1)) 14 { 15 clicked = true; 16 } 17 else 18 { 19 clicked = false; 20 } 21}

Button.cpp's Draw function:

#SelectExpand
1void Button::Draw() 2{ 3 if (clicked) // *** It correctly detects whether clicked was true, so this works *** 4 { 5 for (int i = collisionRect.x+1; i <= collisionRect.x+collisionRect.width; i++) 6 { 7 for (int j = collisionRect.y+1; j <= collisionRect.y+collisionRect.height; j++) 8 { 9 al_draw_pixel(i, j, al_map_rgb(200, 200, 200)); 10 } 11 } 12 al_draw_scaled_bitmap(texture, 13 0, 0, collisionRect.width, collisionRect.height, // source dimensions 14 collisionRect.x+2, collisionRect.y+2, collisionRect.width-4, collisionRect.height-4, // destination dimensions 15 0); 16 } 17 else if (hover) 18 { 19 for (int i = collisionRect.x+1; i <= collisionRect.x+collisionRect.width; i++) 20 { 21 for (int j = collisionRect.y+1; j <= collisionRect.y+collisionRect.height; j++) 22 { 23 al_draw_pixel(i, j, al_map_rgb(200, 200, 200)); 24 } 25 } 26 al_draw_bitmap(texture, collisionRect.x, collisionRect.y, 0); 27 } 28 else // not clicked or hovering 29 { 30 al_draw_bitmap(texture, collisionRect.x, collisionRect.y, 0); 31 } 32}

Peter Hull
Member #1,136
March 2001

It looks a bit odd to me that the 'update' stuff is inside the if(redraw...) {} block, could you move it outside?

Pete

cmasupra
Member #12,969
June 2011

Okay. I moved the 'update' stuff to just above the 'if (redraw...)' block. I then tried it with the 'if (saveMapB.clicked)' inside and outside (not in and out at the same time, though) of the 'if (redraw)' block.

The results are the same both ways. The cout statement never shows up.

So now my main game loop in main.cpp looks like this:

#SelectExpand
1while (!quit) 2{ 3 ALLEGRO_EVENT ev; 4 al_wait_for_event(event_queue, &ev); 5 if (ev.type == ALLEGRO_EVENT_TIMER) 6 { 7 redraw = true; 8 } 9 else if (ev.type == ALLEGRO_EVENT_DISPLAY_CLOSE) 10 { 11 quit = true; 12 } 13 14 // Call any Update functions 15 for (unsigned int i = 0; i < buttonList.size(); i++) 16 { 17 buttonList.at(i).Update(mouse); 18 } 19 for (unsigned int i = 0; i < tileListButton.size(); i++) 20 { 21 tileListButton.at(i).Update(mouse, selectedTileIndex); 22 } 23 map.setTiles(selectedTileIndex, mouse); 24 25 // If a toolbar button is clicked 26 if (saveMapB.clicked) 27 { 28 cout << "SaveMapB clicked!" << endl; 29 map.saveLayer(); 30 } 31 32 // If (redraw) 33 if(redraw && al_is_event_queue_empty(event_queue)) 34 { 35 redraw = false; 36 al_clear_to_color(al_map_rgb(100,149,237)); 37 al_get_mouse_state(&mouse); 38 39 // Call any Draw functions 40 for (int i = 0; i <= monitorW; i++) // draw top toolbar background 41 { 42 for (int j = 0; j <= 45; j++) 43 { 44 al_draw_pixel(i, j, al_map_rgb(128, 128, 128)); 45 } 46 } 47 for (unsigned int i = 0; i < buttonList.size(); i++) 48 { 49 buttonList.at(i).Draw(); 50 } 51 for (unsigned int i = 0; i < tileListButton.size(); i++) 52 { 53 tileListButton.at(i).Draw(); 54 } 55 al_set_target_bitmap(mapBMP); 56 al_clear_to_color(al_map_rgb(0, 0, 0)); 57 map.Draw(tileListBMP); 58 al_set_target_backbuffer(display); 59 al_draw_bitmap(mapBMP, 4.5, 50.5, 0); 60 61 al_flip_display(); 62 } 63}

Bruce Perry
Member #270
April 2000

Questions to consider:

  1. Is saveMapB actually in buttonList? (Where you add it, is it possible that you actually add a copy instead?) If it's not in there, then Update() will never be called for it.

  2. Your calls to Update() seem to happen after that check for 'clicked'. This isn't necessarily going to cause noticeable problems, but it does mean your clicks will register one update later. It's worth getting into the habit of noticing things like this, since sometimes they do matter.

  3. I'm sure Allegro has a rectangle drawing function you could use instead of the mega pixel loop. You might have to look under the primitives add-on or some such. Drawing that many pixels manually can't be good for your performance.

Hope that helps :)

--
Bruce "entheh" Perry [ Web site | DUMB | Set Up Us The Bomb !!! | Balls ]
Programming should be fun. That's why I hate C and C++.
The brxybrytl has you.

cmasupra
Member #12,969
June 2011

1. saveMapB is in buttonList. Here's the code where I add it:

ALLEGRO_BITMAP *saveMapImage = al_load_bitmap("saveMapIcon.png");
Recta saveMapRect (72.5, 4.5, al_get_bitmap_width(saveMapImage), al_get_bitmap_height(saveMapImage));
SaveMapButton saveMapB (saveMapImage, saveMapRect);
buttonList.push_back(saveMapB);

saveMapB does the animation from it's 'Draw()' function correctly, so I would assume there is nothing wrong with the saveMapB entry in buttonList. If you still think it might be a copy, let me know.
EDIT: I just ran the program with a cout for buttonList.size(), and it said 3, which is how many there should be (newMapB, openMapB, saveMapB). Just FYI, I haven't started working with newMapB or openMapB yet, but they probably have the same issue.

2. I think you are referring to my first post. In my second post, I have an updated main loop where 'Update()' & 'if(saveMapB.clicked)' are both called outside of the 'if(redraw...)', and 'Update()' is called before the 'if(saveMapB.clicked)'.

3. I made a note to myself to fix that when I went browsing the API, but then I forgot it ::). I'll have to change that to use the Primitives add-on after we get this working (just to avoid any unnecessary changes that could possibly complicate things).

Your number 3 could possibly explain why I am only getting ~47fps with this simple loop, though. I was planning to fix the fps after finishing the level editor, but hopefully that will fix it. I didn't know that drawing that many pixels could hurt performance that badly. Glad you mentioned it ;)

Bruce Perry
Member #270
April 2000

  1. Since you're directly using instances of SaveMapButton (rather than pointers to them), C++ has no choice but to make a copy when you add it to the list. The copy seems to be fully functional, so the call to Draw() will draw what you expect - but when you Update() it, only the copy gets its 'clicked' field set. The variable 'saveMapB' contains the original, whose 'clicked' field is never set. Using pointers could solve your problem.

  2. You're right :)

  3. I find it useful to add comments like "//TODO: change this" to the code in cases like this. Some IDEs (such as Eclipse) will highlight that point in the code, because it's programmed to recognise the code "TODO". (You get used to breaking the rules of English grammar after a while.)

--
Bruce "entheh" Perry [ Web site | DUMB | Set Up Us The Bomb !!! | Balls ]
Programming should be fun. That's why I hate C and C++.
The brxybrytl has you.

cmasupra
Member #12,969
June 2011

How would I make it a pointer? I'm pretty sure I would have to modify these lines:
1)

vector<Button> buttonList;

2)

ALLEGRO_BITMAP *saveMapImage = al_load_bitmap("saveMapIcon.png");
Recta saveMapRect (72.5, 4.5, al_get_bitmap_width(saveMapImage), al_get_bitmap_height(saveMapImage));
SaveMapButton saveMapB (saveMapImage, saveMapRect);
buttonList.push_back(saveMapB);

3)

if (saveMapB.clicked)

In that second set of code, I would also like to make saveMapB pass saveMapRect as a pointer. How could I do that? I imagine I would need to change my SaveMapButton and/or Button class to do that, but I don't know how.

I'm coming from Java, but I had a 1 semester course on C++, so I somewhat understand pointers, but they're still confusing :-/

Edgar Reynaldo
Major Reynaldo
May 2007
avatar

cmasupra said:

How would I make it a pointer? I'm pretty sure I would have to modify these lines:

By using a pointer instead of passing by value?

Button b;
vector<Button*> buttons;
buttons.push_back(&b);

cmasupra said:

In that second set of code, I would also like to make saveMapB pass saveMapRect as a pointer. How could I do that? I imagine I would need to change my SaveMapButton and/or Button class to do that, but I don't know how.

Passing your rectangle by value is really not a problem you need to solve. Passing a pointer to a rectangle doesn't really gain you much, and it could lead to problems if you modify the rectangle it points to while outside of Button class code.

cmasupra said:

I'm coming from Java, but I had a 1 semester course on C++, so I somewhat understand pointers, but they're still confusing

Pointers are simple. They store the address of another object. You access the data they point to by dereferencing them by using the '->' operator.

typedef struct Test {
   int a;
   double b;
   char c;
};

Test test;
Test* t = &test;
t->a = 1;
t->b = 2.0;
t->c = '3';
assert(test.a == 1);
assert(test.b == 2.0);
assert(test.c == '3');

cmasupra
Member #12,969
June 2011

Thanks, you 3. That worked :D

Bruce Perry, using the Primitives add-on did bring my fps back up to 60. Thanks for reminding me of that.

As for pointers, I understand what they do, I just have trouble knowing what symbol to use when (*, &, ., ->). I get really confused when I have to do a pass-by-reference for a function because then I have to put things in the parameters of the function declaration, reference the variables differently, etc.

Bruce Perry
Member #270
April 2000

Suppose you have 'int x;'

To obtain a pointer to that variable, you can write &x. Likewise you can write &someArray[10]. You can use & with anything that you can put on the left of = (I think).

To get back to the item being pointed to (i.e. dereference), you use *x. So you can write *x=2 or (*x)++ to modify the int. (*x++ means *(x++), so be careful. Pointer arithmetic works in increments of the size of the data type, so you can use it to walk arrays if you have a pointer to one of the elements.)

The -> operator can generally be thought of as syntactic sugar, so x->y is a synonym for (*x).y. While this is accurate for C, I think in C++ you can overload the -> operator separately; but it's still useful conceptually.

To declare a pointer variable (or anything more complicated than a primitive/struct/class type), you use the same syntax you would use to dereference your way back to the primitive/struct/class. So if you have a variable x and you want to use *x to get back to the int, you write "int *x;". There are more complicated constructions such as int (*x)[10] or void (*funcptr)() if you ever want them.

When you need to specify a pointer type without a name (such as when parameterising that vector type), you simply do it without the name - e.g. <int *> or <int (*)[10]> or <void (*)()>.

Coming from Java, you'll be used to the garbage collector and the safety of knowing that everything is safely on the heap until you no longer hold any references to it. In C++ you have to be much more careful, and Edgar's snippet above is dangerous. If you make something inside a method without using a pointer, then it will be on the stack and will cease to exist when you return from that method. Don't take a pointer to such an item and store the pointer permanently.

To instantiate an object on the heap and have a pointer to it, you can use 'new', and then it will last - but unlike in Java, it won't be garbage-collected, so you'll have to write 'delete thePointerVariable;' later - unless it's a one-shot allocation that you're happy for the OS to clean up for you on exit.

There is one final feature in C++ that's worth knowing about. If you use & in a declaration (as opposed to an expression), then it has a completely different meaning. So for example if you write "int x;" and then "int &y=x;", then y is a reference. It's like a pointer, but you can only initialise it once, and the initial 'take a pointer' and subsequent 'dereference the pointer' operations all happen automatically. Any attempt to set 'y' (again) will be interpreted as "set the thing that y points to", which in this case is x.

--
Bruce "entheh" Perry [ Web site | DUMB | Set Up Us The Bomb !!! | Balls ]
Programming should be fun. That's why I hate C and C++.
The brxybrytl has you.

cmasupra
Member #12,969
June 2011

Wow. That helped a lot. There are just 2 things I didn't understand in there:

So if you have a variable x and you want to use *x to get back to the int, you write "int *x;". There are more complicated constructions such as int (*x)[10] or void (*funcptr)() if you ever want them.

I have no idea what you are saying here.

Quote:

...unless it's a one-shot allocation that you're happy for the OS to clean up for you on exit.

What is a one-shot allocation?

Bruce Perry
Member #270
April 2000

If you're trying to declare a variable x which is a pointer to an int, then the way you do it is to think about how you would use that variable. In order to get the int that it points to, you would write *x. Therefore you also write *x to indicate what x is, i.e. to indicate the fact that it is a pointer to something. (You write 'int *x' to indicate that it's a pointer to an int.)

int (*x)[10] is a pointer to a 10-element array of ints. It's useful if you actually think of it as a pointer to a 2D array, perhaps 10x10 or 20x10. It points to the first 10-element sub-array. You can then write (*x)[3] to get the first sub-array, fourth element; or (*(x+5))[3], etc. I didn't mention before that x[5] and *(x+5) are interchangeable in many cases (although again I think you can overload them separately in C++). Given this fact, you can then write x[5][3].

void (*funcptr)() is a pointer to a function that takes no arguments and returns nothing. First you have to dereference the pointer to get the function, then you add the brackets in order to call it. That said, it's more of a convention than a real phenomenon, and C and C++ both let you take a pointer to the function with or without the &, and call it with or without the *.

Doing a 'new' and getting an object involves "allocating" memory from the heap. By one-shot, I mean it's something you do once in your whole program and never do again. You can get away without explicitly deleting it (freeing the memory) because it will happen anyway when you exit. If you're allocating things repeatedly, then you have to free them or else you'll run out. That's how memory leaks happen and is probably the reason so much software becomes slow if it runs too long.

--
Bruce "entheh" Perry [ Web site | DUMB | Set Up Us The Bomb !!! | Balls ]
Programming should be fun. That's why I hate C and C++.
The brxybrytl has you.

Neil Walker
Member #210
April 2000
avatar

^ best not mention reference variables then ;)

Neil.
MAME Cabinet Blog / AXL LIBRARY (a games framework) / AXL Documentation and Tutorial

wii:0356-1384-6687-2022, kart:3308-4806-6002. XBOX:chucklepie

Go to: