The reason these phases were chosen are to make client-server interaction in singleplayer as consistent as * possible. The task queues are when most packets are handled, and without them being run in sequence it would be - * unspecified whether a packet would be handled on the current tick until the next one. The server task queue is before - * the client so that changes on the server appear on the client more readily. The test phase is run after the task - * queues rather than at the end of the physical tick (i.e. {@code MinecraftClient}'s and {@code MinecraftServer}'s - * {@code tick} methods), for no particular reason other than to avoid needing a 5th phase, and having a power of 2 - * number of phases is convenient when using {@linkplain Phaser}, as it doesn't break when the phase counter overflows. + * unspecified whether a packet would be handled on the current tick until the next one. The client task queue is before + * the server because the client task queue is run once per frame, rather than once per tick, meaning multiple frames + * could run before a tick happens. We want the task queue processing for these frames to essentially "merge" into one + * block of task queue processing. The test phase is run after the task queues rather than at the end of the physical + * tick (i.e. {@code MinecraftClient}'s and {@code MinecraftServer}'s {@code tick} methods), for no particular reason + * other than to avoid needing a 5th phase, and having a power of 2 number of phases is convenient when using + * {@linkplain Phaser}, as it doesn't break when the phase counter overflows. * *
Other challenges include that a client or server can be started during {@linkplain #PHASE_TEST} but haven't * reached their semaphore code yet meaning they are unable to accept tasks. This is solved by setting a flag to true @@ -76,8 +78,8 @@ private ThreadingImpl() { private static final String TASK_ON_OTHER_THREAD_METHOD_NAME = "runTaskOnOtherThread"; public static final int PHASE_TICK = 0; - public static final int PHASE_SERVER_TASKS = 1; - public static final int PHASE_CLIENT_TASKS = 2; + public static final int PHASE_CLIENT_TASKS = 1; + public static final int PHASE_SERVER_TASKS = 2; public static final int PHASE_TEST = 3; private static final int PHASE_MASK = 3; diff --git a/fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/gametest/MinecraftClientMixin.java b/fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/gametest/MinecraftClientMixin.java index a191a25c31..670d60b7e0 100644 --- a/fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/gametest/MinecraftClientMixin.java +++ b/fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/gametest/MinecraftClientMixin.java @@ -46,6 +46,8 @@ public class MinecraftClientMixin { @Unique private boolean startedClientGametests = false; @Unique + private boolean inMergedRunTasksLoop = false; + @Unique private Runnable deferredTask = null; @Shadow @@ -100,8 +102,9 @@ private int captureTicksPerFrame(int capturedTicksPerFrame, @Share("ticksPerFram @Inject(method = "render", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/MinecraftClient;runTasks()V")) private void preRunTasksHook(CallbackInfo ci, @Share("ticksPerFrame") LocalIntRef ticksPerFrame) { - // only do our per-tick locking if there will actually be a tick - if (ticksPerFrame.get() > 0) { + // "merge" multiple possible iterations of runTasks into one block from the point of view of locking + if (!inMergedRunTasksLoop) { + inMergedRunTasksLoop = true; preRunTasks(); } @@ -111,9 +114,10 @@ private void preRunTasksHook(CallbackInfo ci, @Share("ticksPerFrame") LocalIntRe @Inject(method = "render", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/MinecraftClient;runTasks()V", shift = At.Shift.AFTER)) private void postRunTasksHook(CallbackInfo ci, @Share("ticksPerFrame") LocalIntRef ticksPerFrame) { - // only do our per-tick locking if there will actually be a tick + // end our "merged" runTasks block if there is going to be a tick this frame if (ticksPerFrame.get() > 0) { postRunTasks(); + inMergedRunTasksLoop = false; } } @@ -151,13 +155,15 @@ private void onDisconnectBusyWait(CallbackInfo ci) { @Unique private void preRunTasks() { - ThreadingImpl.enterPhase(ThreadingImpl.PHASE_SERVER_TASKS); - // server tasks happen here ThreadingImpl.enterPhase(ThreadingImpl.PHASE_CLIENT_TASKS); } @Unique private void postRunTasks() { + ThreadingImpl.enterPhase(ThreadingImpl.PHASE_SERVER_TASKS); + + // server tasks happen here + ThreadingImpl.clientCanAcceptTasks = true; ThreadingImpl.enterPhase(ThreadingImpl.PHASE_TEST); diff --git a/fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/gametest/MinecraftServerMixin.java b/fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/gametest/MinecraftServerMixin.java index 6bc401e7d8..cda44e0cab 100644 --- a/fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/gametest/MinecraftServerMixin.java +++ b/fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/gametest/MinecraftServerMixin.java @@ -60,14 +60,13 @@ protected void onCrash(CallbackInfo ci) { @Inject(method = "runServer", at = @At(value = "INVOKE", target = "Lnet/minecraft/server/MinecraftServer;runTasksTillTickEnd()V")) private void preRunTasks(CallbackInfo ci) { + ThreadingImpl.enterPhase(ThreadingImpl.PHASE_CLIENT_TASKS); + // client tasks happen here ThreadingImpl.enterPhase(ThreadingImpl.PHASE_SERVER_TASKS); } @Inject(method = "runServer", at = @At(value = "INVOKE", target = "Lnet/minecraft/server/MinecraftServer;runTasksTillTickEnd()V", shift = At.Shift.AFTER)) private void postRunTasks(CallbackInfo ci) { - ThreadingImpl.enterPhase(ThreadingImpl.PHASE_CLIENT_TASKS); - // client tasks happen here - ThreadingImpl.serverCanAcceptTasks = true; ThreadingImpl.enterPhase(ThreadingImpl.PHASE_TEST);