Elysian Shadows

I currently am making a project where I apply a grid to the screen if the user presses the g button. The problem I ran into is that if I when I press the button the grid flashes on and off, this is because the g key controls turning on and off the key, is there a way that I can make it so that if the key is pressed it will only switch it once? my code is as bellow

That looks like it should work, but it looks like you're going to have to do a switch statement

Try this, I hope it works. Haven't tried it.

Have a variable hold the previous keystate.
I added that and cleaned your code up a little (changes are listed in the comments): In case anyone bitches about the goto, it was ONLY for breaking out of nested loops. I have a thing against "done" variables for main game loops, so I prefer things like break. But since break wouldn't provide the wanted affect, I went with a goto for the SOLE PURPOSE of breaking out of the nested loop. AND I made an explicit comment stating NOT to use gotos in any other situation.

Edit: now WHY was I the only one to help with the overall bad practices? You people obviously forgotten what it's like to be a newbie .
Edit 2: made buffer local rather than global

Good to see a couple of people knew why that wasn't working, use theirs because switch statements are slow

eatcomics wrote:Good to see a couple of people knew why that wasn't working, use theirs because switch statements are slow

and because you approached it completely wrong.
Given the typical key repeat rate of about 20 times per second, his var would toggle slower, but it would still toggle repeatedly. What you need to do is store a var stating whether or not the key was down the previous frame, and use that to determine if it's a new press or not.

You are correct, I don't know why I didn't think of that, I've even had that problem before, a switch statement was the first thing that came to mind

Ginto8 wrote:Have a variable hold the previous keystate.
I added that and cleaned your code up a little (changes are listed in the comments):

Code: Select all

#include <SDL/SDL.h>

// the C++ const is a better choice than the C #define
const int SCREEN_WIDTH 640;
const int SCREEN_HEIGHT 480;

// the { doesn't have to be on the same line as main(), but since I'm editing it, and that's my style,
// that's how I made it
int main (int argc, char *argv[]) {
   // moved this here because global vars are bad practice
   SDL_Surface *buffer = NULL;
   // variables of the same type can be declared in a comma-separated list
   // and bools are by default false
   // gDown is a bool of whether or not G was down last frame
   bool grid,gDown;
   SDL_Event event;
   // calling functions and declaring variables in a mishmosh is bad taste, so I separated the two
   SDL_Init(SDL_INIT_VIDEO);
   buffer = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, 32, SDL_SWSURFACE);
   SDL_WM_SetCaption("Window", NULL);
   
   // an infinite loop
   while(true) {
      // inside of {'s, there should be indentation
      SDL_FillRect(buffer,NULL,0x000000);
      SDL_Rect area = {520, 360, (SCREEN_WIDTH -120), (SCREEN_HEIGHT -120)}; //{x,y,w,h}
      SDL_FillRect(buffer,&area,0xBBBBBB);
      Uint8 *key = SDL_GetKeyState (NULL);
      // in if()'s, {s are only needed for multi-line actions
      // ! is the opposite when it comes to bools
      // if b is true, then !b is false
      // and vice versa
      if(key[SDLK_g] && !gDown)
         grid = !grid;
      gDown = key[SDLK_g];
      // if(x) is basically the same as if((bool)x == true), so you don't need the "== true"
      // and again, the { on the same line is just a stylistic preference
      if(grid) {
         for (int i = 0; i < SCREEN_WIDTH; i+=40) {
            SDL_Rect grid_vertical = {i, 0, 2, SCREEN_HEIGHT};
            SDL_FillRect(buffer,&grid_vertical,0xAAAAAA);
         }

         for (int i = 0; i < SCREEN_HEIGHT; i+=40) {
         SDL_Rect grid_horizontal = {0, i, SCREEN_WIDTH, 2};
         SDL_FillRect(buffer,&grid_horizontal,0xAAAAAA);
         }
      }

      SDL_Flip(buffer);
      while(SDL_PollEvent(&event)) {
         // goto should only be used in situations like this, where there are nested loops
         // otherwise, STAY AWAY FROM THEM
         if (event.type == SDL_QUIT)
            goto nested_loop_break;
      }   
   }
nested_loop_break:
   SDL_Quit();
   return 0;
}

In case anyone bitches about the goto, it was ONLY for breaking out of nested loops. I have a thing against "done" variables for main game loops, so I prefer things like break. But since break wouldn't provide the wanted affect, I went with a goto for the SOLE PURPOSE of breaking out of the nested loop. AND I made an explicit comment stating NOT to use gotos in any other situation.

Edit: now WHY was I the only one to help with the overall bad practices? You people obviously forgotten what it's like to be a newbie .
Edit 2: made buffer local rather than global

Or you could just use a variable and break out of the loop that way, what's your problem with using them?

I'd say a local done variable is much better than using a goto... like seriously why is that goto statement any better than simply putting a bool done; before your loop?

LeonBlade wrote:

Ginto8 wrote:Have a variable hold the previous keystate.
I added that and cleaned your code up a little (changes are listed in the comments):

Code: Select all

#include <SDL/SDL.h>

// the C++ const is a better choice than the C #define
const int SCREEN_WIDTH 640;
const int SCREEN_HEIGHT 480;

// the { doesn't have to be on the same line as main(), but since I'm editing it, and that's my style,
// that's how I made it
int main (int argc, char *argv[]) {
   // moved this here because global vars are bad practice
   SDL_Surface *buffer = NULL;
   // variables of the same type can be declared in a comma-separated list
   // and bools are by default false
   // gDown is a bool of whether or not G was down last frame
   bool grid,gDown;
   SDL_Event event;
   // calling functions and declaring variables in a mishmosh is bad taste, so I separated the two
   SDL_Init(SDL_INIT_VIDEO);
   buffer = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, 32, SDL_SWSURFACE);
   SDL_WM_SetCaption("Window", NULL);
   
   // an infinite loop
   while(true) {
      // inside of {'s, there should be indentation
      SDL_FillRect(buffer,NULL,0x000000);
      SDL_Rect area = {520, 360, (SCREEN_WIDTH -120), (SCREEN_HEIGHT -120)}; //{x,y,w,h}
      SDL_FillRect(buffer,&area,0xBBBBBB);
      Uint8 *key = SDL_GetKeyState (NULL);
      // in if()'s, {s are only needed for multi-line actions
      // ! is the opposite when it comes to bools
      // if b is true, then !b is false
      // and vice versa
      if(key[SDLK_g] && !gDown)
         grid = !grid;
      gDown = key[SDLK_g];
      // if(x) is basically the same as if((bool)x == true), so you don't need the "== true"
      // and again, the { on the same line is just a stylistic preference
      if(grid) {
         for (int i = 0; i < SCREEN_WIDTH; i+=40) {
            SDL_Rect grid_vertical = {i, 0, 2, SCREEN_HEIGHT};
            SDL_FillRect(buffer,&grid_vertical,0xAAAAAA);
         }

         for (int i = 0; i < SCREEN_HEIGHT; i+=40) {
         SDL_Rect grid_horizontal = {0, i, SCREEN_WIDTH, 2};
         SDL_FillRect(buffer,&grid_horizontal,0xAAAAAA);
         }
      }

      SDL_Flip(buffer);
      while(SDL_PollEvent(&event)) {
         // goto should only be used in situations like this, where there are nested loops
         // otherwise, STAY AWAY FROM THEM
         if (event.type == SDL_QUIT)
            goto nested_loop_break;
      }   
   }
nested_loop_break:
   SDL_Quit();
   return 0;
}

In case anyone bitches about the goto, it was ONLY for breaking out of nested loops. I have a thing against "done" variables for main game loops, so I prefer things like break. But since break wouldn't provide the wanted affect, I went with a goto for the SOLE PURPOSE of breaking out of the nested loop. AND I made an explicit comment stating NOT to use gotos in any other situation.

Edit: now WHY was I the only one to help with the overall bad practices? You people obviously forgotten what it's like to be a newbie .
Edit 2: made buffer local rather than global

Or you could just use a variable and break out of the loop that way, what's your problem with using them?

I'm not really sure, it's more of a stylistic preference than anything else (I think that an infinite loop makes it look cleaner, because execution actually will stop rather than going through a whole extra iteration). It's not a big thing, but it's my style, sorta like the { on the same line as the if(). I'm the one that edited it, so it's my style that is added. If you don't like it that way, then you don't have to use my code.

Ginto8 wrote:I'm not really sure, it's more of a stylistic preference than anything else (I think that an infinite loop makes it look cleaner, because execution actually will stop rather than going through a whole extra iteration). It's not a big thing, but it's my style, sorta like the { on the same line as the if(). I'm the one that edited it, so it's my style that is added. If you don't like it that way, then you don't have to use my code.

Understandable, me personally I feel like forcing yourself to BREAK out of a loop seems like the wrong way to do it.
But, if it works it works, I'm not entirely sure what way is better and why... so I'm just going to leave it at that.

I agree with leon, and I kinda think it looks weird, but maybe that's because I haven't used a GOTO statement since I first started programming

eatcomics wrote:Good to see a couple of people knew why that wasn't working, use theirs because switch statements are slow

What the fuck are you talking about? A switch statement is no slower than an if/else block. It's the same thing at the assembly level.

Actually, I'm pretty sure they're faster because you're only checking the value of one variable.

might as well bench em and find out, right.
2872 - if
2864 - switch

not sure what those units are, i believe nano seconds, anyways, looks like the switch for same function is a nano turd faster.

compiler optimizations make switch statements faster then if else blocks.