0
\$\begingroup\$

This is my first time actually trying to wrap the the SDL event handling functions in a class of my own. I'm trying to achieve a design similar to unity's where you have this static Input class and you can call it's functions whenever to check for various input events.

Here is the class:

class InputManager
{
private:
    InputManager() = default;
    InputManager(const InputManager& c) = delete;

private:
    static SDL_Event event;
    static bool isKeyPressed;
    static bool isKeyReleased;
    static bool isMousePressed;
    static bool isMouseReleased;

    static int keyIndex;
    static int buttonIndex;

public:
    static InputManager* GetInstance();
    void Update();

public:
    bool KeyDown(int iKeyIndex);
    bool KeyStillDown(int iKeyIndex);
    bool KeyUp(int iKeyIndex);
    bool KeyStillUp(int iKeyIndex);
    bool MouseDown(int iButton);
    bool MouseStillDown(int iButton);
    bool MouseUp(int iButton);
    bool MouseStillUp(int iButton);
};

and it's implementations:

SDL_Event InputManager::event;
bool InputManager::isMousePressed = false;
bool InputManager::isMouseReleased = false;
bool InputManager::isKeyPressed = false;
bool InputManager::isKeyReleased = false;

InputManager* InputManager::GetInstance()
{
    static InputManager* instace = new InputManager();
    return instace;
}

void InputManager::Update()
{
    SDL_PollEvent(&event);
}

bool InputManager::KeyDown(int iKeyIndex)
{
    if(event.type == SDL_KEYDOWN && event.key.keysym.sym == iKeyIndex && isKeyPressed == false)
    {
        isKeyPressed = true;
        return true;
    }  
    else if(event.type == SDL_KEYUP)
    {
        isKeyPressed = false;
        return false;
    }
    else
        return false;
}

bool InputManager::KeyStillDown(int iKeyIndex)
{
    if(event.type == SDL_KEYDOWN && 
       event.key.keysym.sym == iKeyIndex)
        return true;
    else
        return false;
}

bool InputManager::KeyUp(int iKeyIndex)
{
    if(event.type == SDL_KEYUP && event.key.keysym.sym == iKeyIndex && isKeyReleased == false)
    {
        isKeyReleased = true;
        return true;
    }
    else
    {
        isKeyReleased = false;
        return false;
    }   
}

bool InputManager::KeyStillUp(int iKeyIndex)
{
    if(event.type == SDL_KEYUP &&
       event.key.keysym.sym == iKeyIndex && event.key.repeat == 0)
        return true;
    else 
        return false;
}

bool InputManager::MouseDown(int iButton)
{
    if(event.type == SDL_MOUSEBUTTONDOWN && event.button.button == iButton && isMousePressed == false)
    {
        isMousePressed = true;
        return true;
    }
    else if(event.type == SDL_MOUSEBUTTONUP)
    {
        isMousePressed = false;
        return false;
    }
    else
        return false;
}

bool InputManager::MouseStillDown(int iButton)
{
    if(event.type == SDL_MOUSEBUTTONDOWN && event.button.button == iButton)
        return true;
    else
        return false;
}

bool InputManager::MouseUp(int iButton)
{
    if(event.type == SDL_MOUSEBUTTONUP && event.button.button == iButton && isMouseReleased == false)
    {
        isMousePressed = true;
        return true;
    }
    else
    {
        isMousePressed = false;
        return false;
    }
}

bool InputManager::MouseStillUp(int iButton)
{
    if(event.type == SDL_MOUSEBUTTONUP && event.button.button == iButton)
        return true;
    else
        return false;
}

Now I know this class has it's flaws and it certainly does, for instance the KeyUp(int iKeyIndex) and MouseUp(int iButton) functions seem to always be true unless I move the mouse or do any type of event. I'm aware of issue and why it's happening but I can't seem to find a solution for it. I would like to know how to fix these issues.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ "I would like some feedback, how to improve on the overall class, what design patterns to keep in mind" This is off topic for this site, but I think it is on purpose on Code Review. "how to fix the issue I mentioned above" This is on topic here and you should ask only about this specifically. \$\endgroup\$ – Vaillancourt Mar 23 at 11:55
2
\$\begingroup\$

First, let's look at the doc for SDL_PollEvent, specifically the example code provided:

while (1) {
    SDL_Event event;
    while (SDL_PollEvent(&event)) {
        /* handle your event here */
    }
    /* do some other stuff here -- draw your app, etc. */
}

and the remarks:

If event is not NULL, the next event is removed from the queue and stored in the SDL_Event structure pointed to by event. The 1 returned refers to this event, immediately stored in the SDL Event structure -- not an event to follow.


There are a couple of issues with this class, as you mention, directly related to how the SDL_PollEvent is used:

  • it can handle only one event per frame, there could be more than one event per frame;
  • it will "handle" all the events, including the SDL_WindowEvent, SDL_QuitEvent, etc. You don't want to handle those events in your InputManager.

I have not worked with Unity's input manager, but the way it looks, it is keeping some kind of state. It looks a bit like what you've tried to do: you need to update a state with the events as a specific phase, so that your game code can consult the input state as it needs, without modifying the state. You need to do this for each key.

The following pseudoc++.

class InputManager
{
private:
  enum class KeyState
  {
    JustUp
    Up
    JustDown
    Down
    DownRepeat
  };
  
  using keyIndex = int;
  map<keyIndex, KeyState> m_keyStates;
  map<keyIndex, KeyState> m_mouseStates;
  
  // singleton ctor stuff

public:
  
  // singleton getter stuff
  
  // This should be called at the beginning of each frame to tell your manager
  // that a new frame is starting. Use this to move the keys that were "just*"
  // to a "*" state.
  void prepareFrame()
  {
    for( auto& [keyIndex, state] : m_keyStates )
    {
      if ( state == JustUp )
        state = Up;
      else if ( state == JustDown )
        state = Down
    }
  }
  
  // This is called by your main loop to update the state of your input manager.
  // Each (keyboard) `key` or (mouse) `button` event will be sent to this 
  // function.
  void handleInputEvent( const SDL_Event* event )
  {
    assert( event->type == key || event->type == button );
    
    if ( event->type == key )
    {
      KeyState newState;
      
      if ( event->key.type == SDL_KEYDOWN )
        newState == KeyState::JustDown;
      else if( event->key.type == SDL_KEYUP )
        newState == KeyState::JustUp;

      // This event is sent by the OS, it is based on user's preferences; Useful
      // when text is typed, less useful in the context of a game.
      if ( event->key.repeat != 0 )
        newState == KeyState::DownRepeat; 
        
      m_keyStates[event.key.keysym.sym] = newState;
      
    }
    else if ( event->type == button )
    {
      // Do the same here for handling the (mouse) `button`. 
    }
  }

  // Assuming your "KeyUp" mean "Has the key just been released?"
  bool KeyUp(int iKeyIndex) const
  {
    if ( m_keyStates.count( iKeyIndex ) > 0 )
    {
      if ( m_keyStates[iKeyIndex] == JustUp )
        return true;
    }
    return false;
  }

  // Assuming your "KeyStillUp" mean "Is the up now?"
  bool KeyStillUp(int iKeyIndex) const
  {
    if ( m_keyStates.count( iKeyIndex ) > 0 )
    {
      if ( m_keyStates[iKeyIndex] == JustUp || m_keyStates[iKeyIndex] == Up )
        return true;
      else
        return false;
    }
    // It's not in the map, so we assume it has not been interacted with yet, we
    // consider it's up by default
    return true;
  }

  // This should be used if you want to offer the same "key repeat" experience
  // as the user has with their OS. 
  bool KeyDownOS(int iKeyIndex) const
  {
    if ( m_keyStates.count( iKeyIndex ) > 0 )
    {
      if ( m_keyStates[iKeyIndex] == JustDown 
        || m_keyStates[iKeyIndex] == DownRepeat )
        return true;
    }
    
    // It's not in the map, so we assume it has not been interacted with yet, we
    // consider it's up by default
    return false;
  }

  // Use the same patterns as above to implement the KeyDown, KeyStillDown, 
  // Button* functions...
  
}

Now you need to have something like this in your main loop to update this state.

while (1) {

  InputManager::GetInstance()->prepareFrame();
  
  SDL_Event event;
  while ( SDL_PollEvent( &event ) ) {
    
    switch( event.type )
    {
      case key:
      case button:
        InputManager::GetInstance()->handleInputEvent( &event );
        break;
      
      // case OTHERS....
    }
  }
  
  // the rest of your game loop....
}

This should help you get you started.

Final notes:

  • I've not worked with SDL before, so this is a rough and needs to be tuned with sweat and tears. I found that handling inputs is some of the most annoying code to write.
  • I'm not sure how SDL handles key-modifiers (Shift, Ctrl, Alt), nor how you would like to handle them. E.g. one key could be received as K (upper-case k), or Shift+k (Shift as modifier, with lower-case k).
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.