Custom Fabric packets aren't handled in camera path mode
Draylar opened this issue ยท 0 comments
Version: Fabric 1.17
Issue: When viewing a camera path, custom Fabric packets (sent by ServerPlayNetworking#send(...)
) are ignored.
From what I can see, the problem is simple: path playback runs on the main thread...
ReplayMod/src/main/java/com/replaymod/replay/FullReplaySender.java
Lines 177 to 186 in 0aa9d57
and Fabric's ClientPlayNetworkAddon
(responsible for sending packet requests to handlers) requires packets be handled off-thread:
// Do not handle the packet on game thread
if (this.client.isOnThread()) {
return false;
}
Reproducing
Here is a quick test suite for Fabric 1.17. Start recording, punch a block a few times, then go back and replay in the standard player as well as the camera path playback mode.
@Override
public void onInitialize() {
// Log test:packet when handled by the client.
ClientPlayNetworking.registerGlobalReceiver(new Identifier("test", "packet"), (client, handler, packet, sender) -> {
System.out.println("hi from the client networking thread");
});
// When the player hits a block (server-side), send a packet to the client.
AttackBlockCallback.EVENT.register((player, world, hand, pos, direction) -> {
if(!world.isClient) {
PacketByteBuf packet = PacketByteBufs.create();
ServerPlayNetworking.send((ServerPlayerEntity) player, new Identifier("test", "packet"), packet);
}
return ActionResult.PASS;
});
}
Standard play mode will log the packet without issue, while the camera path viewer will spam "Unknown custom packed identifier".
Fixing
My personal patch fix, which has seemingly solved the issue for my team & I, has been to simply toggle off the thread check when reading packets in the replay viewer. Does this probably break something? Absolutely! Has it broken yet? No! ;)
I'm sure the final fix will not model this exactly, but it showcases one method of getting around the issue with a patch mod. A more ideal solution would be only disabling the thread-skip when running in synchronous mode, but that's a decision better made by someone who knows more about game networking than me.
@Mixin(FullReplaySender.class)
public class FullReplaySenderMixin {
@Inject(method = "channelRead", at = @At("HEAD"), remap = false)
private void removeThreadCheck(ChannelHandlerContext context, Object message, CallbackInfo ci) {
NetworkHandlerExtensions extensions = (NetworkHandlerExtensions) MinecraftClient.getInstance().getNetworkHandler();
if(extensions != null) {
((ThreadCheckExtensions) extensions.getAddon()).setSkipCheck(true);
}
}
@Inject(method = "channelRead", at = @At("RETURN"), remap = false)
private void reinstateThreadCheck(ChannelHandlerContext context, Object message, CallbackInfo ci) {
NetworkHandlerExtensions extensions = (NetworkHandlerExtensions) MinecraftClient.getInstance().getNetworkHandler();
if(extensions != null) {
((ThreadCheckExtensions) extensions.getAddon()).setSkipCheck(false);
}
}
}
@Mixin(ClientPlayNetworkAddon.class)
public class ClientPlayNetworkAddonMixin implements ThreadCheckExtensions {
@Shadow @Final private MinecraftClient client;
@Unique private boolean skip = false;
@Redirect(method = "handle", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/MinecraftClient;isOnThread()Z"))
private boolean modifyThreadCheck(MinecraftClient instance) {
if(skip) {
return false;
}
return client.isOnThread();
}
@Override
public void setSkipCheck(boolean skip) {
this.skip = skip;
}
}
If you have any additional questions, or there is a way I can be of assistance in solving this issue further, please let me know.