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 financial and park history graphs #22414

Merged
merged 7 commits into from
Aug 5, 2024

Conversation

mrmbernardi
Copy link
Contributor

@mrmbernardi mrmbernardi commented Jul 29, 2024

This PR fixes #22396, #22395, #22307, and #11285.

I should also be able to make the following improvements:

  • Make the graphs resizeable
  • Improve the readability of the hover tooltip
  • Make the guest count graph display numbers above 5,000
  • Improve the algorithm that determines the Y axis labels

I originally wanted to bring the hover tooltips to the park rating and guests in park graphs. However, the values are compressed down to 8 bits and it would require modifying the save format to change this. Since the values aren't even accurate anyway, I also figured there would be no point bringing the resize feature to these graphs either.

This makes STR_GRAPH_AXIS_LABEL and STR_FINANCES_FINANCIAL_GRAPH_CASH_VALUE obsolete in the string tables.

Things to consider in the future which are now easier with the new code:

  • Allowing the user to change the amount of plotted points
  • Bringing resizeability and tooltips to the park graphs

@mrmbernardi
Copy link
Contributor Author

mrmbernardi commented Aug 1, 2024

If the review is too big for the milestone, feel free to push it back to after the next version.

Also please advise on the changelog line as this fixes a few things.

@AaronVanGeffen
Copy link
Member

AaronVanGeffen commented Aug 1, 2024

Also please advise on the changelog line as this fixes a few things.

Just because it's one PR doesn't mean you need to put everything on one line. If it addresses multiple distinct issues, please use separate changelog entries as well :)

I'd like to suggest something among the lines of:

- Fix: [#22307] Scan lines in financial charts are not invalidated properly.
- Fix: [#22395, #22396] Misaligned tick marks in financial and guest count graphs.

Copy link
Member

@AaronVanGeffen AaronVanGeffen left a comment

Choose a reason for hiding this comment

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

This is working very nicely! Vastly improved chart performance, much cleaner code, and I like how the finance window charts are now resizeable.

I wonder if it's worth making the charts in the park window resizeable and 'scanable' as well. Something for a follow-up PR though, perhaps :)

@Gymnasiast Gymnasiast removed this from the v0.4.13 milestone Aug 2, 2024
@mrmbernardi mrmbernardi force-pushed the fix-graphs branch 2 times, most recently from b745cbe to 5f29353 Compare August 2, 2024 15:40
@mrmbernardi
Copy link
Contributor Author

I added a new y axis label algorithm. It simply increments the leading digit of the max and sets all other digits to zero. E.g. if the maximum value in the plot is 6810, then the graph will be plotted from 0 to 7000 (for a non-centred graph).

@mrmbernardi mrmbernardi added this to the After v0.4.13 milestone Aug 2, 2024
@AaronVanGeffen AaronVanGeffen modified the milestones: After v0.4.13, v0.4.14 Aug 4, 2024
@mrmbernardi mrmbernardi added the original bug This was an issue in the original game already. label Aug 5, 2024
@mrmbernardi mrmbernardi merged commit 0e1ba1c into OpenRCT2:develop Aug 5, 2024
22 checks passed
@ZehMatt
Copy link
Member

ZehMatt commented Aug 5, 2024

I think there are a couple of open issues which can probably closed now, also good work.

@ZehMatt
Copy link
Member

ZehMatt commented Aug 5, 2024

openrct2_2024-08-05_18-39-49cf3a5e67dc65752f6
openrct2_2024-08-05_18-40-280976eb7dbb546369f

There are unfortunately some issues.

@J-PIE-314
Copy link

Yes, also notice that the negative numbers are above the positive numbers.

@mrmbernardi
Copy link
Contributor Author

Sorted in #22488

@cheweytoo
Copy link
Contributor

This has literal overflow issues ;-)

As seen in 9f0125b develop:

fgoverflow

Also, I note a distinct lack of graph.

@mrmbernardi
Copy link
Contributor Author

This has literal overflow issues ;-)

As seen in 9f0125b develop:

fgoverflow

Also, I note a distinct lack of graph.

Send me the save and I'll fix it ;)

@cheweytoo
Copy link
Contributor

Here it is:

overflowing_millions.zip

@mrmbernardi
Copy link
Contributor Author

mrmbernardi commented Aug 6, 2024

@cheweytoo

Should be fixed in #22498.

I always knew this was possible, but I didn't think people would typically have so much money. The graph will still glitch out if you get a few orders of magnitude away from the integer maximum for the money count, but you would need hundreds of trillions of euros or something thereabouts.

The guest graph will also overflow if you have 100,000 guests or more, but I don't think this is likely. I thought the practical guest limit was around 10k, but you seem to have 20k in your park, at least before you deleted the entire park for some reason.

@cheweytoo
Copy link
Contributor

@cheweytoo

Should be fixed in #22498.

Thanks!

I thought the practical guest limit was around 10k, but you seem to have 20k in your park

It's stable at about 17.5k thanks to permanent advertising, many ATMs, and lots of entertainers. I should really start a new, larger long-term park now that the various limits are higher – if only to see if a higher soft guest cap would allow to increase this even further.

at least before you deleted the entire park for some reason.

I usually try to deliver as minimal a test case as possible. Might have gone a bit overboard this time, but using the large scale deletion tools with reckless abandon can be fun too. ;-)

janisozaur added a commit that referenced this pull request Sep 1, 2024
- Feature: [#15750] Allow using different types of park entrance in one park.
- Feature: [#20942] Allow removing all park fences from the Cheats window.
- Feature: [#21675] Guests ignore price limit cheat.
- Feature: [#22206] Add option to randomise train or vehicle colours.
- Feature: [#22392] [Plugin] Expose ride vehicle’s spin to the plugin API.
- Feature: [#22414] Finance graphs can be resized.
- Feature: [#22569] Footpath placement now respects the construction modifier keys (ctrl/shift).
- Change: [#21189] Patches to fix scenario bugs are now described in .parkpatch files, instead of inside the code.
- Change: [#21659] Increase the Hybrid Roller Coaster’s maximum lift speed to 17 km/h (11 mph).
- Change: [#22466] The Clear Scenery tool now uses a bulldozer cursor instead of a generic crosshair.
- Change: [#22490] The tool to change land and construction rights has been moved out of the Map window.
- Change: [#22490] In sandbox mode, changing land or construction rights now acts as buying or selling.
- Change: [#22491] Scrollbars are now hidden if the scrollable widget is not actually overflowing.
- Change: [#22541] In editor/sandbox mode, tool widgets now appear on the side of the map window, instead of the bottom.
- Change: [#22592] Cheats have been redistributed along three new tabs: date, staff, and nature/weather.
- Fix: [#21123] Transparency options are not respected on startup.
- Fix: [#21189] Additional missing/misplaced land & construction rights tiles in Schneider Shores and Urban Park.
- Fix: [#21908] Errors showing up when placing/moving track design previews.
- Fix: [#22307] Hover tooltips in financial charts are not invalidated properly.
- Fix: [#22316] Potential crash when switching the drawing engine while the game is running.
- Fix: [#22395, #22396] Misaligned tick marks in financial and guest count graphs (original bug).
- Fix: [#22457] Potential crash opening the scenario select window.
- Fix: [#22520] Virtual floor no longer appears when holding modifier keys during track construction.
- Fix: [#22527] Forcing an element type to “wall” via scripts can crash the game.
- Fix: [#22582] Lighting effects are not enabled/disabled correctly, making the game appear frozen.
- Fix: [#22598] Add several .parkpatch files to .sea scenarios with corresponding patches for RCT1 and RCT2 scenarios.
- Fix: [#22606] Virtual floor is sometimes drawn above the path when placing paths.
- Fix: [#22625] Fix compilation with orignal ride ratings.
- Fix: [#22663] Additional missing/misplaced land & construction rights tiles in Mystic Mountain, Build your own Six Flags Holland and Build your own Six Flags over Texas.
- Fix: [#22671] Game default to hide supports on startup.
- Fix: [#22671] Unchecking invisible option does not uncheck see-through option on transparency options and vice versa.
- Fix: [#22677] Hovering the file list in the load/save window causes a slowdown.
Sadret pushed a commit to Sadret/OpenRCT2 that referenced this pull request Sep 7, 2024
- Feature: [OpenRCT2#15750] Allow using different types of park entrance in one park.
- Feature: [OpenRCT2#20942] Allow removing all park fences from the Cheats window.
- Feature: [OpenRCT2#21675] Guests ignore price limit cheat.
- Feature: [OpenRCT2#22206] Add option to randomise train or vehicle colours.
- Feature: [OpenRCT2#22392] [Plugin] Expose ride vehicle’s spin to the plugin API.
- Feature: [OpenRCT2#22414] Finance graphs can be resized.
- Feature: [OpenRCT2#22569] Footpath placement now respects the construction modifier keys (ctrl/shift).
- Change: [OpenRCT2#21189] Patches to fix scenario bugs are now described in .parkpatch files, instead of inside the code.
- Change: [OpenRCT2#21659] Increase the Hybrid Roller Coaster’s maximum lift speed to 17 km/h (11 mph).
- Change: [OpenRCT2#22466] The Clear Scenery tool now uses a bulldozer cursor instead of a generic crosshair.
- Change: [OpenRCT2#22490] The tool to change land and construction rights has been moved out of the Map window.
- Change: [OpenRCT2#22490] In sandbox mode, changing land or construction rights now acts as buying or selling.
- Change: [OpenRCT2#22491] Scrollbars are now hidden if the scrollable widget is not actually overflowing.
- Change: [OpenRCT2#22541] In editor/sandbox mode, tool widgets now appear on the side of the map window, instead of the bottom.
- Change: [OpenRCT2#22592] Cheats have been redistributed along three new tabs: date, staff, and nature/weather.
- Fix: [OpenRCT2#21123] Transparency options are not respected on startup.
- Fix: [OpenRCT2#21189] Additional missing/misplaced land & construction rights tiles in Schneider Shores and Urban Park.
- Fix: [OpenRCT2#21908] Errors showing up when placing/moving track design previews.
- Fix: [OpenRCT2#22307] Hover tooltips in financial charts are not invalidated properly.
- Fix: [OpenRCT2#22316] Potential crash when switching the drawing engine while the game is running.
- Fix: [OpenRCT2#22395, OpenRCT2#22396] Misaligned tick marks in financial and guest count graphs (original bug).
- Fix: [OpenRCT2#22457] Potential crash opening the scenario select window.
- Fix: [OpenRCT2#22520] Virtual floor no longer appears when holding modifier keys during track construction.
- Fix: [OpenRCT2#22527] Forcing an element type to “wall” via scripts can crash the game.
- Fix: [OpenRCT2#22582] Lighting effects are not enabled/disabled correctly, making the game appear frozen.
- Fix: [OpenRCT2#22598] Add several .parkpatch files to .sea scenarios with corresponding patches for RCT1 and RCT2 scenarios.
- Fix: [OpenRCT2#22606] Virtual floor is sometimes drawn above the path when placing paths.
- Fix: [OpenRCT2#22625] Fix compilation with orignal ride ratings.
- Fix: [OpenRCT2#22663] Additional missing/misplaced land & construction rights tiles in Mystic Mountain, Build your own Six Flags Holland and Build your own Six Flags over Texas.
- Fix: [OpenRCT2#22671] Game default to hide supports on startup.
- Fix: [OpenRCT2#22671] Unchecking invisible option does not uncheck see-through option on transparency options and vice versa.
- Fix: [OpenRCT2#22677] Hovering the file list in the load/save window causes a slowdown.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
original bug This was an issue in the original game already.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Guest count graph has misaligned tick marks
6 participants