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

updating Keybindings of Keys (top right button ingame) to gridkeys #3678

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Hazy-uhyR
Copy link
Contributor

@Hazy-uhyR Hazy-uhyR commented Sep 4, 2024

When the game is set to use Gridkeys, the Keybindings page of Keys (button top right while ingame) shows an odd mix of Grid and Legacy keys. This update makes all the listed keys to be based on Gridkeys, although the odd mix problem persists if someone is using Legacy or off-grid custom keys.

As Gridkeys is now the default, this is still better, even if the "odd mix" problem persists for some users. Ideally, the whole page would update based on the actual keybinds of the user, but setting that up seems way beyond me.

Work done

  • Changed "Selecting build orders" to Gridkeys.
  • Added "select visible+similar" (Q with gridkeys), "command_skip_current" (N) and "command_cancel_last" (ctrl-N).
  • Removed sharedialog and move.
  • Changed "variable names" in gui_keybind_info.lua and in each interface.json file. Also did some cleanup.
  • Capitalized descriptions of unit orders in the english interface.json while I am at it.
  • Many minor fixes.
  • Updated disclaimer.

Addresses Issue(s)

Test steps

Start up the game and check out the Keybindings page of Keys (button top right).

To check out the "odd mix" problem, check the Keybindings page before and after switching between Grid and Legacy keys. When on Legacy keys -with this update running - note that the 3rd column has a bunch of wrong infos.

Screenshots:

BEFORE:

keybindingsweirdmix

AFTER:

image

Changed variable names of some things, and updated the descriptions.

Corrected selection keys.

Capitalized some order descriptions.
Added "select visible + similar" (Q).
Reordered things a bit.
Few minor fixes.
v cute
@Hazy-uhyR
Copy link
Contributor Author

Hazy-uhyR commented Sep 5, 2024

Alright, I'm content with it now. Imma light the signal.

@badosu I assume this is also your thing. How does this look? Any suggestions before a merge?

@badosu
Copy link
Contributor

badosu commented Sep 5, 2024

@badosu I assume this is also your thing. How does this look? Any suggestions before a merge?

Honestly I can't talk a lot about this as I haven't kept up with development (and gameplay) of BAR for a long time now.

What I can say though is that we shouldn't use translation keys for keys that are positional (as in using scancodes) instead of 'labeled' (as in using keycodes), so for example where it would be q in qwerty should be z in azerty.

To mitigate this issue, I've created a helper function that I use in some other places to print the correct key for an action.

See sanitizeKey (for translating from qwerty to the keyboard layout chosen) and this.

I believe the action hotkeys include is quite limited it indexes by command which is the action name (without the args), one would have to index by line or command + extra (and some sanitization) so you can distinguish between gridmenu_category 1 and gridmenu_category 2.

@Hazy-uhyR
Copy link
Contributor Author

Hazy-uhyR commented Sep 5, 2024

I'm planning to finish this with the same (less than ideal) methods as up until now; hopefully have it approved and merged; and probably turn badosus post into an issue for someone other than me to handle.

I could probably sort out the layout etc. as long as getActionHotkey() gets expanded by someone else to look for args. All that stuff should be simple enough for me.

The "Selecting Build Orders" part is significantly different between gridkeys and legacy, so something should be done to handle that as well to really complete this feature.

@badosu
Copy link
Contributor

badosu commented Sep 6, 2024

As an update to the already hardcoded key help this is ok.

The real fix would be a list of actions to show unhardcoded bindings and the corresponding text. And of course the hardcoded texts for things that aren't actions per se

Update interface.json

HopefulFinal

remove sharedialog and move
rename "Selecting Build Orders" variables and update to gridkeys proper
rename sameTypeOnscreen -> sameTypeVisible
add commandSkipCurrent and commandCancelLast
update disclaimer
keybinds -> keybinding
@Hazy-uhyR
Copy link
Contributor Author

Hazy-uhyR commented Sep 8, 2024

OP updated with new image. I consider my work done now, and I am happy with it, but I am open to further suggestions / fixes.

I consider this a good upgrade from what we have, for the few people who actually look at this page.

@Hazy-uhyR
Copy link
Contributor Author

Hazy-uhyR commented Sep 8, 2024

Should be ready to merge.

@WatchTheFort WatchTheFort marked this pull request as draft September 12, 2024 20:38
Copy link
Member

@WatchTheFort WatchTheFort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only change translation entries in the English source files, Transifex will handle updating the other language files.

@@ -298,8 +298,8 @@
},
"keybinds": {
"title": "Keybinds",
"disclaimer": "These keybinds are set by default. If you remove/replace hotkey widgets, or use your own uikeys, they might stop working!",
"howtochangekeybinds":"To change them: in Settings/Control tab, set Keybindings to Custom to create the BAR/data/uikeys.txt file.\nEdit this file and type /keyreload in chat to reload them.",
"disclaimer": "These keybinds are set by default. If you switch to a keybind preset other than Grid, keys in the 3rd column will be inaccurate!",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong information is worse than no information. Either what is shown to the player is correct or it is not shown at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review.

This update is meant to be better than what we have, but not perfect. I don't have the capabilities to make it "perfect." Can we do this as a band-aid until more capable people make the tools needed for "perfection?"

"Perfection" would require getActionHotkey to accept arguments "so you can distinguish between gridmenu_category 1 and gridmenu_category 2" (#3678 (comment)), and the ability to switch the display between gridmenu and the "old" non-gridmenu way of doing things - for the "Selecting build orders" part, top right, specifically. Both of those issues are beyond my capabilities.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, getActionHotkey needs to be able to accept from 0 to 2 arguments so that "gridmenu_key 1 1" also can be can be distinguished from "gridmenu_key 1 2" etc.

Copy link
Contributor Author

@Hazy-uhyR Hazy-uhyR Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, I can remove the "Selecting build orders" part (except rotate keys) and half of the "Group selection" part (the "select" lines, specifically). By that point, all information on the page would be correct no matter the keybind preset, afaik.

Please confirm that I should just do that.

"disclaimer": "These keybinds are set by default. If you remove/replace hotkey widgets, or use your own uikeys, they might stop working!",
"howtochangekeybinds":"To change them: in Settings/Control tab, set Keybindings to Custom to create the BAR/data/uikeys.txt file.\nEdit this file and type /keyreload in chat to reload them.",
"disclaimer": "These keybinds are set by default. If you switch to a keybind preset other than Grid, keys in the 3rd column will be inaccurate!",
"howtochangekeybinds":"To change them: in Settings/Control tab, set Keybindings to Custom to create the BAR/data/uikeys.txt file.\nEdit this file and type /keyreload in chat to reload them. You can find help for keybinding in #keybinding-support on Discord.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The game must stand on its own, we cannot require the player to consult any external reference.

@Hazy-uhyR
Copy link
Contributor Author

Note to self: Add selectcomm. Reorder selection keys (maybe).

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.

3 participants