-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
Add SteerVehicleEvent #3421
Add SteerVehicleEvent #3421
Conversation
Gonna need an API PR too! |
"core.RegistryAccessMixin", | ||
"core.Vec3iMixin", | ||
"core.dispenser.AbstractProjectileDispenseBehaviorMixin", | ||
"data.SpongeDataHolderMixin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't change the formatting here on a whim please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly sure how I managed to do that, but it should be fixed now 😅
|
||
@Inject(method = "handlePlayerInput", at = @At("HEAD")) | ||
public void onHandlePlayerInput(ServerboundPlayerInputPacket packet, CallbackInfo ci) { | ||
SpongeCommon.postEvent(SpongeEventFactory.createSteerVehicleEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the event cancellable? If so, you can make the injection cancellable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre 1.16 that would work fine, but that is not the case in newer vesions since dismounting is handled fully client-side.
@Shadow public ServerPlayer player; | ||
|
||
@Inject(method = "handlePlayerInput", at = @At("HEAD")) | ||
public void onHandlePlayerInput(ServerboundPlayerInputPacket packet, CallbackInfo ci) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make these variables final please
public void onHandlePlayerInput(ServerboundPlayerInputPacket packet, CallbackInfo ci) { | |
public void onHandlePlayerInput(final ServerboundPlayerInputPacket packet, final CallbackInfo ci) { |
@Inject(method = "handlePlayerInput", at = @At("HEAD")) | ||
public void onHandlePlayerInput(ServerboundPlayerInputPacket packet, CallbackInfo ci) { | ||
SpongeCommon.postEvent(SpongeEventFactory.createSteerVehicleEvent( | ||
PhaseTracker.getCauseStackManager().currentCause(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it guaranteed that we'll always be on the main thread here?
|
||
@Inject(method = "setPlayerInput", at = @At("HEAD")) | ||
public void onSetPlayerInput(final float sway, final float surge, final boolean jumping, final boolean dismount, final CallbackInfo ci) { | ||
SpongeCommon.postEvent(SpongeEventFactory.createSteerVehicleEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to fire if the Player Input Packet is sent, regardless of whether the player is actually mounted. It would make more sense to redirect isPassenger
and fire there if isPassenger
would return true
. You could then return false
if you wanted to cancel this packet, which kind of leads me into my next question:
Pre 1.16 that would work fine, but that is not the case in newer vesions since dismounting is handled fully client-side.
...how does that make sense? The server still has to know about this and validate it. Is it because the client dismounts without validation from the server?
(I'll test it at some point, just haven't looked into it yet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to fire if the Player Input Packet is sent, regardless of whether the player is actually mounted. It would make more sense to redirect isPassenger and fire there if isPassenger would return true.
That's probably a good idea 😅
Is it because the client dismounts without validation from the server?
That's exactly what I meant! Sorry for being unclear.
@@ -830,6 +831,30 @@ public void sendMessage(final net.minecraft.network.chat.Component p_241151_1_, | |||
} | |||
} | |||
} | |||
|
|||
@Inject(method = "setPlayerInput", at = @At("HEAD"), cancellable = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a redirect on isPassenger
, so you can avoid calling it twice, and just return false
here to short circuit the if statement.
@@ -830,6 +831,30 @@ public void sendMessage(final net.minecraft.network.chat.Component p_241151_1_, | |||
} | |||
} | |||
} | |||
|
|||
@Inject(method = "setPlayerInput", at = @At("HEAD"), cancellable = true) | |||
public void onSetPlayerInput(final float sway, final float surge, final boolean jumping, final boolean dismount, final CallbackInfo ci) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private boolean impl$onSetPlayerInput(final ServerPlayer player, final float sway, final float surge, final boolean jumping, final boolean dismount)
@@ -204,6 +204,8 @@ | |||
@Shadow protected abstract boolean shadow$fireImmune(); | |||
// @formatter:on | |||
|
|||
@Shadow public abstract boolean isPassenger(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be put in the // @formatter
block above, and prefixed with shadow$
, so
@Shadow public abstract boolean shadow$isPassenger();
@Shadow public float xxa; | ||
@Shadow public float zza; | ||
@Shadow protected boolean jumping; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put these in the // @formatter
block above.
|
||
@Inject(method = "setPlayerInput", at = @At("HEAD"), cancellable = true) | ||
public void onSetPlayerInput(final float sway, final float surge, final boolean jumping, final boolean dismount, final CallbackInfo ci) { | ||
if(isPassenger()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefix all same class accesses with this.
(but this would become this.shadow$isPassenger()
)
Also, space after if
public void onSetPlayerInput(final float sway, final float surge, final boolean jumping, final boolean dismount, final CallbackInfo ci) { | ||
if(isPassenger()) { | ||
if(sway == this.xxa && surge == this.zza && jumping == this.jumping) { | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would return true;
as this is a success condition - everything is the same but the action isn't being cancelled, so continue.
Sponge.eventManager().post(event); | ||
|
||
if(event.isCancelled()) { | ||
ci.cancel(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could replace this with
Sponge.eventManager().post(event); | |
if(event.isCancelled()) { | |
ci.cancel(); | |
} | |
return !Sponge.eventManager().post(event); |
ci.cancel(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll want to return false at the end.
…etListenerImplMixin & fix whatever the fuck happened to mixins.sponge.core.json
Hey! You seem to be running an unsupported version (API 8) of SpongeAPI. Support for API 8 ended in October 2023. API-8 is now end-of-life. We will not be issuing any more API updates for version 8 of our API1 As part of our legacy cleanup we are closing issues relating to API8/1.16 If you wish to move this forward, please rebase this PR on the current branch and reopen it. Footnotes |
Sponge | SpongeAPI
Added SteerVehicleEvent. Resolves SpongePowered/SpongeAPI#2283
While having the event be cancellable would be ideal, I’m not sure how that could be done or if it’s even possible, since Mojang changed the client to handle dismounting fully client-side in 1.16.
Regardless of that, I hope this event makes it to API-8, since I believe it would be of great use when creating vehicle plugins.