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

Improve dialogue navigation including mouse support & history scrolling #61447

Merged
merged 8 commits into from Nov 29, 2022

Conversation

ZeroInternalReflection
Copy link
Contributor

Summary

Interface "Improve dialogue navigation including mouse support & history scrolling"

Purpose of change

As noted in #54756, the conversation history is not scrollable. This is particularly a problem on small screens, since large NPC dialogue lines might not fit in the available space, leaving information inaccessible to the player.
In addition, the dialogue window does not have any mouse support.

Describe the solution

  1. Convert the dialogue history to a scrolling_text_view, allowing scrolling with angle brackets </>, the mousewheel when the mouse is in the window, and by clicking-and-dragging the scrollbar (this last one is not particularly useful on small screens, but...)
  2. Pull display and navigation handling of the dialogue responses out of dialogue_window and into a new helper class ("multiline_list"), and add the following navigation features:
    a. Mousewheel scrolling (when mouse is in window)
    b. Mouseover selection/mouse-click confirmation
    c. Scrollbar click-and-drag
    d. Scrolling of the list by 'hovering' over the top/bottom line of the list (Since this is event-based rather than time-based, you need to wiggle the mouse a bit, but I think it's still fairly intuitive)

Describe alternatives you've considered

  • Leaving drawing of the conversation history unchanged and separately adding scrolling
    This seemed likely to duplicate a lot of code
  • Adding mouse support directly into dialogue_window
    Adding intuitive mousewheel scrolling and mouseover selection to the dialogue response list required a lot of reworking the display code for the responses, and I really don't want to duplicate that.

Testing

Tested scrolling of conversation history with keyboard:

DialogueHistoryScrollingAngleBrackets.mp4

Tested behaviour on resizing:

DialogueResize.mp4

Tested mousewheel scrolling:

DialogueScrollingMousewheel.mp4

Tested behaviour when examining computers:

DialogueWithComputer.mp4

Tested scrollbar dragging and mouseover scrolling:

DialogueScrollbarDragAndMouseoverScroll.mp4

I have tested in both Tiles and Curses. Mouseover behaviour does not work in Curses, but that's not a surprise. Mousewheel scrolling works in Curses, but only if compiled with a newer version of Curses than is officially supported. Keyboard scrolling and mouse-click selection work as expected in Curses.

I have tested in several other languages, and everything seems to work as expected.

Additional context

  • I believe this resolves the original issue for They See Me Scrollin', They Hatin' #54756, but the discussion lists several other screens that need similar scrollability. I'm inclined to leave it open until that list is completed.

  • A local clang-tidy run produced some error messages that baffled me and received no clarification on the development Discord. Let's find out if the automated clang-tidy run produces the same results.

  • There's a bunch of infrastructure in multiline_list that is not used for dialogue_window (e.g. using templates to create entries, activate_entry, not having prefixes, etc. ). I can pull that out if required, but it's setup work for using it elsewhere. In particular, I'm working on adding this to several of the character creation screens to add mouse support/allow for multiline professions/backgrounds/etc.. The test below requires an additional +35/-80 line change (Polish shown because they have some nice long Profession names):

Curses-Professions-Polish.mp4

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Oct 3, 2022
@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2022

This pull request introduces 1 alert when merging 9b5414f into e39079b - view on LGTM.com

new alerts:

  • 1 for Use of c-style math functions

@lgtm-com
Copy link

lgtm-com bot commented Oct 4, 2022

This pull request introduces 1 alert when merging a0c7cf8 into b97d3ea - view on LGTM.com

new alerts:

  • 1 for Declaration hides parameter

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Oct 4, 2022
@leemuar
Copy link
Contributor

leemuar commented Nov 24, 2022

Great work!

src/color.cpp Outdated Show resolved Hide resolved
@Night-Pryanik
Copy link
Contributor

Tested right now, and all new stuff worked as it should. I found no bugs or unintended behavior.

Co-authored-by: Anton Burmistrov <Night_Pryanik@mail.ru>
@github-actions github-actions bot added the NPC / Factions NPCs, AI, Speech, Factions, Ownership label Nov 29, 2022
@I-am-Erk I-am-Erk added this to In Progress in 0.G Stable Tracker via automation Nov 29, 2022
@I-am-Erk I-am-Erk merged commit 08ef66d into CleverRaven:master Nov 29, 2022
0.G Stable Tracker automation moved this from In Progress to Done Nov 29, 2022
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` Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants