Fast/slow camera scrolling jamming

[6AKU66]

Holding Shift or Ctrl while pressing enter (opens chat) leads to fast/slow toggled scrolling (like shift/ctrl still holding, but it's not).

https://youtu.be/J3JS7MMabUU

[MasterBel2]

Camera's fast/slow move states are bound to key actions: https://github.com/beyond-all-reason/spring/blob/BAR105/rts/Game/UnsyncedGameCommands.cpp#L3518

Key actions aren't considered if Lua uses the keypress: https://github.com/beyond-all-reason/spring/blob/BAR105/rts/Game/Game.cpp#L1075

Chat is eating all keypresses by enabling OwnText: https://github.com/beyond-all-reason/Beyond-All-Reason/blob/master/luaui/Widgets/gui_chat.lua#L1468

This unconditionally blocks anything else from using those keypresses: https://github.com/beyond-all-reason/Beyond-All-Reason/blob/master/luaui/barwidgets.lua#L1407

Would an appropriate solution be to release all actions when OwnText is enabled?

[MasterBel2]

cc @badosu

[badosu]

You got it figured out. Releasing all actions is not feasible though.

One solution would be to rethink the way we capture text. Make sure chat uses the textinput event and avoiding keypress halting as much as possible

[badosu]

ownText was a mechanic I introduced to avoid ad-hoc solutions for the chat widget, but it might not be the correct one

[MasterBel2]

I'll have a look into TextInput and see what it does.

[badosu]

Another thing is that most of the keypress captures on the chat widget can be extracted to actions that might solve most of the issue in itself

[MasterBel2]

That would be good for consistency, but I do fear it would overcomplicate things. What it may be wiser to track who ate KeyPress and make sure they receive KeyRelease? Say, return a pointer rather than a boolean.

[badosu]

I'll have a look into TextInput and see what it does.

It's already being used, it's the correct event to capture text. The only remaining being to extract the keypresses to actions although given the complexity of chat feature set might not still be enough, could be though

[badosu]

That would be good for consistency, but I do fear it would overcomplicate things. What it may be wiser to track who ate KeyPress and make sure they receive KeyRelease? Say, return a pointer rather than a boolean.

This hopefully can be addressed without having to change the logic of the chain or external events to chat, just by smart utilization and ordering of actions and bindings.

[MasterBel2]

Do you want to take it down the binding route, then? Or if you get me up to speed I could have a go

[badosu]

You can take a look at some PRs I sent in Mar/April here that do something similar ('*: Extract keypress to actions'): https://github.com/beyond-all-reason/Beyond-All-Reason/pulls?q=is%3Apr+is%3Aclosed+author%3Abadosu

However the featureset of chat is way more complex so it might not be extremely straightforward. The important thing is to know when and how to halt the keypress chain by returning true when chat wants to capture it.

Since releasing shift is not tied to an action it will naturally release the engine bindings while retaining the chat features

[MasterBel2]

A bunch of things definitely could be converted to keyactions, but afaict TextInput will still have to make use of OwnText()? Then how do we stop those keypresses from going to something other than Chat?

I'll play around with this a bit to see what I can discover

[MasterBel2]

The problem is that chat has to claim all input on all keys. If you hold any key and activate chat, it stays locked on. Unless I'm mistaken, this is not negotiable (Unless we go and implement multi-key characters, which given the size of charactersets that use multi-key characters, would be an absolute nightmare, and I'm very glad we have SDL for that.)

So either we need to ensure that everything that gets a keydown gets a keyup (the assumption that this is the case causes the problems here) or we need allow actions based on non-stateful triggers. Repeat is close to this, but doesn't trigger every frame. Presumably we could even kill keyup API access, such that you either trigger on a keyPressedThisFrame, or you check whether keyIsCurrentlyDown

I'm also trying to work out why ActionHandler is implemented both in C++ and Lua. Perf? :/

[badosu]

One way to fix this particular issue is to let keypress/release propagate if it's just a modifier release/press

[MasterBel2]

Sure, except this bug also affects the arrow keys. (I probably should have noted that somewhere before now)

[badosu]

Make it a whitelist in the chat keypress then

[MasterBel2]

Could do. Chat does use arrow keys and shift, but that should all be able to go through keyactions now.

So, then, allow provide OwnText with a whitelist? It feels ugly and like we'll encounter problems with it in the future, but we can do it.

Part of the issue here - if any user binds any other key to an action that expects the keyup notification, they're gonna have to add that to the whitelist, and then that starts poking holes in the entire theory. Of course, we could just not allow them to do that, but I'd much prefer a more robust solution

[badosu]

So, then, allow provide OwnText with a whitelist?

No, should be a check to whether return true on KeyPress or not.

One can remove OwnText logic after that

[MasterBel2]

Thing with OwnText is it overrides established priority based on widget layer - that's important. Whitelist would have to happen to prevent keypresses from being eaten by whichever widget has a lower layer. (lower layers are called first.) That's why it would need to be a whitelist/filter/whatever there. Implementation of course would just change to basically giving it KeyPress priority, I guess, because KeyPress acts as a good enough filter.

Edit: KeyPress + KeyRelease

[badosu]

Feel free to revert https://github.com/beyond-all-reason/Beyond-All-Reason/commit/fd10d2f20866873f6675f1b1a01fabeb5db4e3fb in your feature branch and perform the fix that suits best the problem.

[6AKU66]

P.S Keys up, down, left, right also jamming

[MasterBel2]

Any key with a press + hold action (that waits for keyrelease) will likely exhibit this behaviour. Likely including page up / page down.

[badosu]

The problem is the way it's handled by the game. The action chain is not halted simply by pressing shift due to previous actions not halting the repeat, not halting on keypress on simply not being bound to shift.

There might be ways to address this by halting the chain in the desired circumstances.

[badosu]

Engine defined actions are processed after the normal chain so. with careful considerations, the undesired behavior can be captured while allowing for the desired one.

https://github.com/beyond-all-reason/spring/blob/8922881c44c99854f6ba733b0e346f9ed62fa8d4/rts/Game/Game.cpp#L1034-L1038

If widgets that care about this behavior can't consider for this case - due to either not being a maintainable practice, carelessness, or due to the scope requiring an integrated approach - then a widget with the highest layer possible, capturing for the action, can check for everything the game defines as undesirable state for move(fast|slow|up|down|forward|back|right|left) and halt it there.

[badosu]

There's also the simplest fix possible, that is to not bind to the move* actions in the first place lol. I don't think I know a single person that relies on it

[MasterBel2]

Have you asked? The only one I don't make heavy use of is moveslow

[WatchTheFort]

Can we just get rid of the fast scrolling/fast panning actions altogether?

[MasterBel2]

Sure, but fast panning is a feature I find very valuable - and I can’t be the only person who values it.

On 17 Sep 2022, at 12:13 pm, WatchTheFort @.***> wrote:

Can we just get rid of the fast scrolling/fast panning actions altogether?

— Reply to this email directly, view it on GitHub https://github.com/beyond-all-reason/Beyond-All-Reason/issues/967#issuecomment-1249977850, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJLD4DX3CUDVOQ6AGX7VD3DV6USNDANCNFSM5V5NID7A. You are receiving this because you commented.

[badosu]

Do you use the modifier to move faster/slower often too?

[6AKU66]

@badosu If question to me then i'm using those modifiers usually when i'm recording something.