From 029caa60f3e1a9c7b66715e85af1c0247ebb7aea Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:19:14 +0000 Subject: [PATCH] fix: restore vehicle seats after wardrobe skin changes --- .../managers/VehicleManager.java | 12 +++ .../protocol/VehiclePacketListener.java | 15 ++++ .../VehicleFrameworkCoverageTest.java | 2 +- .../bones/BoneRuntimeCoverageTest.java | 4 +- .../managers/VehicleManagerCoverageTest.java | 45 ++++++++++ .../VehiclePacketListenerRespawnTest.java | 86 +++++++++++++++++++ 6 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 src/test/java/net/tfminecraft/vehicleframework/protocol/VehiclePacketListenerRespawnTest.java diff --git a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java index ae144ae..1430b06 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java +++ b/src/main/java/net/tfminecraft/vehicleframework/managers/VehicleManager.java @@ -388,6 +388,18 @@ public void dismount(Player p) { mountedLeftClickAt.remove(p.getUniqueId()); } } + + /** Reattach the current seat after a client respawn, even when server mount tracking is intact. */ + public void refreshPlayerMount(Player p) { + if (p == null || !p.isOnline() || p.isDead()) return; + ActiveVehicle v = mountedVehicle(p); + if (v == null || v.isDestroyed() || v.getEntity() == null || !v.getEntity().isValid() + || v.getEntity().getWorld() != p.getWorld()) return; + Seat seat = v.getSeat(p); + if (seat == null || seat.getEntity() != p) return; + v.dismountPassenger(p, false); + takeSeat(p, v, seat); + } public void leashedInteract(Player p, ActiveVehicle v, LivingEntity e) { e.setLeashHolder(null); diff --git a/src/main/java/net/tfminecraft/vehicleframework/protocol/VehiclePacketListener.java b/src/main/java/net/tfminecraft/vehicleframework/protocol/VehiclePacketListener.java index f41c51b..5c9e74c 100644 --- a/src/main/java/net/tfminecraft/vehicleframework/protocol/VehiclePacketListener.java +++ b/src/main/java/net/tfminecraft/vehicleframework/protocol/VehiclePacketListener.java @@ -4,6 +4,7 @@ import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.ProtocolManager; import com.comphenix.protocol.events.InternalStructure; +import com.comphenix.protocol.events.ListenerPriority; import com.comphenix.protocol.events.PacketAdapter; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.reflect.StructureModifier; @@ -65,6 +66,20 @@ public void onPacketReceiving(PacketEvent event) { } }); + protocolManager.addPacketListener(new PacketAdapter(plugin, ListenerPriority.MONITOR, PacketType.Play.Server.RESPAWN) { + @Override + public void onPacketSending(PacketEvent event) { + if (event.isCancelled()) return; + Player player = event.getPlayer(); + // Paper's skin refresh resets client mounts without clearing the server seat. + Bukkit.getScheduler().runTaskLater(plugin, () -> { + if (Bukkit.getPlayer(player.getUniqueId()) == player) { + vehicleManager.refreshPlayerMount(player); + } + }, 1L); + } + }); + // Start repeating task to simulate held input Bukkit.getScheduler().runTaskTimer(plugin, () -> { for (Player player : Bukkit.getOnlinePlayers()) { diff --git a/src/test/java/net/tfminecraft/vehicleframework/VehicleFrameworkCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/VehicleFrameworkCoverageTest.java index b2bcfc5..9ba0342 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/VehicleFrameworkCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/VehicleFrameworkCoverageTest.java @@ -106,7 +106,7 @@ class VehicleFrameworkCoverageTest { assertTrue(Files.isRegularFile(temp.resolve("plugin/templates/weapons/gun_turret.yml"))); assertNotNull(FuelLoader.getByString("coal")); assertFalse(WeaponTemplateLoader.map.isEmpty()); verify(plugins, times(7)).registerEvents(any(Listener.class), same(plugin)); - verify(protocol).addPacketListener(any()); verify(plugins, never()).disablePlugin(plugin); + verify(protocol, times(2)).addPacketListener(any()); verify(plugins, never()).disablePlugin(plugin); plugin.onDisable(); assertNull(VehicleFramework.getVehicleRepository()); } diff --git a/src/test/java/net/tfminecraft/vehicleframework/bones/BoneRuntimeCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/bones/BoneRuntimeCoverageTest.java index f1d1831..4566d12 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/bones/BoneRuntimeCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/bones/BoneRuntimeCoverageTest.java @@ -290,8 +290,8 @@ void packetListenerReplaysHeldInputsUntilQuitAndCancelsOppositeDirections() { VehiclePacketListener listener = new VehiclePacketListener(r.manager); listener.register(); ArgumentCaptor capture = ArgumentCaptor.forClass(PacketListener.class); - verify(protocol).addPacketListener(capture.capture()); - PacketListener adapter = capture.getValue(); + verify(protocol, times(2)).addPacketListener(capture.capture()); + PacketListener adapter = capture.getAllValues().getFirst(); Player driver = r.entity(Player.class, new Location(r.world, 0, 64, 0)), other = r.entity(Player.class, new Location(r.world, 1, 64, 0)); r.bukkit.when(Bukkit::getOnlinePlayers).thenReturn(List.of(driver, other)); diff --git a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java index 26d4e96..3839501 100644 --- a/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java +++ b/src/test/java/net/tfminecraft/vehicleframework/managers/VehicleManagerCoverageTest.java @@ -149,6 +149,7 @@ private ActiveVehicle vehicle(String id) { when(v.getId()).thenReturn("cart"); when(v.getName()).thenReturn("Cart " + id); when(v.getEntity().isValid()).thenReturn(true); + when(v.getEntity().getWorld()).thenReturn(world); when(v.getEntity().getLocation()).thenReturn(new Location(world, 0, 64, 0)); when(v.getLocation()).thenAnswer(ignored -> v.getEntity().getLocation()); when(v.getOwnerData()).thenReturn(new OwnerData()); @@ -290,6 +291,50 @@ void inputPacketsRouteMovementAndCaptainJunctionRequests() { for (Keybind key : List.of(Keybind.W, Keybind.S, Keybind.A, Keybind.D)) verify(v).key(player, key); } + @Test + void clientRefreshReattachesTheSameSeatDespiteHealthyServerMountTracking() { + Player player = player("Ryan"); ActiveVehicle v = vehicle("v"); register(v); + Seat seat = mount(player, v); + when(v.getSeat(player)).thenReturn(seat); when(seat.getEntity()).thenReturn(player); + assertTrue(v.getSeatHandler().isMounted(player)); + clearInvocations(v); + manager.refreshPlayerMount(player); + var order = inOrder(v); + order.verify(v).dismountPassenger(player, false); + order.verify(v).addPassenger(player, seat); + verify(v, never()).dismountAll(); verify(v, never()).remove(any(VehicleRemoveReason.class)); + manager.inputPacket(player, 0, 1, false, false); + verify(v).key(player, Keybind.W); + } + + @ParameterizedTest + @ValueSource(strings = {"offline", "dead", "unmounted", "departed", "destroyed", "missing-entity", "invalid-entity", "another-world", "missing-seat", "another-occupant"}) + void clientRefreshDoesNotRestoreASeatThePlayerCanNoLongerUse(String state) { + Player player = player("Ryan"); ActiveVehicle v = vehicle("v"); register(v); + Seat seat = mount(player, v); + when(v.getSeat(player)).thenReturn(seat); when(seat.getEntity()).thenReturn(player); + switch (state) { + case "offline" -> when(player.isOnline()).thenReturn(false); + case "dead" -> when(player.isDead()).thenReturn(true); + case "unmounted" -> manager.dismount(player); + case "departed" -> when(v.isPassenger(player, false)).thenReturn(false); + case "destroyed" -> when(v.isDestroyed()).thenReturn(true); + case "missing-entity" -> when(v.getEntity()).thenReturn(null); + case "invalid-entity" -> when(v.getEntity().isValid()).thenReturn(false); + case "another-world" -> when(player.getWorld()).thenReturn(mock(World.class)); + case "missing-seat" -> when(v.getSeat(player)).thenReturn(null); + case "another-occupant" -> { + Player other = player("Other"); + when(seat.getEntity()).thenReturn(other); + } + } + clearInvocations(v); + manager.refreshPlayerMount(null); + manager.refreshPlayerMount(player); + verify(v, never()).dismountPassenger(any(), anyBoolean()); + verify(v, never()).addPassenger(any(), any()); + } + @Test void leadInteractionOnlyUsesHarnessForSupportedAnimals() { Player player = player("Ryan"); ActiveVehicle v = vehicle("v"); diff --git a/src/test/java/net/tfminecraft/vehicleframework/protocol/VehiclePacketListenerRespawnTest.java b/src/test/java/net/tfminecraft/vehicleframework/protocol/VehiclePacketListenerRespawnTest.java new file mode 100644 index 0000000..4eaf67e --- /dev/null +++ b/src/test/java/net/tfminecraft/vehicleframework/protocol/VehiclePacketListenerRespawnTest.java @@ -0,0 +1,86 @@ +package net.tfminecraft.vehicleframework.protocol; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import com.comphenix.protocol.PacketType; +import com.comphenix.protocol.ProtocolLibrary; +import com.comphenix.protocol.ProtocolManager; +import com.comphenix.protocol.events.*; +import java.util.*; +import net.tfminecraft.vehicleframework.projectiles.ProjectilesCoverageTest.Rig; +import org.bukkit.*; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.*; +import org.mockito.*; + +class VehiclePacketListenerRespawnTest { + private Rig rig; + private final List scopes = new ArrayList<>(); + private PacketListener listener; + private PacketEvent event; + private Player rider; + + @BeforeEach + void setup() { + rig = new Rig(); + Server server = mock(Server.class); + when(server.getVersion()).thenReturn("Paper (MC: 1.21.10)"); + when(server.getBukkitVersion()).thenReturn("1.21.10-R0.1-SNAPSHOT"); + rig.bukkit.when(Bukkit::getServer).thenReturn(server); + rig.bukkit.when(Bukkit::getVersion).thenReturn("Paper (MC: 1.21.10)"); + rig.bukkit.when(Bukkit::getBukkitVersion).thenReturn("1.21.10-R0.1-SNAPSHOT"); + rig.bukkit.when(Bukkit::getLogger).thenReturn(java.util.logging.Logger.getAnonymousLogger()); + var library = mockStatic(ProtocolLibrary.class); scopes.add(library); + var reflection = mockStatic(com.comphenix.protocol.utility.MinecraftReflection.class); scopes.add(reflection); + reflection.when(com.comphenix.protocol.utility.MinecraftReflection::getItemStackClass).thenReturn(org.bukkit.inventory.ItemStack.class); + reflection.when(com.comphenix.protocol.utility.MinecraftReflection::getBlockClass).thenReturn(org.bukkit.block.Block.class); + ProtocolManager protocol = mock(ProtocolManager.class); + library.when(ProtocolLibrary::getProtocolManager).thenReturn(protocol); + new VehiclePacketListener(rig.manager).register(); + var capture = ArgumentCaptor.forClass(PacketListener.class); + verify(protocol, times(2)).addPacketListener(capture.capture()); + listener = capture.getAllValues().get(1); + assertTrue(listener.getSendingWhitelist().getTypes().contains(PacketType.Play.Server.RESPAWN)); + assertEquals(ListenerPriority.MONITOR, listener.getSendingWhitelist().getPriority()); + rider = rig.entity(Player.class, new Location(rig.world, 0, 120, 0)); + rig.bukkit.when(() -> Bukkit.getPlayer(rider.getUniqueId())).thenReturn(rider); + event = mock(PacketEvent.class); + when(event.getPlayer()).thenReturn(rider); + } + + @AfterEach + void cleanup() throws Exception { + Collections.reverse(scopes); + for (var scope : scopes) scope.close(); + rig.close(); + } + + @Test + void respawnRestoresTheSeatOnTheNextTick() { + listener.onPacketSending(event); + verify(rig.manager, never()).refreshPlayerMount(any()); + rig.tick(); + verify(rig.manager).refreshPlayerMount(rider); + } + + @Test + void cancelledRespawnDoesNotRefreshTheMount() { + when(event.isCancelled()).thenReturn(true); + listener.onPacketSending(event); + rig.tick(); + verify(rig.manager, never()).refreshPlayerMount(any()); + } + + @Test + void disconnectOrNewLoginDoesNotRestoreAnOldPlayersSeat() { + listener.onPacketSending(event); + rig.bukkit.when(() -> Bukkit.getPlayer(rider.getUniqueId())).thenReturn(null); + rig.tick(); + listener.onPacketSending(event); + rig.bukkit.when(() -> Bukkit.getPlayer(rider.getUniqueId())).thenReturn(mock(Player.class)); + rig.tick(); + verify(rig.manager, never()).refreshPlayerMount(any()); + } +}