Skip to content
New issue

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

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

Already on GitHub? # to your account

Backends: Allegro: Don't call AddInputCharacter if the pressed key has no character. #3252

Merged
merged 1 commit into from
May 20, 2020
Merged

Backends: Allegro: Don't call AddInputCharacter if the pressed key has no character. #3252

merged 1 commit into from
May 20, 2020

Conversation

Espyo
Copy link
Contributor

@Espyo Espyo commented May 20, 2020

  • Commit ef13d95 moved the logic of checking for invalid Unicode codes from each backend to Dear ImGui's AddInputCharacter function proper.
  • Commit c8ea0a0 then removed that logic, replacing it with something that inserts a question mark-like character in the case of an invalid Unicode code.
  • As a result, after these two commits, if an Allegro keypress has no associated character (arrow keys, End, Page Up, etc.), it will send zero with nobody to filter it out.
  • With this fix, no-character inputs won't even call AddInputCharacter to begin with.
  • Tested with arrow keys, Page Up, etc., tested with regular character keys, and tested with some Unicode symbols directly from the keyboard, like €.

…s no character.

- Commit ef13d95 moved the logic of checking for invalid Unicode codes from each backend to Dear ImGui's AddInputCharacter function proper.
- Commit c8ea0a0 then removed that logic, replacing it with something that inserts a question mark-like character in the case of an invalid Unicode code.
- As a result, after these two commits, if an Allegro keypress has no associated character (arrow keys, End, Page Up, etc.), it will send zero with nobody to filter it out.
- With this fix, no-character inputs won't even call AddInputCharacter to begin with.
- Tested with arrow keys, Page Up, etc., tested with regular character keys, and tested with some Unicode symbols directly from the keyboard, like €.
@ocornut
Copy link
Owner

ocornut commented May 20, 2020

Thanks @Espyo for the thoughtful commit and explanation.

Looking at c8ea0a0 again, I feel like we should add a != 0 test in the lower-level functions AddInputCharacter(). I guess we were focusing on the "other" invalid inputs but since lows of back-ends have been passing value to AddInputCharacter() filtered it makes sense to add the test back as ef13d95 intended.

I don't mind also leaving the if != 0 in Allegro back-end because that event can query both types of data.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants