Skip to content

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.

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

Successfully merging this pull request may close these issues.

2 participants