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

Introduce progress/loader window #22065

Merged
merged 9 commits into from
May 26, 2024

Conversation

AaronVanGeffen
Copy link
Member

@AaronVanGeffen AaronVanGeffen commented May 19, 2024

This PR introduces a dedicated ProgressWindow to denote loading progress in a playful, graphical way. This is to be used instead of the NetworkStatus info when progress has measurable increments.

Preloader scene (slowed down):

Screen.Recording.2024-05-21.at.19.11.05.mov

Joining a server:

Screen.Recording.2024-05-21.at.19.11.17.mov

Track style reel:

Screen.Recording.2024-05-20.at.14.59.32.mov

To do:

  • Show supports beneath the tracks
  • Use in the preloader scene
  • Use for downloading maps (multiplayer)
  • Crop and optimise PNGs before merging
  • Use for loading saves -> follow-up PR
  • Use for map generator -> follow-up PR

@Gymnasiast
Copy link
Member

Nice work! One though that I had: since most loading sessions will contain both status updates with just text, and updates with progress, we should maybe make sure that the respective windows look otherwise similar (i.e. use the same colours, be the same size, avoid text jumping around, ...). Doing so would make it look "calmer", for lack of a better word.

@AaronVanGeffen AaronVanGeffen added this to the v0.4.12 milestone May 20, 2024
@AaronVanGeffen AaronVanGeffen added changelog This issue/PR deserves a changelog entry. labels May 20, 2024
@AaronVanGeffen AaronVanGeffen force-pushed the progress-bars branch 2 times, most recently from e33877f to 3aa0f91 Compare May 20, 2024 19:07
Fixup: deal with -Wdeprecated-anon-enum-enum-conversion warning
@AaronVanGeffen AaronVanGeffen marked this pull request as ready for review May 20, 2024 19:42
@Basssiiie

This comment was marked as resolved.

@AaronVanGeffen
Copy link
Member Author

AaronVanGeffen commented May 21, 2024

@Basssiiie I've decreased the drawing canvas by 1px on every side, effectively adding a slight margin. Here's what it looks like now:

Screenshot 2024-05-21 at 19 07 30

The demo videos in the OP have been updated to reflect this change.

@AaronVanGeffen
Copy link
Member Author

AaronVanGeffen commented May 21, 2024

With the PNGs now cropped and optimised as well, I think this PR is now ready for review.

Copy link
Member

@Basssiiie Basssiiie left a comment

Choose a reason for hiding this comment

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

Had a quick test after the last changes and functionally it all LGTM! 😄

Copy link
Member

@Gymnasiast Gymnasiast left a comment

Choose a reason for hiding this comment

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

Looked over the code and found only a small thing to nag about. Will proceed to test next.

Copy link
Member

@Gymnasiast Gymnasiast left a comment

Choose a reason for hiding this comment

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

Testing revealed no bugs.

Personally, I would have put the train in the middle for progress bar with indefinite progress, rather than at ~30%, but I’m not willing to let that block my approval.

@AaronVanGeffen AaronVanGeffen force-pushed the progress-bars branch 2 times, most recently from 1740cf3 to c9de564 Compare May 26, 2024 09:38
@AaronVanGeffen
Copy link
Member Author

Personally, I would have put the train in the middle for progress bar with indefinite progress, rather than at ~30%, but I’m not willing to let that block my approval.

I am, because the intention was to draw it halfway. Turns out my code was drawing the tip of the vehicle at 50%, rather than centring it there. I've now addressed that, and the result is in 2f05dc9.

@Gymnasiast
Copy link
Member

OK, I’m not at home so I cannot test it easily. I’ll just trust you on this one, you can merge as far as I’m concerned ^^

@AaronVanGeffen AaronVanGeffen merged commit f5b5e45 into OpenRCT2:develop May 26, 2024
22 checks passed
@AaronVanGeffen AaronVanGeffen deleted the progress-bars branch May 26, 2024 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This issue/PR deserves a changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants