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

GUI: Add scrolling #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

GUI: Add scrolling #118

wants to merge 1 commit into from

Conversation

er2off
Copy link

@er2off er2off commented Apr 9, 2024

Fixes #107

@iProgramMC
Copy link
Member

Although stuttering occurs when scrolling through the world select list, it also does when dragging, so this isn't a problem with this PR.


void ScrolledSelectionList::handleScroll(bool down)
{
float diff = 5.0f * (down ? -1.0f : 1.0f);
Copy link
Member

Choose a reason for hiding this comment

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

Feels weird. Like, when you scroll up, and then down, it seems to "continue" scrolling in the direction I was first scrolling in. Maybe it should just instantly set the velocity.

Also I think you should add a timeout of a few ms where the world select is just loose (as if you were holding a finger on the scroll area)

Copy link
Author

Choose a reason for hiding this comment

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

This comment is for RolledSelectionList? Problem doesn't appear with ScrolledSelectionList.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, oops... Comment is related to RolledSelectionList.

Copy link
Member

@iProgramMC iProgramMC left a comment

Choose a reason for hiding this comment

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

GitHub forces me to write something here as well. All's good, except for that one issue I just mentioned.

{
float diff = 5.0f * (down ? -1.0f : 1.0f);
field_34 = field_30 = field_30 + diff;
field_28 = 0;
Copy link
Contributor

@uniformization uniformization Apr 12, 2024

Choose a reason for hiding this comment

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

you should probably rename the unnamed fields that you use, only a suggestion though

Copy link
Author

Choose a reason for hiding this comment

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

Agree with this but I think these classes should be rewritten instead.

Copy link
Member

@iProgramMC iProgramMC Apr 12, 2024

Choose a reason for hiding this comment

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

Agree with this but I think these classes should be rewritten instead.

Disagree. These classes implement the base of scrollable list views. I don't see how they need to be rewritten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't scroll in the options menu
3 participants