[Plugin] Add track progress property to vehicles by Basssiiie · Pull Request #13593 · OpenRCT2/OpenRCT2

Basssiiie · 2020-12-15T19:58:14Z

Hey all, this is my draft attempt at adding a property for track progress to vehicles for plugins. This would be very useful for my RideVehicleEditor plugin to adjust spacing between vehicles, and maybe others can find some usefulness in it as well!

Track progress can be read and set for Car-instances.
~~If the chosen value is beyond the maximum amount of the current track piece, the vehicle will go to the relative position on the next track piece.~~
If the chosen value is negative, the vehicle will go to the relative position on the previous track piece.

What I'm still planning to do:

~~Clamp track progress at the end of tracks (for non-complete circuits).~~
~~Sometimes if it jumps to the next or previous track, the position is off by 1.~~
~~Test it thoroughly to make sure it doesn't crash or mess up the game somehow (so far it doesn't seem to).~~
Run clang-format.

Update 21 dec:

After talking with Duncan, the proposed implementation has been changed to modify the vehicles 'remaining distance' instead.
Duncan provided the related Add individual vehicle motion update flag #13607 update flag which this implementation uses.
The 'track progress' setter has been removed and replaced by a 'move' method which moves the car a relative distance in either forward or backwards along the track.

A screenshot of it working ingame:

Right now it works surprisingly well and I haven't really found any issues so far.

I'd love to hear feedback though. This is my first pull request and I'm not even sure if this is the best approach for it. Please let me know if this is something that would fit into the source code and if there is anything I should change.

James103 · 2020-12-15T21:02:08Z

Have you tried running clang-format on your changes?

Basssiiie · 2020-12-16T07:21:02Z

Have you tried running clang-format on your changes?

No I haven't, I was looking into how to do that after the checks failed. I'll fix that tonight. :)

duncanspumpkin · 2020-12-16T08:52:53Z

This wont be able to set the correct sprite for vehicles going through reverser elements or rotate elements like log flumes, heartline.

duncanspumpkin · 2020-12-16T09:06:48Z

I think it will need to hook in early to the UpdateMotion functions. The vehicle placement code uses UpdateMotion to place the vehicles and this ensures that all of the values are correct. I'm not too sure how you would achieve that though.

Edit: Possibly could work by setting the value of remaining_distance to whatever you want to update the progress with then calling UpdateMotion(nullptr);.

Something like:

vehicle->remaining_distance += trackProgress;
UpdateMotion(nullptr);

remaining_distance will understand negatives and call the appropriate items for backwards movement.

Basssiiie · 2020-12-16T18:34:14Z

@duncanspumpkin Thank you for your reply. I'll give the remaining_distance a shot. What does it exactly hold? The distance to travel within this frame? Or the distance to the next track piece? Or something else?

duncanspumpkin · 2020-12-16T18:47:14Z

From memory the units are the same as track progress. It's a sort of left over amount after a vehicle update when things don't quite line up

This can be used in conjuction with OpenRCT2#13593 to change the placement of vehicles on a track

Basssiiie · 2020-12-20T23:09:35Z

@duncanspumpkin I realise I checked in your code from #13607 here as well. Should I remove it or should it merge fine after your PR has been merged?

This can be used in conjuction with #13593 to change the placement of vehicles on a track

duncanspumpkin · 2020-12-21T13:29:53Z

@duncanspumpkin I realise I checked in your code from #13607 here as well. Should I remove it or should it merge fine after your PR has been merged?

Could you rebase now that my code has been merged.

Basssiiie · 2020-12-21T15:45:46Z

Could you rebase now that my code has been merged.

Sure! I'll do that tonight or tomorrow when I have time. :)

duncanspumpkin · 2020-12-21T19:51:31Z

+    UpdateTrackMotion(nullptr);
+    ClearUpdateFlag(VEHICLE_UPDATE_FLAG_SINGLE_CAR_POSITION);
+
+    const Ride* curRide = GetRide();


I'm pretty sure you don't need to do any of this moveInfo changes. UpdateTrackMotion should take care of it.

Yes you're right, I'll remove it. :)

duncanspumpkin · 2020-12-21T19:52:12Z

 bool Vehicle::UpdateMotionCollisionDetection(const CoordsXYZ& loc, uint16_t* otherVehicleIndex)
 {
-    if (HasUpdateFlag(VEHICLE_UPDATE_FLAG_COLLISION_DISABLED))
+    if (HasUpdateFlag(VEHICLE_UPDATE_FLAG_COLLISION_DISABLED) || HasUpdateFlag(VEHICLE_UPDATE_FLAG_SINGLE_CAR_POSITION))


Any reason you are doing this rather than just setting VEHICLE_UPDATE_FLAG_COLLISION_DISABLED as well as VEHICLE_UPDATE_FLAG_SINGLE_CAR_POSITION

I can do that yes, that sounds like a better idea. Will it not interfere with any other code if I clear the VEHICLE_UPDATE_FLAG_COLLISION_DISABLED-flag again after UpdateTrackMotion? I am not sure when and where this flag is used elsewhere.

I know it's used for vehicle placement (which is not an issue). Have a look see if it's set for anything else

The only case I can find is the Giga Coaster cablelift, where it is enabled permanently. I don't think UpdateMotionCollisionDetection is called for the cablelift though, so it seems safe to clear it. :)

In all other cases where the flag is set (when vehicles are created), it is immediately cleared afterwards already.

duncanspumpkin · 2020-12-27T16:57:53Z

This looks reasonable. Assuming it all works fine on your plugin I've no issues with it. @IntelOrca do you have anything to add?

Basssiiie · 2020-12-28T13:18:34Z

This looks reasonable. Assuming it all works fine on your plugin I've no issues with it. @IntelOrca do you have anything to add?

Yes it works perfect in my plugin. :) Only quirk I could find is that modifying a car on a slope sometimes moves other vehicles on the train as well. (Like I mentioned on Discord.)

Other than that I tested a lot of special rides like Reverser Coaster, Chairlift, Giga Coaster, Lift, Go Karts, Heartline Twister, Log Flume reversers etc. as well and they all work perfectly. (And no crashes or freezes either.)

If you guys wanna test it as well with the plugin, just let me know and I can send over the *.js file for it!

IntelOrca · 2020-12-28T13:27:59Z

Just wondering whether move is the best name for the method in both the source code and the plugin API. Maybe it should be moveBy or travel or travelBy.

Basssiiie · 2020-12-28T14:10:20Z

Just wondering whether move is the best name for the method in both the source code and the plugin API. Maybe it should be moveBy or travel or travelBy.

I can change it to moveBy if that is preferred. That'd also make it more in line with Viewport.moveTo().

travel and travelBy sounds a bit more unconventional, but that's just my personal feeling. I'd not be against it if it's the final verdict.

Who wants to make the final choice? 😃

Broxzier · 2020-12-28T14:29:21Z

Just move doesn't really explain what the function does, especially since it only takes one argument. travelBy would make it more clear that it has to do with the track, though something even more verbose like moveAlongTrack would be even better IMO.

IntelOrca · 2020-12-28T17:41:31Z

Yeah, my thoughts are that move is likely to mean, move by a coordinate relative to the map rather than an amount relative to the track. So I think it should just have a different name in case we have another function called moveBy or moveTo which move it by an x, y, z axis.

Basssiiie · 2020-12-28T20:24:56Z

So travelBy is the final decision?

IntelOrca · 2020-12-28T20:29:01Z

@duncanspumpkin any personal suggestions?

duncanspumpkin · 2020-12-28T20:30:52Z

No opinions here

Basssiiie · 2020-12-28T21:07:21Z

The move method is now renamed to travelBy. :)

IntelOrca · 2020-12-29T14:26:04Z

Okay finally, you just need to increment the plugin API version number and add this to the changelog. Sorry, I should have mentioned that previously.

Basssiiie · 2020-12-29T14:52:59Z

Great, no worries! I think it should be properly rebased now? And the plugin API version and changelog have been updated. :)

duncanspumpkin added a commit to duncanspumpkin/OpenRCT2 that referenced this pull request Dec 18, 2020

Add individual vehicle motion update flag

acd71ed

This can be used in conjuction with OpenRCT2#13593 to change the placement of vehicles on a track

duncanspumpkin mentioned this pull request Dec 18, 2020

Add individual vehicle motion update flag #13607

Merged

tupaschoal reviewed Dec 19, 2020

View reviewed changes

Comment thread src/openrct2/scripting/ScEntity.hpp Outdated

duncanspumpkin added a commit to duncanspumpkin/OpenRCT2 that referenced this pull request Dec 20, 2020

Add individual vehicle motion update flag

b93d7db

This can be used in conjuction with OpenRCT2#13593 to change the placement of vehicles on a track

IntelOrca reviewed Dec 21, 2020

View reviewed changes

Comment thread distribution/openrct2.d.ts

duncanspumpkin added a commit that referenced this pull request Dec 21, 2020

Add individual vehicle motion update flag (#13607)

6da4a8d

This can be used in conjuction with #13593 to change the placement of vehicles on a track

Basssiiie force-pushed the plugin-track-progress branch from 1cfaf49 to 21c20b1 Compare December 21, 2020 16:25

Basssiiie marked this pull request as ready for review December 21, 2020 16:58

duncanspumpkin reviewed Dec 21, 2020

View reviewed changes

Comment thread src/openrct2/scripting/ScEntity.hpp Outdated

Basssiiie requested review from IntelOrca and duncanspumpkin December 27, 2020 16:04

IntelOrca approved these changes Dec 27, 2020

View reviewed changes

IntelOrca approved these changes Dec 29, 2020

View reviewed changes

Basssiiie added 8 commits December 29, 2020 15:41

Add track progress property to vehicles

d5a9881

Add track progress to openrct2.d.ts

614a27c

Run clang-format on ScEntity.hpp

32bccf4

Replace track position setter with new move method

e44c179

Fix whitespace in openrct2.d.ts

833c8a2

Disable vehicle collision on single car position update

559c327

Remove unnecessary MoveTo code from MoveRelativeDistance

2b2efee

Remove unnecessary Invalidates from plugin car.move() method

7d819c0

IntelOrca added the pending rebase PR needs to be rebased. label Dec 29, 2020

Basssiiie added 3 commits December 29, 2020 15:41

Use collision disable flag instead of update car position flag check

75f0f21

Rename car 'move' method to 'travelBy'

7848a39

Increment plugin version, update changelog

918bd01

Basssiiie force-pushed the plugin-track-progress branch from c30bfef to 918bd01 Compare December 29, 2020 14:50

IntelOrca merged commit ce2319f into OpenRCT2:develop Dec 29, 2020

tupaschoal added this to the v0.3.3 milestone Dec 29, 2020

Basssiiie deleted the plugin-track-progress branch November 26, 2021 19:56

Basssiiie mentioned this pull request Jun 10, 2022

[Plugin] Fix Car.travelBy() moving other cars as well #17369

Merged

Conversation

Basssiiie commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

James103 commented Dec 15, 2020

Uh oh!

Basssiiie commented Dec 16, 2020

Uh oh!

duncanspumpkin commented Dec 16, 2020

Uh oh!

duncanspumpkin commented Dec 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Basssiiie commented Dec 16, 2020

Uh oh!

duncanspumpkin commented Dec 16, 2020

Uh oh!

Uh oh!

Basssiiie commented Dec 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

duncanspumpkin commented Dec 21, 2020

Uh oh!

Basssiiie commented Dec 21, 2020

Uh oh!

duncanspumpkin Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

Basssiiie Dec 22, 2020

Choose a reason for hiding this comment

Uh oh!

duncanspumpkin Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

Basssiiie Dec 22, 2020

Choose a reason for hiding this comment

Uh oh!

duncanspumpkin Dec 22, 2020

Choose a reason for hiding this comment

Uh oh!

Basssiiie Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

duncanspumpkin commented Dec 27, 2020

Uh oh!

Basssiiie commented Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IntelOrca commented Dec 28, 2020

Uh oh!

Basssiiie commented Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Broxzier commented Dec 28, 2020

Uh oh!

IntelOrca commented Dec 28, 2020

Uh oh!

Basssiiie commented Dec 28, 2020

Uh oh!

IntelOrca commented Dec 28, 2020

Uh oh!

duncanspumpkin commented Dec 28, 2020

Uh oh!

Basssiiie commented Dec 28, 2020

Uh oh!

IntelOrca commented Dec 29, 2020

Uh oh!

Basssiiie commented Dec 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Basssiiie commented Dec 15, 2020 •

edited

Loading

duncanspumpkin commented Dec 16, 2020 •

edited

Loading

Basssiiie commented Dec 20, 2020 •

edited

Loading

Basssiiie Dec 22, 2020 •

edited

Loading

Basssiiie commented Dec 28, 2020 •

edited

Loading

Basssiiie commented Dec 28, 2020 •

edited

Loading