Allegro.cc - Online Community

Allegro.cc Forums » Programming Questions » Am I using 'do while' loops correctly?

This thread is locked; no one can reply to it. rss feed Print
Am I using 'do while' loops correctly?
Rich Y
Member #15,033
April 2013

Hey Guys,

I'm creating a menu system which when you hover over a button it will changed to a bitmap of a similar image only in a manor so that the user can see it is highlighted. I am using do while loops to do this and have been having issues and have come to the conclusion I probably shouldn't be using them for this task, could anyone tell me exactly what I am doing wrong here?

Thank you!!

#SelectExpand
1//This loop checks if the mouse pointer is within a defined box, if it is then it sets it to true, otherwise it is set as false. 2 3do{ 4 if (mouse_x <= XposMaxPlay && mouse_x >= XposMinPlay && 5 mouse_y <= YposMaxPlay && mouse_y >= YposMinPlay) 6 { 7 mouse_pos = true; 8 } 9 else 10 { 11 mouse_pos = false; 12 } 13 if(key[KEY_ESC]) quit = true; // If ESC is pressed, exit loop 14 } while (!quit); 15 16 17 18//The next loop checks undertakes the clearing of the previous image and the blitting of the new image if the mouse position is detected as true. 19 20 21 do { if (mouse_pos = true) 22 { 23 clear_bitmap(play1); 24 blit(menubackground, screen, XposMinPlay, YposMinPlay, XposMinPlay, YposMinPlay, XposMaxPlay, YposMaxPlay); 25 masked_blit( play2, screen, 0, 0, 320, 240, menubackground->w, menubackground->h ); //Draw the bitmap 26 } 27 if(key[KEY_ESC]) quit = true; // If ESC is pressed, exit loop 28 } while (!quit); 29 30 31 32// This is the final section meaning when the mouse position isn't in the box it ensures that the second image has been cleared and the first image has been blitted. 33 34 35 36 do { if (mouse_pos = false) //checking if the mouse is within the region of the specific bitmap 37 { 38 clear_bitmap(play2); 39 blit(menubackground, screen, XposMinPlay, YposMinPlay, XposMinPlay, YposMinPlay, XposMaxPlay, YposMaxPlay); 40 masked_blit( play1, screen, 0, 0, 320, 240, menubackground->w, menubackground->h ); //Draw the bitmap 41 } 42 if(key[KEY_ESC]) quit = true; // If ESC is pressed, exit loop 43 } while (!quit);

Arthur Kalliokoski
Second in Command
February 2005
avatar

It would work if you enclosed them all in a single do-while with only one key test at the end. The way you have it only runs the first block over and over, then when you press ESC it does the next two blocks once each and quits.

“Throughout history, poverty is the normal condition of man. Advances which permit this norm to be exceeded — here and there, now and then — are the work of an extremely small minority, frequently despised, often condemned, and almost always opposed by all right-thinking people. Whenever this tiny minority is kept from creating, or (as sometimes happens) is driven out of a society, the people then slip back into abject poverty. This is known as "bad luck.”

― Robert A. Heinlein

Rich Y
Member #15,033
April 2013

ahhh okay, that is what I was worried about - I'll give that a go now, thanks for clearing things up.

EDIT: Would the following be the correct layout? as I'm still having some issues as it seems to be crashing on me after it loads up?

Cheers

#SelectExpand
1 do{ 2 if (mouse_x <= XposMaxPlay && mouse_x >= XposMinPlay && 3 mouse_y <= YposMaxPlay && mouse_y >= YposMinPlay) 4 { 5 mouse_pos = true; 6 } 7 if (mouse_pos = true) 8 { 9 clear_bitmap(play1); 10 blit(menubackground, screen, XposMinPlay, YposMinPlay, XposMinPlay, YposMinPlay, XposMaxPlay, YposMaxPlay); 11 masked_blit( play2, screen, 0, 0, 320, 240, menubackground->w, menubackground->h ); 12 } 13 else 14 { 15 mouse_pos = false; 16 } 17 if (mouse_pos = false) 18 { 19 clear_bitmap(play2); 20 blit(menubackground, screen, XposMinPlay, YposMinPlay, XposMinPlay, YposMinPlay, XposMaxPlay, YposMaxPlay); 21 masked_blit( play1, screen, 0, 0, 320, 240, menubackground->w, menubackground->h ); 22 } 23 24 if(key[KEY_ESC]) quit = true; // If ESC is pressed, exit loop 25 } while (!quit);

Jeff Bernard
Member #6,698
December 2005
avatar

Use `==` for equality. `=` is the assignment operator. Or, you don't need to use an operator at all because it's a Boolean:

if (my_bool) // if my_bool is true

if (!my_bool) // if my_bool is false

//Note: 0 = false, non-zero = true

You don't want to call clear_bitmap, because that means you can't re-use the image that was cleared (unless you're restoring it somewhere else that you didn't show).

Also, you don't want to use mouse_pos like that, my guess is that's what's causing your crash. It's an Allegro variable and should be treated as read-only (or reserved/don't use since there are other more specific variables to use).

Edit: That else statement does nothing, since "mouse_pos" would be false anyway.

Edit2: Actually, your crash may be coming from masked_blit. You should pass in the width and height of the bitmap you're blitting, rather than the width and height of the background.

--
I thought I was wrong once, but I was mistaken.

Rich Y
Member #15,033
April 2013

Ahh, Thank you Jeff - what would you suggest using instead of clear_bitmap? Clear to col? (I just want it to be hidden)

Thanks

Jeff Bernard
Member #6,698
December 2005
avatar

Do nothing. If you don't draw it, then it won't be there.

--
I thought I was wrong once, but I was mistaken.

Rich Y
Member #15,033
April 2013

That's a fair point!

I've now changed the code to the following however when I run it the image flickers for a few seconds before stopping, and then the press escape to quit button doesn't work either which suggests the loop isn't working as it should..

#SelectExpand
1 do{ 2 if (mouse_x <= XposMaxPlay && mouse_x >= XposMinPlay && 3 mouse_y <= YposMaxPlay && mouse_y >= YposMinPlay) 4 { 5 pointer_pos = true; 6 } 7 if (pointer_pos) 8 { 9 blit(menubackground, screen, XposMinPlay, YposMinPlay, XposMinPlay, YposMinPlay, XposMaxPlay, YposMaxPlay); 10 masked_blit( play2, screen, 0, 0, 320, 240, menubackground->w, menubackground->h ); 11 } 12 else 13 { 14 pointer_pos = false; 15 } 16 if (!pointer_pos) 17 { 18 blit(menubackground, screen, XposMinPlay, YposMinPlay, XposMinPlay, YposMinPlay, XposMaxPlay, YposMaxPlay); 19 masked_blit( play1, screen, 0, 0, 320, 240, menubackground->w, menubackground->h ); 20 } 21 22 if(key[KEY_ESC]) quit = true; // If ESC is pressed, exit loop 23 } while (!quit);

Edgar Reynaldo
Member #8,592
May 2007
avatar

Look at your logic :

Quote:

if (a) {
   // do stuff
}
else {
   a = false;// a is already false, this is pointless
}

if (!a) {
   // do other stuff
}

That could be reduced into a single if else statement.

You should look into proper game timing loops. Basic structure with A4 is this :

while (!quit) {
   while (ticks < 1) {rest(0);}
   int oldticks = ticks;
   ticks = 0;
   for (int i = 0 ; i < oldticks ; ++i) {
      DoLogic();
   }
   Draw();
}

Rich Y
Member #15,033
April 2013

Okay thanks edgar, as I'm new to all this I would rather stick to the way I'm attempting because at least I can make some sense of it compared to what you posted at the end which I am almost completely clueless about however thank you for letting me know as it will be useful to look into that when I get a bit better.

I have now changed the code to something abit more simple which is essentially

if (//cursor is in box)
{
//draw new picture
}
else
{
//draw original image
}

however this doesn't seem to work and instead when it loads up the image opens and just starts flashing and then freezes up, I've pasted the code again below, any help anybody? Thank you for your time!!

   do{
           if (mouse_x <= XposMaxPlay && mouse_x >= XposMinPlay && 
              mouse_y <= YposMaxPlay && mouse_y >= YposMinPlay)
       {
              blit(menubackground, screen, XposMinPlay, YposMinPlay, XposMinPlay, YposMinPlay, XposMaxPlay, YposMaxPlay);
              masked_blit( play2, screen, 0, 0, 320, 240, menubackground->w, menubackground->h ); 
       }
       else 
       {
              blit(menubackground, screen, XposMinPlay, YposMinPlay, XposMinPlay, YposMinPlay, XposMaxPlay, YposMaxPlay);
              masked_blit( play1, screen, 0, 0, 320, 240, menubackground->w, menubackground->h ); 
       }
      
        if(key[KEY_ESC]) quit = true;  // If ESC is pressed, exit loop
     } while (!quit);

Edgar Reynaldo
Member #8,592
May 2007
avatar

Well first, you don't want to use 100% cpu. You can avoid this by using rest(milliseconds_to_rest); in your logic loop. The screen is flashing because you are drawing one thing to the screen and then drawing another immediately over it. Changes to the screen show as soon as the retrace occurs. What you need to do is draw to a buffer first, and then draw that buffer to the screen once in a single blit call per frame.

So, allocate a BITMAP* with create_bitmap(width,height);, check that it did not return 0 (NULL) and then clear_to_color(buffer , makecol(0,0,0));.

Then when you go to draw :
1) Clear your buffer to whatever background color you want
2) Draw your images onto the buffer
3) Draw the buffer onto the screen
4) Now rest(16) or something until you have a better timing system.

Neil Roy
Member #2,229
April 2002
avatar

I see what you're doing. I did something similar in a game I am currently working on. What I did was I created some functions (all in C) for creating buttons on screen. These buttons have three images associated with them. One is the button you seen when it is on screen and nothing is happening with it. The second button image is what you see if the mouse hovers over it (changes colour usually). the third is an image you see if you actually use that button (click it, press ENTER etc).

Note: I use Allegro 5 with these, but they can just as easily be used with Allegro 4 with minor changes.

I have a struct that is used to define each button:

typedef struct BUTTON {
   int x;                  // upper left corner horizontal position
   int y;                  // upper left corner vertical position
   int w;                  // width of button
   int h;                  // height of button
   int state;              // Button state: 0 up, 1 over, 2 down
   int old_state;          // store the old state to see if it changed
   bool is_switch;         // is this a switch (true) or button (false)?
   ALLEGRO_BITMAP *bmp;    // main bitmap with button images
   ALLEGRO_BITMAP *up;     // subbitmap for button up graphic (when it isn't active)
   ALLEGRO_BITMAP *over;   // subbitmap for button that is being hovered over by mouse
   ALLEGRO_BITMAP *down;   // subbitmap for button after it has been clicked
} BUTTON;

Then I have a bunch of functions for creating new buttons, checking them to see if a mouse is over them and drawing them...

bool new_button(BUTTON *b, bool is_switch, const char *file, int x, int y);
bool check_button(BUTTON *b, ALLEGRO_MOUSE_STATE *mouse);
void draw_button(BUTTON *b);
void destroy_button(BUTTON *b);

When I create a new button I use:

BUTTON Play = {0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL};

This will initialize all the struct elements just to be safe.

When I call my check_button() function I pass the mouse state to it and the function takes care of checking if the mouse is over it and/or being clicked on. if it is, it sets the state variable to indicate which one is happening so then the draw_button() function can draw the proper button.

I want to expand this and have these functions use a linked list to store all buttons and update them, currently you have to manually check each button, but it works well.

When they are drawn it's fairly simple though, you just draw the button at the appropriate location. There is no need to erase the button because the entire screen is cleared and redrawn each frame. When the button changes, you simply redraw the new image. Because the background buffer you're drawing to is cleared and you redraw everything, anything that used to be there is gone. You shouldn't be drawing directly on the screen except to blit() a background buffer to the screen.

You should create a BITMAP the size of your screen, draw to it, then blit it to the screen when you have your scene completely drawn. You clear this bitmap each frame and redraw your scene each time.

Go to: