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

Dynamic compact aim window pt.1 #37715

Merged
merged 13 commits into from
Apr 27, 2020

Conversation

Kilvoctu
Copy link
Contributor

@Kilvoctu Kilvoctu commented Feb 5, 2020

Summary

SUMMARY: Interface "Dynamic compact aim window pt.1"

Purpose of change

Have the aim window dynamically decrease in width (from 55 to 34) when using "compact" and "labels-narrow" sidebars to keep in theme with having compact UI.
This is part 1, which only handles the "Bars" accuracy display style. "Numbers" is not affected.

Describe the solution

  • Include panel manager to watch for which sidebar style is active.
  • When the manager detects that "compact" or "labels-narrow" sidebar is active:
    ~ Sets aim window to 34 width.
    ~ Remove keybinds help text. Instead, "[?] for help" is displayed at the bottom.
    ~ Put "Targets: " on a new line.
    ~ Shorten Symbols help text.
    ~ Right-align "Moves to fire".
    ~ Shorten aim confidence bars to provide blank space on right-hand side.
    ~ Put moves to fire number in said blank space.
  • Otherwise, the aim window will look normal for the other sidebars.

Describe alternatives you've considered

Creating a new accuracy display style. Implementation was janky and it's another option to manage.
Keeping keybinds help text visible. I feel like it's unnecessary.
Making compact aim window the same width as the compact sidebars (32 width). A little too tight.
Changing aim window height. Didn't see a need.

Testing

Game compiles and loads.
Open aim window for each sidebar option in sequence with Bars style and repeat with Numbers style.
Move aim cursor around and steady my aim (Aim confidence adjusts accordingly).
Aim at an NPC and some monsters.
Aim via throwing. Aim via turret.
Aim while application is set to maximized terminal size, as well as minimum possible terminal size.
Press ? to show keybind help window.

Quick video showing aim window and swapping sidebar options:
https://i.imgur.com/yd9EtQq.mp4

Additional context

Comparison screenshots, taken at minimum terminal size:
barsA
barsB

Reiterating that Numbers accuracy display should look the same at the moment. I didn't know how to redo its UI, but ifreund provided an attractive mockup of how it'd fit into the window:
mockup
As approaching this change is quite different from the approach for Bars, I'd decided to save Numbers for part 2.

If these current changes are deemed to be acceptable, this PR can be safely merged on its own once out of feature freeze, if I have not completed part 2 by then.
Afterward, if it all looks good, we can consider applying some of the changes to the normal aim window, which I feel like could be trimmed down to classic sidebar width (44 width).

@AMurkin
Copy link
Contributor

AMurkin commented Feb 5, 2020

Truly dynamic width would if it will depend on the content.
When the text will be translated then it will change its width. And the handmade window width will not be perfect.
It's much more complicated but would be ideal IMO.

@ZhilkinSerg ZhilkinSerg added [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. Ranged Ranged (firearms, bows, crossbows, throwing), balance, tactics labels Feb 5, 2020
@Kilvoctu
Copy link
Contributor Author

Kilvoctu commented Feb 5, 2020

"Dynamic" is just what I'm calling the PR. I can rename it to "automatically compact aim window" or something similar if it's really a problem. I'm not good at making titles.

For translators, I did try to keep them in mind to the best of my knowledge when doing the compact UI, giving the window a couple more width than bare minimum and removing some unnecessary stuff.
That said, I can see possibly a couple places where it may be a problem. (There's probably issues in the code for ease of translation, too.)
Could you provide an example of an issue? The window is 33 characters long for text, and this UI change does provide a bit more vertical space to work with. The goal would be that essential information would not be cut off. I'll see what I can do to move some stuff around, if needed.

Lastly, an aim window that truly dynamically adjusts depending on window content is outside the scope of this PR. The purpose is a compact aim window for compact sidebars. If the window widens because the player is "Firing ++ Very Verbose Automatic Rifle of Lengthy Name+5 *"", it defeats this purpose.

@AMurkin
Copy link
Contributor

AMurkin commented Feb 5, 2020

I've compiled it and for now I can see:

For example in Russian:

  • the lines above the bars are overlap currently
  • the line with a legend (Symbols) doesn't not fit

aim-bars

Moves to fire is not translated yet but in Russian it will not be shorter than this.

@Kilvoctu
Copy link
Contributor Author

Kilvoctu commented Feb 5, 2020

For "Moves to fire", I can change that to just "Moves", which feels like an acceptable shorthand. It's used in other UI such as reloading ammo. Will also be consistent with the tentative Numbers accuracy display style.

As for the line with a legend (Symbols), how much character width is needed? I can cut down 1-4 extra spaces I think, but if a lot more character width is needed, I can try something else.

@olanti-p
Copy link
Contributor

olanti-p commented Feb 5, 2020

@AMurkin The first thing feels like it needs a better representation no matter the width. Something like

Выстрелить с прицеливанием

  • Текущим...............Moves to fire
  • Обычным.............Moves to fire
  • Тщательным........Moves to fire
  • Точным..................Moves to fire

Russian 'Прицеливание' is much longer than english 'aim', it looks like a mess with any width IMHO.
And 'Moves to fire' -> 'Движений' / 'Стоимость движ.' ? Looks manageable.

@Kilvoctu the last word is currently translated as 'Скользящее попадание', so 14 characters short. Probably can be shortened to 'Вскользь' / 'Скользящее' / 'Царапина', tbh

@Kilvoctu
Copy link
Contributor Author

Kilvoctu commented Feb 6, 2020

Shortened "Moves to fire" to "Moves" and color-coded it. When translated to Russian, this should help differentiate so it doesn't look like one sentence. I think the window should be wide enough for "Тщательным Прицеливание Движений", but probably not if "Стоимость движ" is used, unless "Прицеливание" is omitted.

For symbols legend, I used a function to count the amount of characters, then have the line wrap to next line if exceeding window width. It doesn't look beautiful but is atleast readable. See below.
If anyone knows how to make it wrap starting from the symbol or another idea, please make a suggestion.

Capture

@olanti-p
Copy link
Contributor

olanti-p commented Feb 6, 2020

If anyone knows how to make it wrap starting from the symbol or another idea, please make a suggestion.

Something like that, I think?

if (display_type != "numbers") {
    int column_number = 1;
    if (!(panel_type == "compact" || panel_type == "labels-narrow")) {
        auto label = gettext("Symbols:");
        mvwprintw(w, point(column_number, line_number), label);
        column_number += strlen(label) + 1; // 1 for whitespace after 'Symbols:'
    }
    for (const confidence_rating& cr : confidence_config) {
        auto label = pgettext("aim_confidence", cr.label.c_str());
        auto text = string_format("<color_%s>%s</color> = %s ", cr.color, cr.symbol,
            label);
        auto line_len = strlen(label) + 4; // 4 for '# = ' part
        if ((window_width - 1 - column_number) < line_len) { // 1 for right border
            column_number = 1;
            line_number++;
        }
        print_colored_text(w, point(column_number, line_number), col, col, text);
        column_number += line_len;
    }
    line_number++;
}

I didn't test it, but the idea is to create 'symbol = explanation' string for each confidence_rating and try to fit it onto the current line. On wide screen, in theory, all strings will fit onto same line

src/ranged.cpp Outdated Show resolved Hide resolved
src/ranged.cpp Outdated Show resolved Hide resolved
src/ranged.cpp Outdated Show resolved Hide resolved
src/ranged.cpp Outdated Show resolved Hide resolved
@ifreund
Copy link
Contributor

ifreund commented Apr 24, 2020

@Kilvoctu this has grown some merge conflicts after being on hold so long due to 0.E. Could you please resolve them? I'll put this on my merge list pending conflict resolution.

Make aim window dynamically shrink in size whenever sidebar is set to compact
Right align text using right_print instead of janky solution
Add conditionals to exclude Numbers indicator style, to work on later. Also fix minor bugs.
fix translation markers and clean up some code
Minor refactor of aim strings, change "moves to fire" to "moves", detect length of "symbols" line and wrap if necessary
@Kilvoctu Kilvoctu force-pushed the dynamic-compact-aim-window-pt1 branch from 3bcc222 to 709918e Compare April 26, 2020 17:49
Fix conflicts by adapting code into new existing code
remove the unnecessary includes that I accidentally added during fixing conflicts
@Kilvoctu
Copy link
Contributor Author

Kilvoctu commented Apr 26, 2020

The conflicts have been resolved. There've been a few changes to the ranged.cpp file over the last few months so hopefully I didn't break anything when re-adapting my code into it... It compiled, and I tested the panels under various sidebar styles. Appears fine, unless I missed something.
Some of what exists here should be cleaned up a bit when part 2 of the project is implemented (I've had that code sitting around for a few months too)

Copy link
Contributor

@ifreund ifreund left a comment

Choose a reason for hiding this comment

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

Thanks!

@ifreund ifreund merged commit 5926e6e into CleverRaven:master Apr 27, 2020
@Kilvoctu Kilvoctu deleted the dynamic-compact-aim-window-pt1 branch April 28, 2020 00:20
Drewscriver pushed a commit to Drewscriver/Cataclysm-DDA that referenced this pull request Apr 30, 2020
* dynamic aim window

Make aim window dynamically shrink in size whenever sidebar is set to compact

* fix astyle regressions

* use right_print

Right align text using right_print instead of janky solution

* Set up to split project into two parts

Add conditionals to exclude Numbers indicator style, to work on later. Also fix minor bugs.

* Right align "moves to fire" text and remove hotkeys display

* fix translation markers

fix translation markers and clean up some code

* text handling

Minor refactor of aim strings, change "moves to fire" to "moves", detect length of "symbols" line and wrap if necessary

* revise text wrapping

* commit suggestions

* use correct types

* Fix conflicts

Fix conflicts by adapting code into new existing code

* astyle

* trim includes

remove the unnecessary includes that I accidentally added during fixing conflicts
@olanti-p olanti-p mentioned this pull request Aug 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. Ranged Ranged (firearms, bows, crossbows, throwing), balance, tactics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants