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

Don't hardcode help text: Zones Manager #79286

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

Kilvoctu
Copy link
Contributor

@Kilvoctu Kilvoctu commented Jan 22, 2025

Summary

SUMMARY: Interface "Don't hardcode help text: Zones Manager"

Purpose of change

Help text should not be hardcoded as, if end user changes their keybinds, it will cause discrepancies in the UI.
This PR targets the Zone Manager window and partially addresses #78986

Describe the solution

Rewrite zones_manager_shortcuts to pull the currently set keybind for each command, and display the help text in window using a simple loop.

Describe alternatives you've considered

I've found in the codebase many bespoke approaches to displaying help text... I ended up using the Crafting window as a reference, because it can place the key inside the name of the help text. It also wraps nicely. I simplified and cleaned up the code when implementing it for Zone Manager window.

Currently Show all / Hide distant is the help text for SHOW_ALL_ZONES, and it highlights the respective text in yellow for the toggle. Too convoluted for what it's worth, in my opinion, when the game can communicate this information using just one extra line of code.

Testing

Game compiles and loads.
Open Zone Manager window, all help text appear correctly in the help text area.
Toggle SHOW_ALL_ZONES and observe that the help text automatically updates to text to show Showing all zones or Hiding distant zones, respectively.
Change keybind for ADD_ZONE and observe that the help text automatically updates the text.

Additional context

There is a minor bug in which MOVE_ZONE_UP and MOVE_ZONE_DOWN is reversed. i.e., Pressing the key to move a zone down, will instead move it up, and vice versa. While this is a bit out of scope, I fixed it.

The Zone Manager window is always the same width, regardless of terminal size, so creating solution and testing was relatively simple.

Screenshots

The zones are far right now, so they aren't appearing in the list
Screenshot 2025-01-21 212043

After pressing s to toggle SHOW_ALL_ZONES. The zones now appear and the help text updated accordingly.
Screenshot 2025-01-21 212049

After I changed the keybind for "Add" to o
Screenshot 2025-01-21 202922

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jan 22, 2025
also squash in
rename "Disable zone" to "Disable" for consistency
fix move zone up/down being reversed
github bot suggestions

Co-Authored-By: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 22, 2025
@GuardianDll GuardianDll merged commit 77480fd into CleverRaven:master Jan 22, 2025
23 of 28 checks passed
@Kilvoctu Kilvoctu deleted the help-text branch January 22, 2025 09:56
Kilvoctu added a commit to Kilvoctu/Cataclysm-DDA that referenced this pull request Jan 23, 2025
The help text wrapping looked fine on my upstream merged PR CleverRaven#79286, but with gamepad glyphs, I see text does not wrap on 5th line... for some reason.

Move the help text up 1 line, so now it has 6 lines to work with. Adjust the boundary so the text looks nicer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants