Try / OpenGothic

Reimplementation of Gothic 2 Notr
MIT License
1.17k stars 85 forks source link

[macOS] keyboard issues #395

Closed sjavora closed 1 year ago

sjavora commented 1 year ago

Hi, just discovered this project - awesome job!

Trying to play I discovered a couple of problems with the controls. I'm unable to bind most of the lowest row of my keyboard. Specifically on my MBP keyboard, these are control, left option (alt), left command, right command, right option (alt). Also neither Shift key works. The main menu just does nothing when I try to bind them. Only the space bar works. I'm assuming this is some kind of mapping issue, but I have no idea what to do about it. I'm using an US keyboard layout.

I also tried equipping a weapon (the heavy branch at the very beginning), but hitting action in the inventory drops the item instead of using it. This can be worked around by using the numbers to assign slots, but I imagine that won't work for consumables.

Finally, swinging the weapon left or right and then holding down either left or right when not focused on an enemy will cause the character to keep turning in that direction, same as if action was not held down. In "official" Gothic I believe the behavior is that as long as action is being held, the character won't turn.

sjavora commented 1 year ago

I suppose the binding issues have something to do with the contents of keycodec.cpp, but I have no idea where the hex values in list come from. I noticed that Tempest has cases for the Command keys which aren't specified in keycodec.cpp, but then again, I can't even bind Ctrl and that is specified, so 🀷

Try commented 1 year ago

Hi, @sjavora and thanks for bug report!

I'm unable to bind most of the lowest row of my keyboard.

This is an engine bug: engine assumes, that NSEventTypeKeyDown[Up] is valid way to handle all key, but commmand/alt/ctr are special cases on Mac and event is key-event is not generated for them.

noticed that Tempest has cases for the Command keys which aren't specified in keycodec.cpp

Yep, need to add them. As for hex values, those are internal values used by original Gothic2 *.ini files. For Mac-specific keys, assigning some unique values should be good enough.

but hitting action in the inventory drops the item instead of using it.

Apparently you have overlap of action-key and trow-key somehow. For item drop: Z - drop 10 pieces X - 100 pieces Space - all pieces Jump - one piece

In "official" Gothic I believe the behavior is that as long as action is being held, the character won't turn.

Not 100% if I truly follow this, yet seem to be correct behavior. If left/right key is not participating in combo (or timing is missed), then this is a move.

sjavora commented 1 year ago

Hey @Try!

For item drop: Z - drop 10 pieces X - 100 pieces Space - all pieces Jump - one piece

Is Space hardcoded to do that? Because I've always used Space as my action key.

Trying it in G2 in a Wineskin bottle, it looks to me like Jump = drop all. I think Action is used to move 1 piece when interacting with merchants or chests. I know that in G1, you used modifier keys for transfering different amounts.

Not 100% if I truly follow this, yet seem to be correct behavior. If left/right key is not participating in combo (or timing is missed), then this is a move.

So I just checked this again. It's not even limited to a weapon being drawn. As long as I hold Action, the character won't turn when I hold the "turn left/right" button. Even when I'm already pressing one of those buttons and then also hold Action, the character stops turning. Walking forward (and I imagine backward) and strafing works normally.

Is there something I can do to help? I have an M1 Mac (obviously) and experience with software development, though probably not very useful here (never worked on something like this, only iOS apps).

Try commented 1 year ago

Tested how keys are working in vanilla: Jump - drop all Space (assume it's not jump) has no effect

Following GIT blame, X and Z were introduced by 5e0ca8b4b85adf6e6d3b18a63d6bf1b9ec9904ce :

Author: da3m0n, Sat Feb 1 19:43:06 2020 +0000 (3 years ago)
Inventory QOL

Refactored faster looting.
Added x10, x100 and stack looting.
Now gold and equipped items can't be sold.

For now decide to lower priority of hard-coded keys, so they wont clash with game-related actions

Try commented 1 year ago

Is there something I can do to help?

It would prefect, if you can take a look in playercontrol.cpp and adjust weapon-related behavior to vanilla game. Code in PlayerControl can be messy, so fell free to ask questions, in needed.

sjavora commented 1 year ago

Is there something I can do to help?

It would prefect, if you can take a look in playercontrol.cpp and adjust weapon-related behavior to vanilla game. Code in PlayerControl can be messy, so fell free to ask questions, in needed.

Ok, I'll try πŸ‘

Just tried b9f20ee and I can conform that space/action now correctly equips weapons - thank you!

sjavora commented 1 year ago

Alright, I've found the line that needs to be altered:

bool  allowRot = !(pl.isPrehit() || pl.isFinishingMove() || pl.bodyStateMasked()==BS_CLIMB);

I tested this by adding || pl.isInWater() in there and it does indeed stop rotation while standing in water and left/right swings are still possible.

The question now is... what to put there? I'm not sure what property I can use - the only condition in my head is "action is being held down". I don't know if that in itself is tracked anywhere or whether that sets some bit somewhere...

Try commented 1 year ago

"action is being held down".

There are multiple options: ctrl[KeyCodec::ActionGeneric] is probably what you looking for (+check for weapon state)

sjavora commented 1 year ago

ctrl[KeyCodec::ActionGeneric] is probably what you looking for

Indeed - adding that does exactly what I observe in vanilla!

+check for weapon state

Why? πŸ€” This behavior happens regardless of whether you have a weapon drawn.

Should I make a merge request?

Try commented 1 year ago

Why? πŸ€” This behavior happens regardless of whether you have a weapon drawn.

Ah, OK then :)
Yes, feel feel free to send a PR

sjavora commented 1 year ago

Sorry for the delay - PR is up - #400.

sjavora commented 1 year ago

Now for the final item

This is an engine bug: engine assumes, that NSEventTypeKeyDown[Up] is valid way to handle all key, but commmand/alt/ctr are special cases on Mac and event is key-event is not generated for them.

Could you point me to where this is processed?

Try commented 1 year ago

Could you point me to where this is processed?

Probably need to do similar hack to Windows. On windows, when receiving any event, engine does explicit check for ALT:

  static bool altL = false;
  static bool altR = false;

  if((GetKeyState(VK_LMENU) < 0)!=altL) {
    altL = !altL;
    handleKeyEvent(cb,altL ? WM_SYSKEYDOWN : WM_SYSKEYUP,VK_MENU,0);
    }
  if((GetKeyState(VK_RMENU) < 0)!=altR) {
    altR = !altR;
    handleKeyEvent(cb,altR ? WM_SYSKEYDOWN : WM_SYSKEYUP,VK_MENU,0x01000000);
    }

On apple, there seem to be a proper flagsChanged event: https://developer.apple.com/documentation/appkit/nsresponder/1527647-flagschanged Need to check that as well.

PS Relevant code is here: lib\Tempest\Engine\system\api\macosapi.mm

sjavora commented 1 year ago

Created PR implementing modifiers handling over in https://github.com/Try/Tempest/pull/45.

sjavora commented 1 year ago

And final PR for this issue - #402.

Thanks again for your help, I'm sure I'll find other things later, but for now I can finally play 🀩.