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

Info panel fixes and adding scrollbars #50892

Merged

Conversation

Brambor
Copy link
Contributor

@Brambor Brambor commented Aug 19, 2021

Summary

None

Purpose of change

Fixes #50851,
While fixing this issue I found (and fixed) other issues:

  • EFFECTS tab has priority over PROFICIENCIES tab.
  • PROFICIENCIES tab has disgusting scrolling & is scrolling with others (while it's not selected).
  • Tabs TRAITS, BIONICS, EFFECTS are missing scrollbar.
  • Left side of the BIONICS tab doesn't have a border.
  • Always start with space (| Abc instead of |Abc) - EFFECTS & PROFICIENCIES

Describe the solution

Describe alternatives you've considered

Since the scrollbar was only on one tab, remove it. But the scrollbar is useful, so I fixed it there and added it elsewhere.

Testing

Made save with lots of effects, bionic traits, mutations & proficiencies. Scrolling works, scrollbars too, everything is accounted for (nothing is under the screen as was in #50851). Resizing works.

Additional context

  • There are more opportunities for merging two or more similar blocks of code into a function.
  • c++17 would be nice.

Before:
looks_master

After:
loks_current

@Brambor Brambor marked this pull request as ready for review August 19, 2021 05:54
@Brambor Brambor force-pushed the info-panel-fixes-and-scrollbars branch from d3e091f to 558163e Compare August 19, 2021 05:56
@Brambor
Copy link
Contributor Author

Brambor commented Aug 19, 2021

My first c++ change in CDDA + first proper pull request.
So feedback is welecome. Did I overdo or underdo something?

src/player_display.cpp Outdated Show resolved Hide resolved
src/player_display.cpp Outdated Show resolved Hide resolved
@Qrox
Copy link
Contributor

Qrox commented Aug 19, 2021

To fix the border bug, change the x position and width of the border at src/player_display.cpp:1407.

 ui_bionics.on_screen_resize( [&]( ui_adaptor & ui_bionics ) {
     bionics_win_size_y = calculate_trait_and_bionic_height().second;
     bionics_win_size_y = trait_and_bionic_height.second;
     w_bionics = catacurses::newwin( bionics_win_size_y, grid_width,
                                     point( grid_width + 1,
                                            infooffsetybottom + trait_win_size_y + 1 ) );
-    w_bionics_border = catacurses::newwin( bionics_win_size_y + 1, grid_width + 1,
-                                           point( grid_width + 1,
+    w_bionics_border = catacurses::newwin( bionics_win_size_y + 1, grid_width + 2,
+                                           point( grid_width,
                                                    infooffsetybottom + trait_win_size_y + 1 ) );
    border_bionics.set( point( grid_width, infooffsetybottom + trait_win_size_y ),
                        point( grid_width + 2, bionics_win_size_y + 2 ) );
    ui_bionics.position_from_window( w_bionics_border );
 } );

And I think a similar change should also be made for the traits tab, in case the traits tab is longer than the skills tab.

src/player_display.cpp Outdated Show resolved Hide resolved
src/player_display.cpp Outdated Show resolved Hide resolved
src/player_display.cpp Outdated Show resolved Hide resolved
src/player_display.cpp Outdated Show resolved Hide resolved
@chrispikula
Copy link
Contributor

This is a really silly question, but is there any left-border bug on the third column? Of was that never an issue?

@actual-nh actual-nh added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. labels Aug 19, 2021
@Qrox
Copy link
Contributor

Qrox commented Aug 20, 2021

The third column already draws the left border according to the code.

@Brambor Brambor force-pushed the info-panel-fixes-and-scrollbars branch from 51df41e to b790807 Compare August 20, 2021 23:03
@Brambor Brambor force-pushed the info-panel-fixes-and-scrollbars branch from f7a34e7 to 8e21424 Compare August 20, 2021 23:43
@Brambor Brambor force-pushed the info-panel-fixes-and-scrollbars branch 2 times, most recently from 90d56ef to a6f47c0 Compare August 20, 2021 23:48
@Brambor Brambor force-pushed the info-panel-fixes-and-scrollbars branch from a6f47c0 to 378d154 Compare August 21, 2021 00:00
@Brambor
Copy link
Contributor Author

Brambor commented Aug 23, 2021

Edit: fixed that the scrollbar on encumbrance wasn't displaying.

Is that all @Qrox ?

@Qrox
Copy link
Contributor

Qrox commented Aug 24, 2021

Yeah, thanks for addressing the comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proficiencies slidebar doesn't show up when 1 item is off the screen.
5 participants