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
    Commented Mar 23, 2021 at 11:55

1 Answer 1

1
\$\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

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .