OpenTTD
OpenTTD copied to clipboard
OpenTTD
[WIP] Change: add support for next/previous railtype global hotkeys
This patch adds support for next/prev railtype global hotkeys which are basically autorail + special modifier which selects previous or next available railtype.
Such behaviour is useful for players who use PURR, Useless Tracks and similar NewGRFs for quick selection.
I use PURR with semantic tracks coloring (e.g. white purr for waiting bays, red purr for prio zones etc).
Current patch binds this feature to GLOBAL+[ and GLOBAL+].
UPD If user explicitly selected some widget in railway build toolbar it won't switch to autorail.
Signed-off-by: Konstantin Gribov [email protected]
This needs to be tested with keyboard layouts where the [ ] characters are on AltGr combinations with the number row. (Several continental European layouts.)
@nielsmh, I tested it on Linux w/ SDL2 and default German layout where [/] are on AltGr+8/9. Hotkeys work as expected when PR #7850 is applied, on same keys where [/] are on US layout.
Some additional thoughts about this feature. Currently it switches to autorail but better in UX terms would be to stay with same chosen widget if one is selected.
Then you can just switch railtype on the fly with say converse or depot instead of switching railtype and choosing converse/depot/whatever after that. Behavior when rail building tools are inactive should stay the same: choose railtype and activate autorail.
@LordAro, ok, I'll fix things mentioned above. In case of code style I just did by example and got smacked by CI once)
Anyway, I will mark this as work-in-progress since I wish to change some logic (see https://github.com/OpenTTD/OpenTTD/pull/7851#issuecomment-562877015)
hi @grossws : exactly a year later, as if I planned this! (I did not :D). I was wondering if you are still interested in seeing this PR through? It seems you have a few comments open that needs resolving .. would be cool if you could! Tnx!
Is it extensible to roadtypes also? :D No problem if not, just curious.
@TrueBrain, I can possibly look into this and update PR but after Jan, 7th when I return from vacation. Quite eventful year it was ,)
@andythenorth, didn't look into that, but it may be possible if it has similar implementation. I don't recall different road types at all, could you point what additional GRFs should I have to check that?
Possible modification: instead of adding RailToolbarWidgetModifiers enum just add prev/next_railtype to RailToolbarWidgets with explicit constants like 0xfd/0xfe.
Initially I put them into separate enum because I thought that it would be confusing to have WID_RAT_* enum values not represented in toolbar as a button. But caption already exists, so maybe it's not the case.
Also, GHK_MOD_RAT_* became simple special hotkeys and not actually modifiers for WID_RAT_AUTORAIL, another point to putting them into RailToolbarWidgets enum.
@ldpl, I'll comment on most of the review comment here.
I [snip] think this implementation is quite messy and can cause some problems in the future.
Quite possible. I mark PR as [WIP] again.
Also, I'd expect these hotkeys to loop around tbh, not just stop at first/last railtype.
It was deliberate. First time I tried with wrapping around it was no as good ux as with current behavior. When it saturates at simple rail/last available rail it's easy to press [/] several times and return back to required from there without looking at widget at all.
Thanks for your review and looking forward for your reply ,)
@grossws Are you still interested in working on this? I'm going through the PR backlog to find low-hanging fruit and this is on my list.
@2TallTyler, yeah. I'd like some answers for @ldpl's review above about constant naming and in which file implementation should live.
Likely I'll have a bit of time to update the PR and fix issues mentioned above this or next weekend
I'd look at other widget enums for naming conventions, and I've seen other hotkey implementations (#10110) stay in the corresponding *_gui.cpp file so I'd imagine that would be fine.