Bug 626235 - Conflicting definitions of SDLK_LEFT and event->type
Summary: Conflicting definitions of SDLK_LEFT and event->type
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: SDL
Version: 13
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-08-22 22:31 UTC by Jonathan Ryshpan
Modified: 2010-09-06 20:24 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-08-23 11:20:22 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Tail of the log of compiling lbreakout2. (2.91 KB, text/plain)
2010-08-22 22:31 UTC, Jonathan Ryshpan
no flags Details
Move ENABLE_MACRO in menu.c to proper place (955 bytes, patch)
2010-08-23 11:19 UTC, Petr Pisar
no flags Details | Diff

Description Jonathan Ryshpan 2010-08-22 22:31:07 UTC
Created attachment 440265 [details]
Tail of the log of compiling lbreakout2.

Description of problem:
SDLK_LEFT = 276
(defined in /usr/include/SDL/SDL_keysym.h)
while a place for its intended use
event->type  has type Uint8
(defined in /usr/include/SDL/SDL_events.h)

Version-Release number of selected component (if applicable):
SDL-1.2.14-7.fc13.x86_64

How reproducible:
Always

Steps to Reproduce:
1. Attempt to compile lbreakout2-2.6.1 as released by Sourceforge

Actual results:
Compilation fails as in the attached piece of a compilation log shows.

Expected results:
Compilation succeeds.

Additional info:
Very likely this is an SDL problem, but you should see it first.  There may be other problems besides.

Comment 1 Petr Pisar 2010-08-23 10:59:14 UTC
Let's look on the code:

menu.c:
int menu_handle_event( Menu *menu, SDL_Event *event )
{
  if ( event )
    switch ( event->type ) {
      case SDL_KEYDOWN:
        switch ( event->key.keysym.sym ) {
          case SDLK_RIGHT:
          [...]
          case SDLK_LEFT:
        }
      }
    }
  }
}

Now, look into SDL header files:

SDL/SDL_events.h:
typedef union SDL_Event {
    Uint8 type;
    SDL_ActiveEvent active;
    SDL_KeyboardEvent key;
    SDL_MouseMotionEvent motion;
    SDL_MouseButtonEvent button;
    SDL_JoyAxisEvent jaxis;
    SDL_JoyBallEvent jball;
    SDL_JoyHatEvent jhat;
    SDL_JoyButtonEvent jbutton;
    SDL_ResizeEvent resize;
    SDL_ExposeEvent expose;
    SDL_QuitEvent quit;
    SDL_UserEvent user;
    SDL_SysWMEvent syswm;
} SDL_Event;

typedef struct SDL_KeyboardEvent {
    Uint8 type; /**< SDL_KEYDOWN or SDL_KEYUP */
    Uint8 which;    /**< The keyboard device index */
    Uint8 state;    /**< SDL_PRESSED or SDL_RELEASED */
    SDL_keysym keysym;
} SDL_KeyboardEvent;

SDL/SDL_keyboard.h:
typedef struct SDL_keysym {
    Uint8 scancode;         /**< hardware specific scancode */
    SDLKey sym;         /**< SDL virtual keysym */
    SDLMod mod;         /**< current key modifiers */
    Uint16 unicode;         /**< translated character */
} SDL_keysym;

SDL/SDL_keysym.h:
typedef enum {
  [...]
  SDLK_RIGHT      = 275,
  SDLK_LEFT       = 276,
  [...]
}

As you can see, the event->key.keysym.sym variable is type of enum. The SDLK_LEFT (276) fits into enum type as SDLK_RIGTH either (int type must fit into enum by C standard.)

Thus there is no problem from point of view of SDL. (Be ware the SDL_Event is union, big enough to contain any of its members.)

However the compilation failed on SDLK_LEFT and passed SDL_RIGHT before. I guess there is some strange redefinition. But I cannot find it.

Comment 2 Petr Pisar 2010-08-23 11:13:23 UTC
However the compilation error is due to unbalanced ifdef:

                    case SDLK_RIGHT:
                        if ( !menu->cur_item ) break;
                        item_used = 1;
                        /* callback */
                        callback = menu->cur_item->callback;
                        /* action */
                        switch ( menu->cur_item->type ) {
                            case ITEM_SWITCH:
                            case ITEM_SWITCH_X:
                            case ITEM_RANGE:
                                value_inc( menu->cur_item->value );
                                break;
                            default: item_used = 0; break;
                        }
#ifdef AUDIO_ENABLED
                        if ( item_used ) {
                            stk_sound_play( wav_menu_click );
#endif
                            if ( callback ) (callback)();
                        }
                        break;

If #ifdef should be placed under `if ( item_used ) {' line. See case SDLK_RETURN: for proper use. The same bug is has SDLK_LEFT case too.

And after that the compiles becomes silent. I guess the warning is just because of bad parsing due to curly brackets.

Comment 3 Petr Pisar 2010-08-23 11:19:54 UTC
Created attachment 440359 [details]
Move ENABLE_MACRO in menu.c to proper place

This patch fixes lbreakout2 code.

Comment 4 Petr Pisar 2010-08-23 11:27:34 UTC
Fix sent to lbreakout2 maintainer via e-mail.

Comment 5 Petr Pisar 2010-08-23 11:56:07 UTC
Just a notice about packaging: This game is packaged under name `lbrickbuster2' due to trademark in Fedora.

Comment 6 Jonathan Ryshpan 2010-09-06 20:24:41 UTC
(In reply to comment #3)
> Created attachment 440359 [details]
> Move ENABLE_MACRO in menu.c to proper place
> 
> This patch fixes lbreakout2 code.

Very clever debugging; I wish I had found it.  Thanks for the patch and also for Comment 7, about packaging.


Note You need to log in before you can comment on or make changes to this bug.