Added additional stats to Plugin interface

[AT41]

[ZehMatt]

I'd rather see the getters throw when ride is null, instead of returning a magic number. This can be done later in another PR, just please create an issue for it if you decide to leave it for now.

Please increment the API version, then this is good to go.

I think the error handling should be changed in this PR already, that's quite the different behavior.

[Broxzier]

I think the error handling should be changed in this PR already, that's quite the different behavior.

It's not different from the current behaviour. The existing ride properties don't throw on null either, so it needs to be done for all the other ride properties too, then.

[ZehMatt]

I think the error handling should be changed in this PR already, that's quite the different behavior.

It's not different from the current behaviour. The existing ride properties don't throw on null either, so it needs to be done for all the other ride properties too, then.

Fair point.

[AT41]

API incremented, modified return values so that they can be formatted through {VELOCITY} and {LENGTH}. Will make a new issue for throwing exceptions for getters rather than returning 0 once PR is accepted.

[StephanSpengler]

About the formatting comments: First, I think it is cluttering the documentation. In my opinion we do not need to cross-reference every function that might be useful in some context. Second, formatString does not "convert to kph" but to a string representation of a localised value and unit. Plugin developers should code in a way that is independent of the actual units. formatString should not be used to convert internal units to some other units that the dev prefers to work with. The documentation should be clear to not suggest that.

All in all, my proposal is to improve the formatString documentation instead (separate PR).

[github-actions[bot]]

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

[Basssiiie]

Please keep open.

[github-actions[bot]]

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

[SpartanFrederic104]

Again, please keep this one open.

[ZehMatt]

@AT41 are you planning to continue this PR?

[Basssiiie]

Are there still any unresolved issues? It seems @Sadret's last comment was addressed in the commit after it, without another comment on the PR.

[AT41]

Sorry, been busy but I'll see this PR through to the end!

All the requests should be resolved in the latest version - I'll add a new issue for throwing exceptions for null rides. Comments should also be good unless @Sadret has any further suggestions.

There's a CI/CD error that's failing after I implemented the extra enum for formatting, but unless there's a specific reason for those enum values to be set to those values the tests just need to be adjusted. OpenRCT2\test\tests\FormattingTests.cpp(46): error: Expected equality of these values: "[29:{BLACK}][1:Guests: ][8:{INT32}]" actual Which is: "[30:{BLACK}][1:Guests: ][8:{INT32}]"

Let me know if there's anything else that's needed on my end, thanks!

[ZehMatt]

There's a CI/CD error that's failing after I implemented the extra enum for formatting, but unless there's a specific reason for those enum values to be set to those values the tests just need to be adjusted. OpenRCT2\test\tests\FormattingTests.cpp(46): error: Expected equality of these values: "[29:{BLACK}][1:Guests: ][8:{INT32}]" actual Which is: "[30:{BLACK}][1:Guests: ][8:{INT32}]"

Let me know if there's anything else that's needed on my end, thanks!

It would be good if you can change the tests if they need adjustments.

[AT41]

Ready for another review @Sadret @Broxzier

[AT41]

Time for another review :)

[tupaschoal]

@Broxzier, @Sadret and @Basssiiie would you give this another review, please?

[tupaschoal]

Seems to be an error on the mingw build, can you check @AT41 ?

[AT41]

@Broxzier Everything seems to be good to go. Added a new issue as you suggested earlier: https://github.com/OpenRCT2/OpenRCT2/issues/20994. I can squash commits myself, unless PRs get squashed automatically?

Let me know if there's anything else needed

[github-actions[bot]]

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

[tupaschoal]

@AT41 can you add a changelog entry for this, please?

[Gymnasiast]

Needs updating for the changelog.

[pfroud]

I think this pull request is to support new features in the StatRequirementChecklist plugin.

You may want to coordinate with pull request #20913, which I think has a similar goal in mind.

I think that PR would let you read the requirements from the native openrct2 code, so you wouldn't need to hard-code them in the plugin.

[Basssiiie]

I've tested the implementation and while most of it looks great and correct, I ran into two issues;

• The formatted height string of the highest drop height seems to be off by one in some rare cases, like with the Giga Coaster in Six Flags Holland and this blue coaster I built (it's not launched so it doesn't make a full lap).
• I cannot figure out what the "powered lifts" are supposed to refer to. For some rides it's 0 and for some it's 64 (which is either incorrect or simply a flag instead of a number?):

On other rides everything seems right:

Click here to see my plugin script in case anyone would like to test it for themselves. 🙂

registerPlugin({
	name: "Test for ride stats",
	version: "1",
	authors: ['Basssiiie'],
	targetApiVersion: 82,
	type: "local",
	licence: "MIT",
	main() {
		ui.registerMenuItem("Check ride stats", function() {
			var dropdownName = "test-ride-stats-dropdown";
			var listName = "test-ride-stats-list";
			var rides = map.rides.sort(function(a,b){return a.name.localeCompare(b.name)});
			var window = ui.openWindow({
				classification: "test-ride-stats",
				title: "Ride stats",
				width: 300,
				height: 220,
				widgets: [
					{
						type: "listview",
						name: listName,
						showColumnHeaders: true,
						isStriped: true,
						columns: [
							{ header: "Property", width: 150 },
							{ header: "Raw", width: 40 },
							{ header: "Real", width: 80 },
						],
						items: [],
						x: 5,
						y: 20,
						width: 290,
						height: 170,
					},
					{
						type: "dropdown",
						name: dropdownName,
						x: 5,
						y: 200,
						width: 290,
						height: 15,
						items: rides.map(function(r){return r.name})
					},
				],
				onUpdate()
				{
					var dropdown = window.findWidget(dropdownName);
					var ride = rides[dropdown.selectedIndex];
					if (!ride) return;

					var list = window.findWidget(listName);
					list.items = [
						[ "Max speed:", String(ride.maxSpeed), context.formatString('{VELOCITY}', ride.maxSpeed) ],
						[ "Average speed:", String(ride.averageSpeed), context.formatString('{VELOCITY}', ride.averageSpeed) ],
						[ "Ride time:", String(ride.rideTime), context.formatString('{DURATION}', ride.rideTime) ],
						[ "Ride length:", String(ride.rideLength), context.formatString('{LENGTH}', ride.rideLength) ],
						[ "Max positive vertical Gs:", String(ride.maxPositiveVerticalGs), "" ],
						[ "Max negative vertical Gs:", String(ride.maxNegativeVerticalGs), "" ],
						[ "Max lateral Gs:", String(ride.maxLateralGs), "" ],
						[ "Total air time:", String(ride.totalAirTime), "" ],
						[ "Drops:", String(ride.drops), "" ],
						[ "Powered lifts:", String(ride.poweredLifts), "" ],
						[ "Highest drop height (raw):", String(ride.highestDropHeight), context.formatString('{HEIGHT}', ride.highestDropHeight) ]
					];
				}
			})
		})
	}
});

[AT41]

Fixed the bug with height and powered lifts. The bit logic on height caused precision to be lost, but the new calculations should fix that. Powered lift values are combined with number of drops in a byte but I didn't truncate off the trailing 0s, so that should be fixed as well

[Gymnasiast]

@tupaschoal Yes, I’d like to do another round.

[github-actions[bot]]

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

[AT41]

@Gymnasiast Is there anything blocking this PR?