Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KEYMAPPER: executeAction - keep original ascii value #2812

Closed
wants to merge 1 commit into from

Conversation

@ZvikaZ
Copy link
Contributor

@ZvikaZ ZvikaZ commented Mar 4, 2021

If user presses SHIFT+. , it was interpreted as a dot.
That's because there is a kKeymapMatchPartial with
"Skip line" action, which caused execution of executeAction.
And executeAction replaced the original incomingEvent
with action, thus 'forgetting' the original ascii code.

The problem was seen (and fixed) on PQ1 (AGI) and SQ3 (SCI).

NOTE
I'm not certain that it's a good fix, as I don't fully understand executeAction, and all the implications of my change. It's only a single line change, but please review it carefully...

If user presses SHIFT+. , it was interpreted as a dot.
That's because there is a kKeymapMatchPartial with
"Skip line" action, which caused execution of executeAction.
And executeAction replaced the original incomingEvent
with 'action', thus 'forgetting' the original ascii code.

The problem was seen (and fixed) on PQ1 (AGI) and SQ3 (SCI).
@sev-
Copy link
Member

@sev- sev- commented Mar 14, 2021

Wouldn't that void potential key remapping when the ASCII value truly differs?

Loading

@ZvikaZ
Copy link
Contributor Author

@ZvikaZ ZvikaZ commented Mar 15, 2021

Yes, you're right (if I understand you correctly...)

I have checked SOMI before this PR, and with this PR. Before the PR, when pressing SHIFT+., it skips lines in dialog. With the PR, it doesn't. (of course, just . does skip lines). If it's bad - then this PR is wrong...

Another possibility - currently, the Skip line action is added for all engines (in engines\metaengine.cpp). Maybe it should be added only for engines supporting it? (SCUMM, and what else? I don't know. Grepping for Skip line or SKLI doesn't help)

Loading

@ZvikaZ
Copy link
Contributor Author

@ZvikaZ ZvikaZ commented Apr 12, 2021

I have opened a bug for the issue this PR tries to solve: https://bugs.scummvm.org/ticket/12410

Loading

@sev-
Copy link
Member

@sev- sev- commented Nov 1, 2021

I think the proper way to address this is to check the modifier key bitfield. This PR is not a correct approach and would hide the problem, unfortunately. Closing.

Loading

@sev- sev- closed this Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants