Swap the order of client and server tasks and merge the client tasks …

…into one block

fabric-client-gametest-api-v1/src/client/java/net/fabricmc/fabric/impl/client/gametest/ThreadingImpl.java

@@ -51,11 +51,13 @@
  *
  * <p>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.
  *
  * <p>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;
 

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);
 

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);