Draw dangerous G forces in red on ride graph #7730
Conversation
wolfreak99
force-pushed
the
improvements/ride_graph_red_danger_gs
branch
from
eb45ea1
to
5523276
Compare
wolfreak99
changed the title
src/openrct2-ui/windows/Ride.cpp
Outdated
| @@ -1201,6 +1201,10 @@ static constexpr const rct_window_graphs_y_axis window_graphs_y_axi[] = { | |||
| {13, -4, 1, STR_RIDE_STATS_G_FORCE_FORMAT}, // GRAPH_LATERAL | |||
| }; | |||
|
|
|||
| #define RIDE_G_FORCES_RED_POS_VERTICAL FIXED_2DP(5,00) | |||
| #define RIDE_G_FORCES_RED_NEG_VERTICAL -FIXED_2DP(2,00) | |||
| #define RIDE_G_FORCES_RED_LATERAL FIXED_2DP(2,80) | |||
There was a problem hiding this comment.
It is better to use C++ constants now, e.g.:
static constexpr auto RIDE_G_FORCES_RED_LATERAL = FIXED_2DP(2,80);
|
@wolfreak99 So the red areas correlate to any sections which would appear red by themselves in the stats page? I don't know if the thresholds are different for different rides but it would be ideal if the highlighted portions directly correlate to the stats penalty thresholds for different rides. |
src/openrct2-ui/windows/Ride.cpp
Outdated
| gfx_fill_rect(dpi, x, top, x, bottom, x > measurement->current_item ? PALETTE_INDEX_17 : PALETTE_INDEX_21); | ||
|
|
||
| // If the lateral or vertical forces are too extreme, make line red. | ||
| bool colour_is_red = false; |
|
Can you make it so that the red parts start from the value where it's over the limit, and not the entire line?
|
|
@Broxzier Honestly, I wanted to do that.. But I'm unsure how to approach modifying the code to do that, at least without spending alot of time trial-and-errroring it and making it much more complicated than what it could be made to be. |
wolfreak99
force-pushed
the
improvements/ride_graph_red_danger_gs
branch
from
db5400d
to
2960a5c
Compare
src/openrct2-ui/windows/Ride.cpp
Outdated
| gfx_fill_rect(dpi, x, top, x, bottom, x > measurement->current_item ? PALETTE_INDEX_17 : PALETTE_INDEX_21); | ||
|
|
||
| // If the lateral or vertical forces are too extreme, make line red. | ||
| bool colourIsRed = false; |
There was a problem hiding this comment.
Nitpicking, but perhaps a nice idea to make this variable name a bit more descriptive - gforceAboveThreshold or something.
AaronVanGeffen
force-pushed
the
improvements/ride_graph_red_danger_gs
branch
from
2960a5c
to
c3c1b9d
Compare
|
@Broxzier Would you be fine with merging this as-is? |
|
I think this can be simplified and I'd like to see the red areas drawn more clearly. I'll do it if @wolfreak99 is okay with it. |
|
Yeah. If you want to tackle it, by all means feel free to do so. I tried multiple times but I just can't get the idea to execute right. |
|
I think I figured out how to finally go about drawing the areas. |
wolfreak99
force-pushed
the
improvements/ride_graph_red_danger_gs
branch
2 times, most recently
from
ac50815
to
d5d885b
Compare
|
I screwed up wih the rebase, I meant to rename and tweak the last commit but instead rebased the whole thing by accident. My apologies, but I've found a better way of approaching this without using a giant contraption of functions (finally!). This now shows the lines as @Broxzier and original me wanted. |
src/openrct2-ui/windows/Ride.cpp
Outdated
| { | ||
| if (top >= lineRedNeg || bottom >= lineRedNeg) | ||
| //If line exceeds negative threshold (at bottom of graph) |
src/openrct2-ui/windows/Ride.cpp
Outdated
| top = widget->bottom - widget->top - top - 13; | ||
| bottom = widget->bottom - widget->top - bottom - 13; | ||
|
|
||
| // If red threshold is supported, adjust as well. Otherwise, keep 0. | ||
| if (!(redThresholdPositive == redThresholdNegative == 0)) |
There was a problem hiding this comment.
Afaik, this gets parsed as !((redThresholdPositive == redThresholdNegative) == false) aka !(!(redThresholdPositive == redThresholdNegative)) aka redThresholdPositive == redThresholdNegative. Is this what you meant, or did you mean redThresholdPositive != 0 && redThresholdNegative != 0?
There was a problem hiding this comment.
You've changed a different if statement. The one @SijmenSchoon pointed out still remains as it was.
wolfreak99
force-pushed
the
improvements/ride_graph_red_danger_gs
branch
from
4ea9058
to
38a99f7
Compare
There was a problem hiding this comment.
Changes in 38a99f7 modified the overall logic:
!(redThresholdNegative == 0 && redThresholdPositive == 0) is equivalent to redThresholdNegative != 0 || redThresholdPositive != 0, not redThresholdNegative != 0 && redThresholdPositive != 0
src/openrct2-ui/windows/Ride.cpp
Outdated
| @@ -6019,12 +6019,13 @@ static void window_ride_graphs_scrollpaint(rct_window* w, rct_drawpixelinfo* dpi | |||
| bottom = widget->bottom - widget->top - bottom - 13; | |||
|
|
|||
| // If red threshold is supported, adjust as well. Otherwise, keep 0. | |||
| if (!(redThresholdPositive == redThresholdNegative == 0)) | |||
| if (redThresholdPositive != 0 && redThresholdNegative != 0) | |||
There was a problem hiding this comment.
Discard my previous comment which showed wrong logic.
Because of left-associativity and unless I am wrong, !(redThresholdPositive == redThresholdNegative == 0) resolves the same as !((redThresholdPositive == redThresholdNegative) == 0), ie redThresholdPositive == redThresholdNegative.
Do not mix up comparison and definition: a = b = 0 does what you intuitively think, a == b == 0 does not.
For this reason, such chained constructions shall be avoided at all costs.
src/openrct2-ui/windows/Ride.cpp
Outdated
| top = widget->bottom - widget->top - top - 13; | ||
| bottom = widget->bottom - widget->top - bottom - 13; | ||
|
|
||
| // If red threshold is supported, adjust as well. Otherwise, keep 0. |
There was a problem hiding this comment.
Can you clarify what needs adjusting?
src/openrct2-ui/windows/Ride.cpp
Outdated
| @@ -5974,6 +5979,9 @@ static void window_ride_graphs_scrollpaint(rct_window* w, rct_drawpixelinfo* dpi | |||
| // Plot | |||
| int32_t x = dpi->x; | |||
| int32_t top, bottom; | |||
| // Uses the limits (used to draw high forces in red on measurement tab) to determine if line should be drawn red. | |||
| // By default they are kept 0 unless the graph type supports it. | |||
| int32_t redThresholdPositive = 0, redThresholdNegative = 0; | |||
There was a problem hiding this comment.
Please rename these to make it clearer they are related to the intensity stats. Something like intensityThreasholdPositive.
Also I think this is a good place to use std::optional, so that you don't have to rely on a zero-value.
There was a problem hiding this comment.
Not really sure how to use std::optional offhand to check if it's assigned or not, but I renamed the variable.
There was a problem hiding this comment.
Instead of using std::optional, the checks for a zero value could be replaced with listType == GRAPH_VERTICAL || listType == GRAPH_LATERAL, which I have done now in 2bcf08b.
src/openrct2/interface/Colour.h
Outdated
| @@ -97,6 +97,7 @@ enum | |||
| PALETTE_INDEX_162 = 162, // | |||
| PALETTE_INDEX_171 = 171, // Saturated Red (lightest) Bright Red (middark) | |||
| PALETTE_INDEX_172 = 172, // Saturated Red (10-11), Bright Red (midlight) | |||
| PALETTE_INDEX_173 = 173, // | |||
There was a problem hiding this comment.
Add a comment to describe the palette, please.
There was a problem hiding this comment.
What would be a good comment? I'd like to match the previous comments but i'm unsure what this color would represent
src/openrct2-ui/windows/Ride.cpp
Outdated
| @@ -5981,7 +5981,7 @@ static void window_ride_graphs_scrollpaint(rct_window* w, rct_drawpixelinfo* dpi | |||
| int32_t top, bottom; | |||
| // Uses the limits (used to draw high forces in red on measurement tab) to determine if line should be drawn red. | |||
| // By default they are kept 0 unless the graph type supports it. | |||
| int32_t redThresholdPositive = 0, redThresholdNegative = 0; | |||
| int32_t itensityThresholdPositive = 0, intensityThresholdNegative = 0; | |||
There was a problem hiding this comment.
itensityThresholdPositive has a typo in it (missing an 'n').
|
This needs to be rebased, so the changelog entry can move to its apropirate location (0.2.1+). |
|
Commit history should be cleaned up a little, too. Can one of us do this? I don't like to see the proverbial moving of goalposts in PRs too much… |
Broxzier
force-pushed
the
improvements/ride_graph_red_danger_gs
branch
from
2bcf08b
to
64c52fa
Compare
- Feature: [#4418] Allow steep slopes on the side-friction roller coaster. - Feature: [#7726] Add shortcut to advance one tick. - Feature: [#7956, #7964] Add sprite font glyphs for Hungarian and some Czech letters. - Feature: [#7971] Toolbox option to open custom content folder. - Feature: [#7980] Allow data path for RCT1 to be specified by a command line argument. - Feature: [#8073] Auto-upload minidumps to backtrace.io (optional, MSVC/Windows only) - Feature: [#8078] Add save_park command to in-game console. - Feature: [#8080] New console variable "current_rotation" to get or set view rotation. - Feature: [#8098] Glyph for Russian rouble sign. - Feature: [#8099] Add Powered Launch mode to Inverted RC (for RCT1 parity). - Feature: [#8190] Allow building footpaths on 'corner down' terrain. - Feature: [#8191] Allow building on-ride photos and water S-bends on the Water Coaster. - Feature: [#8259] Add say command to in-game console. - Feature: [#8374] Add replay system. - Feature: [#8377] Add option to adjust amount of autosaves to keep. - Feature: [#8458] Add sprite sorting benchmark. - Feature: [#8583] Add boosters to water coaster. - Feature: [#8648] Add optional chat button to top toolbar in multiplayer games. - Feature: [#8652] Add network window including a graph for data usage visualisation. - Feature: [#8670] Add ability to download missing objects when loading a park. - Change: [#7961] Add new object types: station, terrain surface, and terrain edge. - Change: [#8222] The climate setting has been moved from objective options to scenario options. - Change: [#8718] Allow TARMAC object to be removed when running the `remove_unused_objects` command. - Change: [#8718] No longer require the generic scenery groups and tarmac footpath to be checked when creating a scenario. - Change: [#8734] Disable kick button in multiplayer window when unable to use it. - Fix: [#3832] Changing the colour scheme of track pieces does not work in multiplayer. - Fix: [#4094] Coasters with long flat-to-steep pieces offer them in diagonal mode (original bug). - Fix: [#5684] Player list can desync between clients and server and can crash. - Fix: [#6191] OpenRCT2 fails to run when the path has an emoji in it. - Fix: [#7439] Placement messages have mixed strings - Fix: [#7473] Disabling sound effects also disables "Disable audio on focus loss". - Fix: [#7536] Android builds fail to start. - Fix: [#7689] Deleting 0-tile maze gives a MONEY32_UNDEFINED (negative) refund. - Fix: [#7828] Copied entrances and exits stay when demolishing ride. - Fix: [#7945] Client IP address is logged as `(null)` in server logs. - Fix: [#7952] Performance drop caused by code refactor. - Fix: [#7954] Key validation fails on Windows due to non-ASCII user / player name. - Fix: [#7975] Inspection flag not cleared for rides which are set to never be inspected (original bug). - Fix: [#7985] Giant Screenshot ignores 'Map rendering' settings. - Fix: [#7987] Broken track designs increase money by MONEY32_UNDEFINED. - Fix: [#7991] Scenery and footpaths on Construction Rights tiles can be deleted using Clear Scenery. - Fix: [#8034] Vanilla sprites are broken when making screenshots from command line. - Fix: [#8045] Crash when switching between languages. - Fix: [#8062] In multiplayer warnings for unstable cheats are shown when disabling them. - Fix: [#8090] Maze designs saved incorrectly. - Fix: [#8101] Title sequences window flashes after opening. - Fix: [#8120] Crash trying to place peep spawn outside of map. - Fix: [#8121] Crash Renaming park with server logging enabled. - Fix: [#8139] Buying land costs money when the game is in "no money" mode. - Fix: [#8141] Attempting to build entrance/exit on station 2 does not work. - Fix: [#8142] Reliability of mazes and crooked houses can go below 100%. - Fix: [#8187] Cannot set land ownership over ride entrances or exits in sandbox mode. - Fix: [#8200] Incorrect behaviour when removing entrances and exits that are on the same tile. - Fix: [#8204] Crash when tile element has no surface elements. - Fix: [#8264] Rides and scenery placeable outside of map with ZC and Sandbox mode enabled. - Fix: [#8335] Rides with arbitrary ride types can crash the game when they break down. - Fix: [#8358] Infinite loop when changing vehicle count on stopped ride. - Fix: [#8402] Crash closing a window in some cases. - Fix: [#8431] Crash when game action logging is enabled. - Fix: [#8433] Crash if master server response is not valid JSON. - Fix: [#8434] Crash if curl_easy_init fails. - Fix: [#8443] Crash when selecting the current vehicle for ride that has none available. - Fix: [#8456] Junior booster track piece doesn't connect properly. - Fix: [#8464] Crash on game shutdown. - Fix: [#8469] Crash modifying colour on hacked rides. - Fix: [#8508] Underground roto-drop is not going up. - Fix: [#8555] Multiplayer window text limits are not computed properly. - Fix: [#8572] Steel Twister track pieces ID 64 and 65 drawn incorrectly. - Fix: [#8585] Part of track missing on air powered vertical coaster. - Fix: [#8588] Guest list scrolling breaks above ~2000 guests. - Fix: [#8591] Game loop does not run at a consistent tick rate of 40 Hz. - Fix: [#8647] Marketing campaigns check for entry fees below £1 (original bug). - Fix: [#8653] Crash when peeps attempt to enter a ride with no vehicles. - Fix: [#8720] Desync due to boats colliding with ghost pieces. - Fix: [#8739] Savegame from original game crashes when cruising through map. - Fix: [#8742] Access violation in vehicle_update_sound_params. - Fix: [#8804] Raising water shows money effect at the bottom rather than new height. - Fix: [#8811] Some fields in the sv6 save file not being copied correctly. - Fix: [#8824] Invalid read in footpath_chain_ride_queue. - Improved: [#2940] Allow mouse-dragging to set patrol area (Singleplayer only). - Improved: [#7730] Draw extreme vertical and lateral Gs red in the ride window's graph tab. - Improved: [#7930] Automatically create folders for custom content. - Improved: [#7980] Show the full path of the scenario in the scenario select window. - Improved: [#7993] Allow assigning a keyboard shortcut for opening the tile inspector. - Improved: [#8107] Support Discord release of RCT2. - Improved: [#8491] Highlight entrance and exit with different colours in track design previews. - Improved: Almost completely new Hungarian translation. - Removed: [#7929] Support for scenario text objects.
This is an experimental project/idea. The goal is to take the g force caps that would usually draw the forces red in the ride details tab, and use them to draw the high g-forces as red in the graph tab.
https://i.imgur.com/cJp3SRx.png
https://i.imgur.com/HJMrWrF.png
If all is well, I'll add a changelog entry and whatnot. I just figured someone might be able to provide some improvements with the code such as better variable names or where to place the defines for the limits, etc..