Fix unvanish skin layers and mod crashes by using vanilla entity trac…#15
Fix unvanish skin layers and mod crashes by using vanilla entity trac…#15Madtrent wants to merge 1 commit into
Conversation
| viewer.networkHandler.sendPacket(PlayerListS2CPacket.entryFromPlayer(List.of(toShow))); | ||
|
|
||
| WrapperPlayServerPlayerInfoUpdate.PlayerInfo info = new WrapperPlayServerPlayerInfoUpdate.PlayerInfo( | ||
| profile, true, toShow.networkHandler.getLatency(), peGameMode, | ||
| net.kyori.adventure.text.Component.text(toShow.getName().getString()), null | ||
| EntityTrackerEntry trackerEntry = new EntityTrackerEntry( | ||
| toShow.getServerWorld(), | ||
| toShow, | ||
| toShow.getType().getTrackTickInterval(), | ||
| toShow.getType().alwaysUpdateVelocity(), | ||
| packet -> { | ||
| } | ||
| ); | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerPlayerInfoUpdate( | ||
| EnumSet.of( | ||
| WrapperPlayServerPlayerInfoUpdate.Action.ADD_PLAYER, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_LISTED, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_LATENCY, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_GAME_MODE | ||
| ), | ||
| info | ||
| )); | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerSpawnEntity( | ||
| toShow.getId(), | ||
| Optional.of(toShow.getUuid()), | ||
| EntityTypes.PLAYER, | ||
| new Vector3d(toShow.getX(), toShow.getY(), toShow.getZ()), | ||
| toShow.getPitch(), | ||
| toShow.getYaw(), | ||
| toShow.getYaw(), | ||
| 0, | ||
| Optional.of(new Vector3d(0, 0, 0)) | ||
| )); | ||
| trackerEntry.sendPackets(viewer, packet -> viewer.networkHandler.sendPacket(packet)); |
There was a problem hiding this comment.
unvanish() calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
Rule Used: THE CENTRAL RULE everything else serves: maintain ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: platforms/fabric-121/src/main/java/gg/modl/minecraft/fabric/v1_21_1/handler/FabricStaffModeHandler.java
Line: 195-206
Comment:
**Unconditional tracker pairing**
`unvanish()` calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
**Rule Used:** THE CENTRAL RULE everything else serves: maintain ... ([source](https://app.greptile.com/review/custom-context?memory=21719055-65fa-440f-b434-eba8d358d917))
How can I resolve this? If you propose a fix, please make it concise.| viewer.networkHandler.sendPacket(PlayerListS2CPacket.entryFromPlayer(List.of(toShow))); | ||
|
|
||
| WrapperPlayServerPlayerInfoUpdate.PlayerInfo info = new WrapperPlayServerPlayerInfoUpdate.PlayerInfo( | ||
| profile, true, toShow.networkHandler.getLatency(), peGameMode, | ||
| net.kyori.adventure.text.Component.text(toShow.getName().getString()), null | ||
| EntityTrackerEntry trackerEntry = new EntityTrackerEntry( | ||
| toShow.getServerWorld(), | ||
| toShow, | ||
| toShow.getType().getTrackTickInterval(), | ||
| toShow.getType().alwaysUpdateVelocity(), | ||
| packet -> { | ||
| } | ||
| ); | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerPlayerInfoUpdate( | ||
| EnumSet.of( | ||
| WrapperPlayServerPlayerInfoUpdate.Action.ADD_PLAYER, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_LISTED, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_LATENCY, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_GAME_MODE | ||
| ), | ||
| info | ||
| )); | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerSpawnEntity( | ||
| toShow.getId(), | ||
| Optional.of(toShow.getUuid()), | ||
| EntityTypes.PLAYER, | ||
| new Vector3d(toShow.getX(), toShow.getY(), toShow.getZ()), | ||
| toShow.getPitch(), | ||
| toShow.getYaw(), | ||
| toShow.getYaw(), | ||
| 0, | ||
| Optional.of(new Vector3d(0, 0, 0)) | ||
| )); | ||
| trackerEntry.sendPackets(viewer, packet -> viewer.networkHandler.sendPacket(packet)); |
There was a problem hiding this comment.
unvanish() calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
Rule Used: THE CENTRAL RULE everything else serves: maintain ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: platforms/fabric-1214/src/main/java/gg/modl/minecraft/fabric/v1_21_4/handler/FabricStaffModeHandler.java
Line: 194-205
Comment:
**Unconditional tracker pairing**
`unvanish()` calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
**Rule Used:** THE CENTRAL RULE everything else serves: maintain ... ([source](https://app.greptile.com/review/custom-context?memory=21719055-65fa-440f-b434-eba8d358d917))
How can I resolve this? If you propose a fix, please make it concise.| viewer.networkHandler.sendPacket(PlayerListS2CPacket.entryFromPlayer(List.of(toShow))); | ||
|
|
||
| WrapperPlayServerPlayerInfoUpdate.PlayerInfo info = new WrapperPlayServerPlayerInfoUpdate.PlayerInfo( | ||
| profile, true, toShow.networkHandler.getLatency(), peGameMode, | ||
| net.kyori.adventure.text.Component.text(toShow.getName().getString()), null | ||
| ); | ||
| EntityTrackerEntry trackerEntry = new EntityTrackerEntry( | ||
| toShow.getWorld(), | ||
| toShow, | ||
| toShow.getType().getTrackTickInterval(), | ||
| toShow.getType().alwaysUpdateVelocity(), | ||
| packet -> {}, | ||
| (packet, excludedPlayers) -> { | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerPlayerInfoUpdate( | ||
| EnumSet.of( | ||
| WrapperPlayServerPlayerInfoUpdate.Action.ADD_PLAYER, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_LISTED, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_LATENCY, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_GAME_MODE | ||
| ), | ||
| info | ||
| )); | ||
| } | ||
| ); | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerSpawnEntity( | ||
| toShow.getId(), | ||
| Optional.of(toShow.getUuid()), | ||
| EntityTypes.PLAYER, | ||
| new Vector3d(toShow.getX(), toShow.getY(), toShow.getZ()), | ||
| toShow.getPitch(), | ||
| toShow.getYaw(), | ||
| toShow.getYaw(), | ||
| 0, | ||
| Optional.of(new Vector3d(0, 0, 0)) | ||
| )); | ||
| } | ||
| trackerEntry.sendPackets(viewer, packet -> viewer.networkHandler.sendPacket(packet)); |
There was a problem hiding this comment.
unvanish() calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
Rule Used: THE CENTRAL RULE everything else serves: maintain ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: platforms/fabric-1218/src/main/java/gg/modl/minecraft/fabric/v1_21_8/handler/FabricStaffModeHandler.java
Line: 195-208
Comment:
**Unconditional tracker pairing**
`unvanish()` calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
**Rule Used:** THE CENTRAL RULE everything else serves: maintain ... ([source](https://app.greptile.com/review/custom-context?memory=21719055-65fa-440f-b434-eba8d358d917))
How can I resolve this? If you propose a fix, please make it concise.| viewer.networkHandler.sendPacket(PlayerListS2CPacket.entryFromPlayer(List.of(toShow))); | ||
|
|
||
| WrapperPlayServerPlayerInfoUpdate.PlayerInfo info = new WrapperPlayServerPlayerInfoUpdate.PlayerInfo( | ||
| profile, true, toShow.networkHandler.getLatency(), peGameMode, | ||
| net.kyori.adventure.text.Component.text(toShow.getName().getString()), null | ||
| ); | ||
| EntityTrackerEntry trackerEntry = new EntityTrackerEntry( | ||
| toShow.getEntityWorld(), | ||
| toShow, | ||
| toShow.getType().getTrackTickInterval(), | ||
| toShow.getType().alwaysUpdateVelocity(), | ||
| new EntityTrackerEntry.TrackerPacketSender() { | ||
| @Override | ||
| public void sendToListeners(Packet<? super ClientPlayPacketListener> packet) { | ||
| } | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerPlayerInfoUpdate( | ||
| EnumSet.of( | ||
| WrapperPlayServerPlayerInfoUpdate.Action.ADD_PLAYER, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_LISTED, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_LATENCY, | ||
| WrapperPlayServerPlayerInfoUpdate.Action.UPDATE_GAME_MODE | ||
| ), | ||
| info | ||
| )); | ||
| @Override | ||
| public void sendToSelfAndListeners(Packet<? super ClientPlayPacketListener> packet) { | ||
| } | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerSpawnEntity( | ||
| toShow.getId(), | ||
| Optional.of(toShow.getUuid()), | ||
| EntityTypes.PLAYER, | ||
| new Vector3d(toShow.getX(), toShow.getY(), toShow.getZ()), | ||
| toShow.getPitch(), | ||
| toShow.getYaw(), | ||
| toShow.getYaw(), | ||
| 0, | ||
| Optional.of(new Vector3d(0, 0, 0)) | ||
| )); | ||
| @Override | ||
| public void sendToListenersIf( | ||
| Packet<? super ClientPlayPacketListener> packet, | ||
| Predicate<ServerPlayerEntity> predicate | ||
| ) { | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| trackerEntry.sendPackets(viewer, packet -> viewer.networkHandler.sendPacket(packet)); |
There was a problem hiding this comment.
unvanish() calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
Rule Used: THE CENTRAL RULE everything else serves: maintain ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: platforms/fabric-12111/src/main/java/gg/modl/minecraft/fabric/v1_21_11/handler/FabricStaffModeHandler.java
Line: 196-221
Comment:
**Unconditional tracker pairing**
`unvanish()` calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
**Rule Used:** THE CENTRAL RULE everything else serves: maintain ... ([source](https://app.greptile.com/review/custom-context?memory=21719055-65fa-440f-b434-eba8d358d917))
How can I resolve this? If you propose a fix, please make it concise.| viewer.connection.send( | ||
|
|
||
| peApi.getPlayerManager().sendPacket(viewer, | ||
| new WrapperPlayServerSpawnEntity( | ||
| toShow.getId(), | ||
| Optional.of(toShow.getUUID()), | ||
| EntityTypes.PLAYER, | ||
| new Vector3d(toShow.getX(), toShow.getY(), toShow.getZ()), | ||
| toShow.getXRot(), | ||
| toShow.getYRot(), | ||
| toShow.getYRot(), | ||
| 0, | ||
| Optional.of(new Vector3d(0, 0, 0)) | ||
| )); | ||
| ClientboundPlayerInfoUpdatePacket.createPlayerInitializing(List.of(toShow)) | ||
| ); | ||
|
|
||
| ServerEntity trackerEntry = new ServerEntity( | ||
| toShow.level(), | ||
| toShow, | ||
| toShow.getType().updateInterval(), | ||
| toShow.getType().trackDeltas(), | ||
| new ServerEntity.Synchronizer() { | ||
| @Override | ||
| public void sendToTrackingPlayers(Packet<? super ClientGamePacketListener> packet) { | ||
| } | ||
|
|
||
| @Override | ||
| public void sendToTrackingPlayersAndSelf(Packet<? super ClientGamePacketListener> packet) { | ||
| } | ||
|
|
||
|
|
||
| @Override | ||
| public void sendToTrackingPlayersFiltered( | ||
| Packet<? super ClientGamePacketListener> packet, | ||
| Predicate<ServerPlayer> predicate | ||
| ) { | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| trackerEntry.sendPairingData(viewer,packet -> viewer.connection.send(packet) |
There was a problem hiding this comment.
unvanish() calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
Rule Used: THE CENTRAL RULE everything else serves: maintain ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: platforms/fabric-26/src/main/java/gg/modl/minecraft/fabric/v26/handler/FabricStaffModeHandler.java
Line: 227-256
Comment:
**Unconditional tracker pairing**
`unvanish()` calls this for every online player, but this new vanilla-packet path sends player-list and tracker pairing data without checking that the viewer is in the same world or inside the normal tracking range. If a vanished staff member unvanishes in another dimension or far away, this can spawn a ghost player entity for viewers who vanilla tracking would not pair with at all. Gate this send through the same world/range conditions, or use the real server tracker’s pairing decision instead of sending a fresh tracker entry to every viewer.
**Rule Used:** THE CENTRAL RULE everything else serves: maintain ... ([source](https://app.greptile.com/review/custom-context?memory=21719055-65fa-440f-b434-eba8d358d917))
How can I resolve this? If you propose a fix, please make it concise.|
This looks good to me but I'll wait for @byteful to handle this into dev branch for next update since there are some conflicts and it's not a bug that requires immediate attention. |
…ker packets
Confidence Score: 3/5
This should be fixed before merging.
All changed
FabricStaffModeHandlerversions need the same tracking eligibility guard.Important Files Changed
ServerEntitypairing data for the re-show path.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Fix unvanish skin layers and mod crashes..." | Re-trigger Greptile
Context used: