GarageGames / Torque3D

MIT Licensed Open Source version of Torque 3D from GarageGames
http://torque3d.org
MIT License
3.35k stars 1.2k forks source link

Corrects modifier mask for keyboard events #2244

Closed OTHGMars closed 6 years ago

OTHGMars commented 6 years ago

This commit updates the modifier mask that is sent with key events to correctly reflect the state of the six modifier keys.

dottools commented 6 years ago

You sure about these changes? Cause such as SI_SHIFT means any shift key (either left or/and right) is pressed. So for key binding code when you don't care which shift is used, such as say text input mode but need to know when to engage CAPS, you check for SI_SHIFT instead of (SI_LSHIFT | SI_RSHIFT). So naturally SI_SHIFT needs to be set anytime right or left shift is pressed in this case.

OTHGMars commented 6 years ago

Yes, quite sure actually. As you point out when checking to see if caps need applied, we use (mask & SI_SHIFT). ((mask & SI_SHIFT) == true) if EITHER bit is set in the mask, it does not require both. That's why we have 3 defines for only 2 bits. If the intent of the function was to assign both bits in response to either key press, the whole function could be reduced to:

U32 getTorqueModFromSDL(U16 mod) { U32 ret = 0;

  if (mod & KMOD_SHIFT)
     ret |= SI_SHIFT;

  if (mod & KMOD_CTRL)
     ret |= SI_CTRL;

  if (mod & KMOD_ALT)
     ret |= SI_ALT;

  return ret;

}

“you check for SI_SHIFT instead of (SI_LSHIFT | SI_RSHIFT)"...yes, because it's more convenient, not because it's different: https://github.com/GarageGames/Torque3D/blob/development/Engine/source/platform/input/event.h#L339

dottools commented 6 years ago

Ah ok SI_SHIFT isn't a unique bit/state then. Makes sense now.

OTHGMars commented 6 years ago

Withdrawing this PR. It will not be needed after SDL event refactoring.