Feature: Allow vehicle dragger tool to snap vehicles to track #124
Conversation
guysv
force-pushed
the
vehicle-dragger-snap-to-track-3
branch
from
a9c7b2b to
339c41b
Compare
src/viewmodels/vehicleViewModel.ts
Outdated
| readonly _spacing = store<number | null>(0); | ||
| readonly _x = store<number>(0); | ||
| readonly _y = store<number>(0); | ||
| readonly _z = store<number>(0); | ||
| readonly _xyz = compute(this._x, this._y, this._z, (x, y, z) => { return {x, y, z}; }); |
There was a problem hiding this comment.
I don't think it's necessary to introduce a new store for this, there's already the separate available stores and I prefer not to add memory and computing usage if it's not strictly necessary.
src/services/vehicleDragger.ts
Outdated
| state: DragState; | ||
| } | ||
|
|
||
| /** | ||
| * Get a possible position to drag the vehicle to. | ||
| */ | ||
| function getPositionFromTool(args: ToolEventArgs, vehicleType: RideObjectVehicle | null): CoordsXYZ | null | ||
| function getPositionFromTool(args: ToolEventArgs, vehicleType: RideObjectVehicle | null, currentId: number): [CoordsXYZ | null, CarTrackLocation | null] |
There was a problem hiding this comment.
Wouldn't it be better to make this return type a single object with all the required fields, instead of a tuple with two different objects and some duplicate fields? 😅
src/services/vehicleDragger.ts
Outdated
| } else { | ||
| // internally, travelBy calls MoveTo so the car will render on the holding | ||
| // track | ||
| // HACK: seems to make the vehicle update render properly |
There was a problem hiding this comment.
I think we need a new scripting API for it. something like Car.redraw()
There was a problem hiding this comment.
Hmm it sounds more like setting the track location property should just invalidate/redraw the vehicle on the cpp side. Such a fix could also be done/merged separately without having to put this PR in a wait for a new API. 🙂
There was a problem hiding this comment.
Umm I still think the hack is needed though. for now at least. Having the vehicle render in the wrong place can be unpleasent to use.
There was a problem hiding this comment.
Would it be possible to just travelBy once without the getDestination method?
car.travelBy(1);Or possibly with 0 instead of 1 if that works too.
src/services/vehicleDragger.ts
Outdated
| @@ -84,25 +112,33 @@ const enum DragState | |||
| Cancel | |||
| } | |||
|
|
|||
| interface DragPosition | |||
There was a problem hiding this comment.
Maybe this can be merged with DragVehicleArgs as well with reducing all duplicate fields?
There was a problem hiding this comment.
it is now embedded within DragVehicleArgs. I still needed DragPosition to be stand alone to keep away from adding alot of new code to getPositionFromTool
guysv
force-pushed
the
vehicle-dragger-snap-to-track-3
branch
from
339c41b to
4e32721
Compare
guysv
force-pushed
the
vehicle-dragger-snap-to-track-3
branch
4 times, most recently
from
46a2634 to
15eba54
Compare
Basssiiie
force-pushed
the
vehicle-dragger-snap-to-track-3
branch
from
278f4ea to
b58edab
Compare
There was a problem hiding this comment.
LGTM! I'll merge it after OpenRCT2/OpenRCT2#23359 is completed and merged. 🙂
guysv
force-pushed
the
vehicle-dragger-snap-to-track-3
branch
from
b58edab to
16a0236
Compare
To my understanding this is useful for shoestringing
demo:
https://github.com/user-attachments/assets/8a1e764e-b0d6-4c17-95ce-7973902d3a7f