feat: proper passenger position handling for most entities, fix passengers not always being applied to the client, don't sync passengers (#2060)

* feat: proper passenger position handling for most entities, fix passengers not being applied properly on the client sometimes

* chore: ditch the complex passenger solution as 1.20.5 makes this much simpler

* chore: change EntityViewer to check for a vehicle rather than isViewer, add a test case for this bug

* fix: test only worked in isolation

* chore: revert additional pos allocation

* chore: this instead of vehicle
This commit is contained in:
DeidaraMC 2024-03-29 11:11:16 -04:00 committed by GitHub
parent dc17d171ce
commit 832f0e7f5a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 66 additions and 20 deletions

View File

@ -577,7 +577,7 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
effectTick();
}
// Scheduled synchronization
if (ticks >= nextSynchronizationTick) {
if (vehicle == null && ticks >= nextSynchronizationTick) {
synchronizePosition();
}
}
@ -1310,30 +1310,15 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
}
/**
* @return The height offset for passengers of this vehicle
*/
private double getPassengerHeightOffset() {
// TODO: Move this logic elsewhere
if (entityType == EntityType.BOAT) {
return -0.1;
} else if (entityType == EntityType.MINECART) {
return 0.0;
} else {
return entityType.height() * 0.75;
}
}
/**
* Sets the X,Z coordinate of the passenger to the X,Z coordinate of this vehicle
* and sets the Y coordinate of the passenger to the Y coordinate of this vehicle + {@link #getPassengerHeightOffset()}
* Sets the coordinates of the passenger to the coordinates of this vehicle + {@link EntityUtils#getPassengerHeightOffset(Entity, Entity)}
*
* @param newPosition The X,Y,Z position of this vehicle
* @param passenger The passenger to be moved
* @param newPosition the new position of this vehicle
* @param passenger the passenger to be moved
*/
private void updatePassengerPosition(Point newPosition, Entity passenger) {
final Pos oldPassengerPos = passenger.position;
final Pos newPassengerPos = oldPassengerPos.withCoord(newPosition.x(),
newPosition.y() + getPassengerHeightOffset(),
newPosition.y() + EntityUtils.getPassengerHeightOffset(this, passenger),
newPosition.z());
passenger.position = newPassengerPos;
passenger.previousPosition = oldPassengerPos;

View File

@ -46,6 +46,8 @@ final class EntityView {
player.viewEngine.viewerOption.register(entity);
}
}
// Entity#updateNewViewer handles calling itself for passengers
if (entity.getVehicle() != null) return;
entity.updateNewViewer(player);
},
player -> {

View File

@ -2,11 +2,31 @@ package net.minestom.server.utils.entity;
import net.minestom.server.coordinate.Pos;
import net.minestom.server.entity.Entity;
import net.minestom.server.entity.EntityType;
import net.minestom.server.instance.Chunk;
import net.minestom.server.instance.block.Block;
import org.jetbrains.annotations.NotNull;
import java.util.Set;
public final class EntityUtils {
private static final Set<EntityType> SITTING_ENTITIES = Set.of(EntityType.ZOMBIE, EntityType.HUSK, EntityType.DROWNED,
EntityType.SKELETON, EntityType.STRAY, EntityType.WITHER_SKELETON, EntityType.PIGLIN, EntityType.PIGLIN_BRUTE,
EntityType.ZOMBIFIED_PIGLIN);
/**
* @param vehicle the target vehicle
* @param passenger the target passenger
* @return the height offset for the passenger of this vehicle
*/
public static double getPassengerHeightOffset(@NotNull Entity vehicle, @NotNull Entity passenger) {
// TODO: Refactor this in 1.20.5
if (vehicle.getEntityType() == EntityType.BOAT) return -0.1;
if (vehicle.getEntityType() == EntityType.MINECART) return 0.0;
if (SITTING_ENTITIES.contains(passenger.getEntityType()))
return vehicle.getBoundingBox().height() * 0.75;
return vehicle.getBoundingBox().height();
}
private EntityUtils() {
}

View File

@ -1,5 +1,7 @@
package net.minestom.server.entity;
import net.minestom.server.network.packet.server.play.SetPassengersPacket;
import net.minestom.server.network.packet.server.play.SpawnEntityPacket;
import net.minestom.testing.Env;
import net.minestom.testing.EnvTest;
import net.minestom.server.coordinate.Pos;
@ -46,4 +48,41 @@ public class PassengerIntegrationTest {
assertTrue(passenger.getDistance(vehicle) < 2);
}
@Test
public void passengerPacketOrder(Env env) {
var instance = env.createFlatInstance();
var vehicle = new Entity(EntityType.ZOMBIE);
vehicle.setInstance(instance, new Pos(0, 40, 0)).join();
// Add 3 passengers to vehicle to test Entity#updateNewViewer recursion
var passenger1 = new Entity(EntityType.ZOMBIE);
var passenger2 = new Entity(EntityType.ZOMBIE);
var passenger3 = new Entity(EntityType.ZOMBIE);
vehicle.addPassenger(passenger1);
passenger1.addPassenger(passenger2);
passenger2.addPassenger(passenger3);
var connection = env.createConnection();
var spawnTracker = connection.trackIncoming(SpawnEntityPacket.class);
var passengerTracker = connection.trackIncoming(SetPassengersPacket.class);
connection.connect(instance, new Pos(0, 40, 0)).join();
int startingId = passenger3.getEntityId();
passengerTracker.assertCount(3);
var passengerPackets = passengerTracker.collect();
for (int i = 0; i < passengerPackets.size(); i++) {
// Passenger packet order will be sent backwards down the chain of passenger vehicles
assertEquals(startingId - i, passengerPackets.get(i).passengersId().get(0));
}
// Ensure spawn packets are never sent more than once per entity
startingId = vehicle.getEntityId();
spawnTracker.assertCount(4);
var spawnPackets = spawnTracker.collect();
for (int i = 0; i < spawnPackets.size(); i++) {
// If the passenger spawn packets are sent in order we know that
// Entity#updateNewViewer ran as it should
assertEquals(startingId + i, spawnPackets.get(i).entityId());
}
}
}