[SDL] Need some advice here

M. Egmond sdl at elmerproductions.com
Sun Jan 5 06:59:01 PST 2003

At 2003-01-03 21:54, you wrote:
>#define UP 1
>#define RIGTH 2

RIGTH should probably be RIGHT. It's not a problem since you make the same 
spelling mistake in the code, but it might look better if you correct it.


>void loadimg()

I would opt for using more parameters instead of global variables here, but 
that may be more of a style issue. However, often global variables obscure 
dataflow. For example my choice might be something like:
   int loadimg(SDL_Surface *screen, SDL_Surface **image1return, SDL_Surface 
and then return an error code from the function. If you don't do error 
checking (for example because it's meant to be a simple example) then you 
can simply 'return 0' at the end to indicate it always succeeds (assuming 0 
is your choice to return OK, and non-zero are error codes). Anyway, I 
already said this is more a style issue, so I will shut up about it now :-)

>  SDL_Surface *tmp;
>  //Load background from a bitmap
>  tmp=SDL_LoadBMP("backg.bmp");
>  back=SDL_DisplayFormat(tmp);
>  //SDL_BlitSurface(back, NULL, screen, NULL);

You need to free the tmp surface here to avoid memory leaks (already 
mentioned by another poster). You may also consider what happens if the 
'backg.bmp' is not the same size as the screen.


>         fprintf(stderr, "No se puede inicializar SDL: %s\n",SDL_GetError());

Ah, I like printing errors to stderr instead of stdout. Good! :-)



Not sure why you need KeyRepeat. Your code sets 'direc' on a keypress, but 
never resets it, so a keyrepeat would simply overwrite direc with the same 
value. If you want to allow multiple directions to be pressed at the same 
time, it might be useful.

You should also be careful. You poll only 1 event per loop. This means that 
if events come in faster than your loop executes you will start lagging 
behind. I would consider that especially dangerous because you have 
key-repeat on...

Also, because you don't reset 'direc' when you stop pressing a key, the 
direc flag will remain set, and your 'object' will keep travelling in the 
direction that was pressed last.


>  //Ahora movemos la imagen de acuerdo a la direccion
>  switch (direc){
>         case UP:
>            if (area.y>1) area.y--;

This code is not timed, and will execute each loop. This means that if you 
have a very fast videocard (or refresh rate) the 'object' will travel a lot 
faster than on a slower machine/refresh rate. You might want to do 
something like:
   if (current_time-time_of_last_move>movement_interval)
     time_of_last_move = current_time;
     [insert your switch statement here]


>         case RIGTH:
>            if (area.x<640) area.x++;

Small point: this might need to be 639, since position 640 is off-screen. 
It's interesting to note that your 'lower' limit is 1, even though 0 is 
still on screen. Depending on what the 'object' actually represents this 
might or might not be meaningful. If it's a single pixel, then the correct 
borders would probably be 0 and 639.


>  SDL_BlitSurface(back, NULL, screen, NULL);
>  SDL_BlitSurface(sp, NULL, screen, &area);
>  if ((screen->flags & SDL_DOUBLEBUF) != SDL_DOUBLEBUF) 
> SDL_UpdateRect(screen,0,0,0,0);
>         else SDL_Flip(screen);

This piece of code was already discussed by another poster.


Hope this helps!

More information about the SDL mailing list