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

Feature: Vehicle spin controller #123

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

guysv
Copy link

@guysv guysv commented Oct 20, 2024

Allows modifying the vehicle spin value, which people might find useful.

Earlier demo:
https://github.com/user-attachments/assets/9c54007b-7f91-4c5d-a736-c9ddb5298d56

Current UI:
image

Copy link
Owner

@Basssiiie Basssiiie left a comment

Choose a reason for hiding this comment

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

Thanks for your PR and for splitting this off from the other work! I just have a few nitpicks before merging. 🙂

src/services/spacingEditor.ts Outdated Show resolved Hide resolved
Comment on lines 379 to 387
positionSpinner({
_label: { text: "Seat spin:" },
minimum: 0,
maximum: 255,
disabled: model._isPositionDisabled,
step: model._multiplier,
value: model._spin,
format: model._formatPosition,
onChange: (_, incr) => model._modifyVehicle(setSpin, incr)
Copy link
Owner

Choose a reason for hiding this comment

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

If I'm not mistaken, this could just be a labelSpinner instead of a positionSpinner as it doesn't need all the extra checks, so it can be implemented more like seats or mass (no format, no incr in onChange, include the missing wrapMode)

Also, there should probably be a different disabled store, one that checks if the vehicle can actually spin (similar to _isUnpowered).

Copy link
Author

Choose a reason for hiding this comment

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

kinda hard to say when a vehicle is allowed to spin. there's some object flag that marks that the car rotates during turns but you could turn that off and still be able modify spinning cars to render other (spin) frames. Spin-less cars obviously just ignore that field.

Copy link
Owner

Choose a reason for hiding this comment

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

Wouldn't the flag only be incorrectly off on incorrectly configured CSO vehicles? I'd assume it would need some flag to be aware that there are spinning frames, but it's been a while since I looked at that code. 😅

Copy link
Author

@guysv guysv Oct 21, 2024

Choose a reason for hiding this comment

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

Well but even with the object turn spin flag turned off, modifying the current spin frame is still useful.
Clip-optimized

Copy link
Author

@guysv guysv Oct 21, 2024

Choose a reason for hiding this comment

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

actually you could've done that with the spin locking track piece as well. (that toggles VehicleFlags::SpinningIsLocked). But cars with the turn-spin flag (CAR_ENTRY_FLAG_SPINNING) turned off can still render spin sprites if we modify the car spin value.

Thing is, setting VehicleFlags::SpinningIsLocked will still reset the car spin when reaching the station (which is not desired for rides like the one in the gif). so turning off CAR_ENTRY_FLAG_SPINNING at ride object level + modifying the car spin value can enable some new tricks.

Copy link
Author

@guysv guysv Oct 21, 2024

Choose a reason for hiding this comment

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

so in short CAR_ENTRY_FLAG_SPINNING doesn't indicate if the car respect Car.spin field.

I see this bit in src/openrct2/paint/vehicle/VehiclePaint.cpp:

    if (carEntry->flags & CAR_ENTRY_FLAG_SPINNING_ADDITIONAL_FRAMES)
    {
        baseImageId += (vehicle->spin_sprite / 8) & 31;
    }

The only rides that use spin_sprite (Car.spin) without CAR_ENTRY_FLAG_SPINNING_ADDITIONAL_FRAMES are:

  • src/openrct2/paint/vehicle/Vehicle.RiverRapids.cpp
  • src/openrct2/paint/vehicle/Vehicle.VirginaReel.cpp

So maybe make the spinner disable test !CAR_ENTRY_FLAG_SPINNING_ADDITIONAL_FRAMES && type!=rapids && type != virginiaReel?

Copy link
Owner

@Basssiiie Basssiiie Oct 22, 2024

Choose a reason for hiding this comment

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

CAR_ENTRY_FLAG_SPINNING is the flag I meant yes. We don't need to look at VehicleFlags::SpinningIsLocked as I agree with the points you raised in regards to that. 🙂

But cars with the turn-spin flag (CAR_ENTRY_FLAG_SPINNING) turned off can still render spin sprites if we modify the car spin value.

But wouldn't this only occur on custom objects that don't have the correct flags? Or are there also built-in vehicles that can spin but don't have this flag?

Copy link
Author

Choose a reason for hiding this comment

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

Don't think there are, but this is a tool meant for game state hacking anyway I presumed?

Copy link
Owner

Choose a reason for hiding this comment

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

It is a tool for game state hacking, but it can be overwhelming if there are too many options that don't do anything It's also consistent with how the powered spinners are disabled when they are of no use.

src/services/vehicleEditor.ts Outdated Show resolved Hide resolved
tests/services/spacingEditor.tests.ts Show resolved Hide resolved
@guysv
Copy link
Author

guysv commented Oct 22, 2024

image I also want to bump the window width a bit so this glitch won't occur?

Copy link
Owner

@Basssiiie Basssiiie left a comment

Choose a reason for hiding this comment

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

Thank you for the quick fixes!

I also want to bump the window width a bit so this glitch won't occur?

Yes good point, please do. 🙂

Comment on lines +441 to +442
{ trackProgress: 31, trackLocation: {...flatTrackPiece.position, trackType: flatTrackPiece.type} }, // front car
{ trackProgress: 0, trackLocation: {...flatTrackPiece.position, trackType: flatTrackPiece.type} } // back car
Copy link
Owner

Choose a reason for hiding this comment

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

These one can use the new toLocation() as well.

Comment on lines +427 to +428
{ trackProgress: 17, trackLocation: {...flatTrackPiece.position, trackType: flatTrackPiece.type} }, // front car
{ trackProgress: 7, trackLocation: {...flatTrackPiece.position, trackType: flatTrackPiece.type} }
Copy link
Owner

Choose a reason for hiding this comment

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

These can use toLocation() too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants