Skip to content

Commit

Permalink
Fix spectator camera implementation
Browse files Browse the repository at this point in the history 
There are several problems that were solved:

1. Spectating failed to follow players through cross-region/cross-world
   teleporting.
2. Inability to set camera to off-region entity
3. Various crashes involved with off-region camera entity
4. Spectator does not always follow entities through portals

We fix #1 by sending ClientboundSetCameraPacket when the spectated
entity becomes in tracked, as the client appears to be unable to
correctly handle when the camera entity is removed, even temporarily.

We fix #2 by allowing off-region entities that are not dead.
However, this required the teleportation code in both setCamera
and the ServerPlayer#tick method to be modified to correctly
handle off-region camera targets.

We fix #3 by correctly teleporting to off-region locations
when needed.

We fix #4 by following the Bukkit entity. This allows spectator
mode to properly follow non-player entities through portals.

Issues #1 (cross-world) and #4 are probably present in Paper/Vanilla, however
solving #2 and #3 required a solution for these.

Fixes #324
  • Loading branch information
@Spottedleaf
Spottedleaf committed Jan 28, 2025
1 parent e7bb50e commit ec78da5
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 8 deletions.
20 changes: 16 additions & 4 deletions ...erver/minecraft-patches/sources/io/papermc/paper/threadedregions/TeleportUtils.java.patch
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
--- /dev/null
+++ b/io/papermc/paper/threadedregions/TeleportUtils.java
@@ -1,0 +_,70 @@
@@ -1,0 +_,82 @@
+package io.papermc.paper.threadedregions;
+
+import ca.spottedleaf.concurrentutil.completable.CallbackCompletable;
Expand All @@ -13,8 +13,14 @@
+
+public final class TeleportUtils {
+
+ public static void teleport(final Entity from, final boolean useFromRootVehicle, final Entity to, final Float yaw, final Float pitch,
+ final long teleportFlags, final PlayerTeleportEvent.TeleportCause cause, final Consumer<Entity> onComplete) {
+ public static <T extends Entity> void teleport(final T from, final boolean useFromRootVehicle, final Entity to, final Float yaw, final Float pitch,
+ final long teleportFlags, final PlayerTeleportEvent.TeleportCause cause, final Consumer<Entity> onComplete) {
+ teleport(from, useFromRootVehicle, to, yaw, pitch, teleportFlags, cause, onComplete, null);
+ }
+
+ public static <T extends Entity> void teleport(final T from, final boolean useFromRootVehicle, final Entity to, final Float yaw, final Float pitch,
+ final long teleportFlags, final PlayerTeleportEvent.TeleportCause cause, final Consumer<Entity> onComplete,
+ final java.util.function.Predicate<T> preTeleport) {
+ // retrieve coordinates
+ final CallbackCompletable<Location> positionCompletable = new CallbackCompletable<>();
+
Expand All @@ -27,10 +33,16 @@
+ return;
+ }
+ final boolean scheduled = from.getBukkitEntity().taskScheduler.schedule(
+ (final Entity realFrom) -> {
+ (final T realFrom) -> {
+ final Vec3 pos = new Vec3(
+ loc.getX(), loc.getY(), loc.getZ()
+ );
+ if (preTeleport != null && !preTeleport.test(realFrom)) {
+ if (onComplete != null) {
+ onComplete.accept(null);
+ }
+ return;
+ }
+ (useFromRootVehicle ? realFrom.getRootVehicle() : realFrom).teleportAsync(
+ ((CraftWorld)loc.getWorld()).getHandle(), pos, null, null, null,
+ cause, teleportFlags, onComplete
Expand Down
77 changes: 75 additions & 2 deletions folia-server/minecraft-patches/sources/net/minecraft/server/level/ServerPlayer.java.patch
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,31 @@
} else {
LOGGER.warn("Failed to spawn player ender pearl in level ({}), skipping", optional.get());
}
@@ -817,12 +_,23 @@

Entity camera = this.getCamera();
if (camera != this) {
- if (camera.isAlive()) {
+ if (camera.canBeSpectated()) { // Folia - region threading - replace removed check
+ if (ca.spottedleaf.moonrise.common.util.TickThread.isTickThreadFor(camera) && !camera.isRemoved()) { // Folia - region threading
this.absMoveTo(camera.getX(), camera.getY(), camera.getZ(), camera.getYRot(), camera.getXRot());
this.serverLevel().getChunkSource().move(this);
if (this.wantsToStopRiding()) {
this.setCamera(this);
}
+ } else { // Folia start - region threading
+ Entity realCamera = camera.getBukkitEntity().getHandleRaw();
+ if (realCamera != camera) {
+ this.setCamera(this);
+ this.setCamera(realCamera);
+ } else {
+ this.teleportToCameraOffRegion();
+ }
+ }
+ // Folia end - region threading
} else {
this.setCamera(this);
}
@@ -1357,9 +_,332 @@
}
}
Expand Down Expand Up @@ -521,18 +546,66 @@
if (this.isSleeping()) return null; // CraftBukkit - SPIGOT-3154
if (this.isRemoved()) {
return null;
@@ -2398,6 +_,11 @@
@@ -2397,7 +_,30 @@
return (Entity)(this.camera == null ? this : this.camera);
}

+ // Folia start - region threading
+ private void teleportToCameraOffRegion() {
+ Entity cameraFinal = this.camera;
+ // use the task scheduler, as we don't know where the caller is invoking from
+ if (this != cameraFinal) {
+ this.getBukkitEntity().taskScheduler.schedule((final ServerPlayer newPlayer) -> {
+ io.papermc.paper.threadedregions.TeleportUtils.teleport(
+ newPlayer, false, cameraFinal, null, null, 0L,
+ org.bukkit.event.player.PlayerTeleportEvent.TeleportCause.SPECTATE, null,
+ (final ServerPlayer newerPlayer) -> {
+ return newerPlayer.camera == cameraFinal;
+ }
+ );
+ }, null, 1L);
+ } // else: do not bother teleporting to self
+ }
+ // Folia end - region threading
+
public void setCamera(@Nullable Entity entityToSpectate) {
+ // Folia start - region threading
+ if (entityToSpectate != null && !ca.spottedleaf.moonrise.common.util.TickThread.isTickThreadFor(entityToSpectate)) {
+ if (entityToSpectate != null && (entityToSpectate != this && !entityToSpectate.canBeSpectated())) {
+ return;
+ }
+ // Folia end - region threading
Entity camera = this.getCamera();
this.camera = (Entity)(entityToSpectate == null ? this : entityToSpectate);
if (camera != this.camera) {
@@ -2416,16 +_,19 @@
}
}
// Paper end - Add PlayerStartSpectatingEntityEvent and PlayerStopSpectatingEntity
- if (this.camera.level() instanceof ServerLevel serverLevel) {
- this.teleportTo(serverLevel, this.camera.getX(), this.camera.getY(), this.camera.getZ(), Set.of(), this.getYRot(), this.getXRot(), false, org.bukkit.event.player.PlayerTeleportEvent.TeleportCause.SPECTATE); // CraftBukkit
- }
-
- if (entityToSpectate != null) {
- this.serverLevel().getChunkSource().move(this);
- }
-
+ // Folia - region threading - move down
+
+ // Folia - region threading - not needed
+
+ // Folia start - region threading - handle camera setting better
+ if (this.camera == this
+ || (ca.spottedleaf.moonrise.common.util.TickThread.isTickThreadFor(this.camera) && this.camera.moonrise$getTrackedEntity() != null
+ && this.camera.moonrise$getTrackedEntity().seenBy.contains(this.connection))) {
+ // Folia end - region threading - handle camera setting better
this.connection.send(new ClientboundSetCameraPacket(this.camera));
- this.connection.resetPosition();
+ } // Folia - region threading - handle camera setting better
+ //this.connection.resetPosition(); // Folia - region threading - not needed
+ this.teleportToCameraOffRegion(); // Folia - region threading - moved down
}
}

@@ -2896,11 +_,11 @@
}

Expand Down
32 changes: 31 additions & 1 deletion folia-server/minecraft-patches/sources/net/minecraft/world/entity/Entity.java.patch
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
private EntityDimensions dimensions;
private float eyeHeight;
public boolean isInPowderSnow;
@@ -525,6 +_,19 @@
@@ -525,6 +_,23 @@
}
}
// Paper end - optimise entity tracker
Expand All @@ -43,6 +43,10 @@
+ this.pistonDeltasGameTime += fromRedstoneTimeOffset;
+ }
+ }
+
+ public boolean canBeSpectated() {
+ return !this.getBukkitEntity().taskScheduler.isRetired();
+ }
+ // Folia end - region ticking

public Entity(EntityType<?> entityType, Level level) {
Expand Down Expand Up @@ -992,6 +996,32 @@
protected void removeAfterChangingDimensions() {
this.setRemoved(Entity.RemovalReason.CHANGED_DIMENSION, null); // CraftBukkit - add Bukkit remove cause
if (this instanceof Leashable leashable && leashable.isLeashed()) { // Paper - only call if it is leashed
@@ -4246,6 +_,12 @@
}

public void startSeenByPlayer(ServerPlayer serverPlayer) {
+ // Folia start - region threading
+ if (serverPlayer.getCamera() == this) {
+ // set camera again
+ serverPlayer.connection.send(new net.minecraft.network.protocol.game.ClientboundSetCameraPacket(this));
+ }
+ // Folia end - region threading
}

public void stopSeenByPlayer(ServerPlayer serverPlayer) {
@@ -4255,6 +_,12 @@
new io.papermc.paper.event.player.PlayerUntrackEntityEvent(serverPlayer.getBukkitEntity(), this.getBukkitEntity()).callEvent();
}
// Paper end - entity tracking events
+ // Folia start - region threading
+ if (serverPlayer.getCamera() == this) {
+ // unset camera, the player tick method should TP us close enough again to invoke startSeenByPlayer
+ serverPlayer.connection.send(new net.minecraft.network.protocol.game.ClientboundSetCameraPacket(serverPlayer));
+ }
+ // Folia end - region threading
}

public float rotate(Rotation transformRotation) {
@@ -4790,7 +_,8 @@
}
}
Expand Down
7 changes: 6 additions & 1 deletion folia-server/minecraft-patches/sources/net/minecraft/world/entity/LivingEntity.java.patch
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
protected int autoSpinAttackTicks;
protected float autoSpinAttackDmg;
@Nullable
@@ -307,6 +_,21 @@
@@ -307,6 +_,26 @@
return this.getYHeadRot();
}
// CraftBukkit end
Expand All @@ -23,6 +23,11 @@
+ }
+
+ @Override
+ public boolean canBeSpectated() {
+ return super.canBeSpectated() && this.getHealth() > 0.0F;
+ }
+
+ @Override
+ protected void resetStoredPositions() {
+ super.resetStoredPositions();
+ this.lastClimbablePos = Optional.empty();
Expand Down

0 comments on commit ec78da5

Please sign in to comment.