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

Fix MOTD/Credits scrolling bug #58140

Merged
merged 4 commits into from
Jun 6, 2022
Merged

Fix MOTD/Credits scrolling bug #58140

merged 4 commits into from
Jun 6, 2022

Conversation

Ezrashaw
Copy link
Contributor

@Ezrashaw Ezrashaw commented Jun 4, 2022

Summary

Bugfixes "Fix MOTD/Credits scrolling bug"

Purpose of change

Fixes bug no. 1 in #58086, scrolling past the very bottom in either the credits or MOTD submenu would continue to scroll internally (but not visually) requiring extra key pushes to begin to scroll up.

Describe the solution

First PR with cpp code so might not be 'correct', need feedback.
We calculate the end of both the MOTD text and Credits text and simply cap the 'sel_line' variable at that position.

Describe alternatives you've considered

None.

Testing

Builds and runs, scrolling now works as intended.

Additional context

  1. We might be able to to this better by making the 'mmenu_motd' and 'mmenu_credits' be vectors of strings instead of just a single newline separated string, as far as I can figure this could require changes to 'display_text' (or maybe even the internal API functions) to accept vectors of strings.
  2. Pretty sure that that this check is redundant now and a situation where this is actually invoked cannot exist; this check is basically now just done elsewhere.
diff --git a/src/main_menu.cpp b/src/main_menu.cpp
index d9535633df..8f2847a858 100644
--- a/src/main_menu.cpp
+++ b/src/main_menu.cpp
@@ -515,12 +515,6 @@ void main_menu::display_text( const std::string &text, const std::string &title,
     const auto vFolded = foldstring( text, width );
     int iLines = vFolded.size();
 
-    if( selected < 0 || iLines < height ) {
-        selected = 0;
-    } else if( selected >= iLines - height ) {
-        selected = iLines - height;
-    }
-
     fold_and_print_from( w_text, point_zero, width, selected, c_light_gray, text );
 
     draw_scrollbar( w_border, selected, height, iLines, point_south, BORDER_COLOR, true );

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions labels Jun 4, 2022
Copy link
Member

@dseguin dseguin left a comment

Choose a reason for hiding this comment

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

Just a minor correction to appease the clang-tidy test. Looks fine to me otherwise 👍

src/main_menu.cpp Outdated Show resolved Hide resolved
src/main_menu.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 4, 2022
Co-authored-by: David Seguin <[email protected]>
@github-actions github-actions bot added the astyled astyled PR, label is assigned by github actions label Jun 4, 2022
@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 5, 2022
@dseguin dseguin merged commit 293c808 into CleverRaven:master Jun 6, 2022
@Ezrashaw Ezrashaw deleted the menu_scrolling branch June 6, 2022 04:13
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 6, 2022
bombasticSlacks pushed a commit to bombasticSlacks/Cataclysm-DDA that referenced this pull request Jun 10, 2022
* Fixes menu scrolling bug in CleverRaven#58086

* Styling corrections

Co-authored-by: David Seguin <[email protected]>

* Update src/main_menu.cpp with style chnges

Co-authored-by: David Seguin <[email protected]>

* Remove redundant check from 'display_text'

Co-authored-by: David Seguin <[email protected]>
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 <Bugfix> This is a fix for a bug (or closes open issue) [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