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

Tos symbolic review #36

Closed
wants to merge 6 commits into from

Conversation

bbbradsmith
Copy link
Contributor

@bbbradsmith bbbradsmith commented Oct 4, 2024

At the request of Thomas Huth I have closed this PR and relocated it to my own fork here: bbbradsmith#1

It seems that my work in progress on the per-region TOS symbolic keyboard mappings was reorganized and integrated yesterday. For reference, this was that work in progress:

Comparison: https://github.com/bbbradsmith/hatari_hatari/compare/tos-symbolic-compare..bbbradsmith:hatari_hatari:tos-symbolic

I've reviewed the changes, and I've found a lot was omitted or altered, I think some errors were made. I have revised it and tested it to prepare this PR. My notes follow:

  • Trying to put a clear comment about every assignment that is arbitrary or unusual, indicating both what host layout it's from, and what ST key it's mapped to. List all regions involved, not just one.
  • I had avoided having a separate "default" keymap below US, because I was because I was trying to reduce the number of arbitrary decisions we have to maintain here, and because I was treating each layout as a complete set, not overlays to a fallback routine like this. However, if we wish to do this, I think that is OK, but I have some commentary and suggestions:
    • I've added comments to the individual region code to indicate which arbitrary additions to the default are being relied upon, so that if we need to maintain one we can see better what is affected.
    • The point of the "default" should be to reduce code, so I think when making arbitrary decisions we should pick the ST scancode which applies to the most maps, resulting in fewest overrides. This is not the same as deciding "only XXX language has this key, so the default must be for XXX host and XXX TOS", which leads to frustratingly arbitrary choices about which language priority, and that's precisely what inspired me to do this in the first place, so we wouldn't have to have arguments about whether German minus is more "default" than US minus.
      • Each language will be an override on the default, so the "default" set really only has code significance, and this arbitrary "default" choice will not impact the user in any way. Therefore: pick the scancode which will result in the fewest overrides.
      • Some keys only belong to one host language, however this is a symbolic SDL mapping from any host keyboard to a specific ST TOS language. So, even if a key belongs to one host only, it may still have valid mappings on other TOS languages. Therefore, defer to the fewest-overrides note above. This should apply to all keys with an SDLK_* semantic enumeration.
      • When a key belongs to only one host language and has no natural mappings to other ST TOS languages, there we can use that language's ST code as a default. This should be limited to just those extra symbols which have no SDLK semantic enumeration.
    • SDLK_EXCLAIM mapped to 0x09 with the comment "on azerty?". I believe this is only present on FR host, but the majority of TOS will map to 0x02. However, additional note about French: there are 3 keys on the FR ST keyboard which have no natural correspondence with the FR AZERTY host keyboard. Because of this there were 3 keys I assigned an arbitrary mapping, so that it is always possible to press every FR ST key from a standard FR AZERTY host keyboard. This key was assigned to 0x0D. Explanation here: https://listengine.tuxfamily.org/lists.tuxfamily.org/hatari-devel/2024/02/msg00050.html
    • SDL_QUOTEDBL present on FR host only, I believe, but the majority is 0x03.
    • SDLK_HASH present on DE, UK only, but the majority is 0x2B.
    • SDLK_DOLLAR present on CHFR, CHDE only, but majority is 0x05.
    • SDLK_AMPERSAND is not something I see on AZERTY, not sure what host has this, but the majority is 0x07.
    • SDLK_QUOTE the majority is 0x0C.
    • SDLK_PLUS 0x43 is the wrong semantic mapping, this is not KP_PLUS. Majority is 0x0D.
    • SDLK_MINUS majority is 0x35.
    • SDLK_SLASH majority is 0x08.
    • SDLK_SEMICOLON majority is 0x33.
    • SDLK_EQUALS majority ix 0x0B.
    • SDLK_GREATER majority is 0x60.
    • SDLK_QUESTION majority is 0x0C.
    • SDLK_UNDERSCORE majority is 0x35.
    • SDLK_KP_EQUALS is a numpad key with no natural mapping. 0x61 (ST Undo) was chosen with no comment, which feels like a mistake. Changing it to 0x72 as a duplicate of KP_ENTER but if this wasn't a mistake it should have an explicit comment indicating that it's for Undo.
    • Finally, the default fallback in Keymap_SetCountry must be a specific mapping, not a "default" that is an arbitrary combination of mismatched elements. I suggest US as the default, just because historically the default mapping was essentially US already.
  • Spanish international fallback differences, these are not in conflict so I suggest keeping both, but we may have tested slightly different layouts?
    • 161 was missing from yours, ¡ on the ES layout I tested, mapped to ST °§. It's an arbitrary mapping, but ¡¿ on a Spanish host keyboard has no natural mapping to ST which has them on 1 and 2, and ST °§ is otherwise inaccessible by any natural mapping.
    • 176 was missing from mine, not on the ES layout I tested (º in your comment), mapped to ST .
    • 186 was missing from yours, º on the ES layout I tested, mapped to ST .
  • French international differences seem to conflict with mine, and I can't reconcile your differences with my research. These all had valid key mappings for other keyboards already, but not the AZERTY French layout I tested. I can only seem to find them in reference for the BÉPO French variant, and not AZERTY, and all of them are redundant to the top row numeral keys anyway. I've added them instead as redundances in the FR symbolic map instead, and restored the conflicted fallbacks to more appropriate languages. Please comment on whether I have made an incorrect assumption here:
  • US mapping completely absent from code? This needed to be recreated. The "default" mapping had been changed significantly from a proper US mapping already before I instigated the "majority" rule.
  • DE mapping missing overrides: SDLK_QUOTE, SDLK_AT, SDLK_CARET, SDLK_UNDERSCORE, SDLK_BACKQUOTE, SDLK_LEFTBRACKET,SDLK_BACKSLASH, SDLK_RIGHTBRACKET, SDLK_CARET, SDLK_UNDERSCORE, SDLK_BACKQUOTE.
  • FR mapping:
    • SDLK_ASTERISK, 178, 249 as noted above.
    • Missing overrides: SDLK_PLUS, SDLK_PERIOD, SDLK_SLASH, SDLK_QUESTION, SDLK_BACKSLASH, SDLK_UNDERSCORE.
    • Override for 167 seems to be from CHFR host keyboard, noting it.
    • 224, 231, 232, 233 assumed as BÉPO only, added as FR only override rather than replacing important defaults belonging to other languages as noted above.
  • UK mapping missing overrides: SDLK_AMPERSAND, SDLK_COLON, SDLK_LESS, SDLK_BACKSLASH, SDLK_CARET.
  • ES mapping missing overrides: SDLK_QUOTEDBL, SDLK_AMPERSAND, SDLK_QUOTE, SDLK_SLASH, SDLK_COLON, SDLK_QUESTION, SDLK_CARET
  • IT mapping missing overrides: SDLK_HASH, SDLK_AT, SDLK_UNDERSCORE, SDLK_BACKQUOTE
  • SE mappings:
    • Missing overrides: SDLK_HASH, SDLK_ASTERISK, SDLK_UNDERSCORE, SDLK_BACKQUOTE
    • 252 override is valid, but not present on SE host keyboards. Making comment to note this.
    • Like French I added one arbitrary mapping for a SE host keyboard that doesn't have a natural ST mapping and would be unpressable. 167 SE § as ST .
  • CH mappings: SDLK_EXCLAIM, SDLK_HASH, SDLK_DOLLAR, SDLK_ASTERISK, SDLK_PLUS, SDLK_AT, SDLK_RIGHTBRACKET, SDLK_UNDERSCORE, SDLK_BACKQUOTE

I'll try to finish my research for FI, NO, DK, SA, NL, CS, HU so that I can compare against and review what you've added for some of those, and maybe supplement the rest if I can.

@bbbradsmith bbbradsmith force-pushed the tos-symbolic-review branch 2 times, most recently from b3afc96 to ea2e128 Compare October 5, 2024 07:44
…g and make an explicit US mapping and everything else is a difference from that.
@bbbradsmith
Copy link
Contributor Author

bbbradsmith commented Oct 5, 2024

It seemed like it should make this easier to follow and perhaps simpler to maintain, if instead of having a "default", instead have an explicitly US map and everything else is differences from that. This results in a few more lines of overrides, but there seems to be a lot less ambiguity about the meaning of each mapping this way.

Also applied the tos.h enums in Keymap_SetCountry as suggested by Eero Tamminen on the mailing list.

There is intent to change all of the symbolic maps to use the return idiom instead of temporary code variable, but leaving it out for the moment because it makes the diff view less useful.

@bbbradsmith
Copy link
Contributor Author

At the request of Thomas Huth I am closing this PR and relocating it to my own fork here: bbbradsmith#1

@bbbradsmith bbbradsmith closed this Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant