Rework the flow of events to ensure clients verify mods AFTER the ser…

…ver has sent them! (Fabric's server JOIN event fires too late so client needs a custom event to check after it)

src/main/java/com/sollace/fabwork/impl/FabworkClientImpl.java

@@ -1,8 +1,11 @@
 package com.sollace.fabwork.impl;
 
 import com.sollace.fabwork.api.client.ModProvisionCallback;
+import com.sollace.fabwork.impl.event.ClientConnectionEvents;
 
+import java.util.Set;
 import java.util.concurrent.*;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.apache.logging.log4j.LogManager;
@@ -14,6 +17,7 @@
 import net.fabricmc.fabric.api.client.networking.v1.*;
 import net.fabricmc.fabric.api.networking.v1.PacketByteBufs;
 import net.fabricmc.loader.api.FabricLoader;
+import net.minecraft.client.network.ClientPlayNetworkHandler;
 
 public class FabworkClientImpl implements ClientModInitializer {
     private static final Logger LOGGER = LogManager.getLogger("Fabwork::CLIENT");
@@ -22,14 +26,16 @@ public class FabworkClientImpl implements ClientModInitializer {
     private static SynchronisationState STATE = EMPTY_STATE;
     public static final FabworkClient INSTANCE = () -> STATE.installedOnServer().stream();
 
-    private static final Executor WAITER = CompletableFuture.delayedExecutor(300, TimeUnit.MILLISECONDS);
+    private static final int MAX_RETRIES = 5;
+    private static final long VERIFY_DELAY = 300;
+
+    private static final Executor WAITER = CompletableFuture.delayedExecutor(VERIFY_DELAY, TimeUnit.MILLISECONDS);
 
     @Override
     public void onInitializeClient() {
         if (!FabworkConfig.INSTANCE.get().disableLoginProtocol) {
             ClientPlayConnectionEvents.INIT.register((handler, client) -> {
                 LoaderUtil.invokeUntrusted(() -> {
-                    LOGGER.info("Client provisioned new connection {}", handler.hashCode());
                     STATE.installedOnServer().forEach(entry -> {
                         ModProvisionCallback.EVENT.invoker().onModProvisioned(entry, false);
                     });
@@ -40,25 +46,33 @@ public void onInitializeClient() {
             ClientPlayNetworking.registerGlobalReceiver(FabworkServer.CONSENT_ID, (client, handler, buffer, response) -> {
                 LoaderUtil.invokeUntrusted(() -> {
                     STATE = new SynchronisationState(FabworkImpl.INSTANCE.getInstalledMods(), ModEntryImpl.read(buffer));
-                    LOGGER.info("Responding to server sync packet {}", handler.hashCode());
+                    LOGGER.info("Got mod list from server: {}", ModEntriesUtil.stringify(STATE.installedOnServer()));
+                    Set<String> serverModIds = STATE.installedOnServer().stream().map(ModEntryImpl::modId).distinct().collect(Collectors.toSet());
                     response.sendPacket(FabworkServer.CONSENT_ID, ModEntryImpl.write(
-                            FabworkImpl.INSTANCE.getInstalledMods().filter(ModEntryImpl::requiredOnEither),
+                            FabworkImpl.INSTANCE.getInstalledMods().filter(entry -> entry.requiredOnEither() || serverModIds.contains(entry.modId())),
                             PacketByteBufs.create())
                     );
                 }, "Responding to server sync packet");
             });
-            ClientPlayConnectionEvents.JOIN.register((handler, sender, client) -> {
-                LoaderUtil.invokeUntrusted(() -> {
-                    LOGGER.info("Entered play state. Server has 300ms to respond {}", handler.hashCode());
-                    CompletableFuture.runAsync(() -> {
-                        LOGGER.info("Performing verify of server's installed mods {}", handler.hashCode());
-                        STATE.verify(handler.getConnection(), LOGGER, true);
-                    }, WAITER);
-                }, "Entering play state");
+            ClientConnectionEvents.CONNECT.register((handler, sender, client) -> {
+                LoaderUtil.invokeUntrusted(() -> delayVerify(handler, MAX_RETRIES), "Entering play state");
             });
         }
         LoaderUtil.invokeEntryPoints("fabwork:client", ClientModInitializer.class, ClientModInitializer::onInitializeClient);
 
-        LOGGER.info("Loaded Fabwork " + FabricLoader.getInstance().getModContainer("fabwork").get().getMetadata().getVersion().getFriendlyString());
+        LOGGER.info("Loaded Fabwork {}", FabricLoader.getInstance().getModContainer("fabwork").get().getMetadata().getVersion().getFriendlyString());
+    }
+
+    private void delayVerify(ClientPlayNetworkHandler handler, int retries) {
+        CompletableFuture.runAsync(() -> {
+            LoaderUtil.invokeUntrusted(() -> {
+                if (STATE == EMPTY_STATE && retries > 0) {
+                    LOGGER.info("Server has not responded. Retrying ({}/{})", (MAX_RETRIES - retries) + 1, MAX_RETRIES);
+                    delayVerify(handler, retries - 1);
+                } else {
+                    STATE.verify(handler.getConnection(), LOGGER, true);
+                }
+            }, "Verifying host mods retry=" + (MAX_RETRIES - retries));
+        }, WAITER);
     }
 }

src/main/java/com/sollace/fabwork/impl/FabworkConfig.java

@@ -56,6 +56,7 @@ private static FabworkConfig save(@Nullable FabworkConfig config, Path path) {
 
     public boolean doNotEnforceModMatching;
     public boolean disableLoginProtocol;
+    public boolean allowUnmoddedClients;
 
     public Stream<ModEntryImpl> getCustomRequiredMods() {
         if (requiredModIds == null || requiredModIds.isEmpty()) {

src/main/java/com/sollace/fabwork/impl/FabworkServer.java

@@ -10,6 +10,7 @@
 import com.google.common.collect.Streams;
 import com.sollace.fabwork.api.Fabwork;
 import com.sollace.fabwork.impl.PlayPingSynchroniser.ResponseType;
+import com.sollace.fabwork.impl.event.ServerConnectionEvents;
 
 import net.fabricmc.api.ModInitializer;
 import net.fabricmc.fabric.api.networking.v1.*;
@@ -35,29 +36,39 @@ public void onInitialize() {
         if (!config.disableLoginProtocol) {
             ServerPlayNetworking.registerGlobalReceiver(CONSENT_ID, (server, player, handler, buffer, response) -> {
                 LoaderUtil.invokeUntrusted(() -> {
-                    LOGGER.info("Received synchronize response from client " + handler.getConnection().getAddress().toString());
-                    clientLoginStates.put(handler.getConnection(), new SynchronisationState(ModEntryImpl.read(buffer), emptyState.installedOnServer().stream()));
+                    SynchronisationState state = new SynchronisationState(ModEntryImpl.read(buffer), emptyState.installedOnServer().stream());
+                    LOGGER.info("Got mod list from {}[{}]: {}", player.getName().getString(), handler.getConnection().getAddress(), ModEntriesUtil.stringify(state.installedOnClient()));
+                    clientLoginStates.put(handler.getConnection(), state);
                 }, "Received synchronize response from client");
             });
+
             ServerPlayConnectionEvents.JOIN.register((handler, sender, server) -> {
                 LoaderUtil.invokeUntrusted(() -> {
-                    LOGGER.info("Sending synchronize packet to {}", handler.getConnection().getAddress());
+                    LOGGER.info("Sending mod list to {}[{}]", handler.getPlayer().getName().getString(), handler.getConnection().getAddress());
                     sender.sendPacket(CONSENT_ID, ModEntryImpl.write(
                             emptyState.installedOnServer().stream(),
                             PacketByteBufs.create())
                     );
+                }, "Sending synchronize packet");
+            });
 
+            ServerConnectionEvents.CONNECT.register((handler, sender, server) -> {
+                LoaderUtil.invokeUntrusted(() -> {
                     PlayPingSynchroniser.waitForClientResponse(handler.getConnection(), responseType -> {
                         if (responseType == ResponseType.COMPLETED) {
-                            LOGGER.info("Performing verify of client's installed mods {}", handler.getConnection().getAddress());
                             if (clientLoginStates.containsKey(handler.getConnection())) {
                                 clientLoginStates.remove(handler.getConnection()).verify(handler.getConnection(), LOGGER, true);
                             } else {
-                                LOGGER.warn("Client failed to respond to challenge. Assuming vanilla client {}", handler.getConnection().getAddress());
-                                emptyState.verify(handler.getConnection(), LOGGER, false);
+                                LOGGER.warn("{}[{}] did not send a mod list. They may not have fabwork installed", handler.getPlayer().getName().getString(), handler.getConnection().getAddress());
+                                if (config.allowUnmoddedClients) {
+                                    LOGGER.warn("Connection to {}[{}] has been force permitted by server configuration. They are allowed to join checking installed mods! Their game may be broken upon joining!", handler.getPlayer().getName().getString(), handler.getConnection().getAddress());
+                                } else {
+                                    emptyState.verify(handler.getConnection(), LOGGER, false);
+                                }
                             }
                         } else {
-                            LOGGER.warn("Failed to receive response from client. {} ConnectionState: {}",
+                            LOGGER.warn("Failed to receive response from client. {}[{}] ConnectionState: {}",
+                                    handler.getPlayer().getName().getString(),
                                     handler.getConnection().getAddress(),
                                     handler.getConnection().isOpen() ? " OPEN" : " CLOSED"
                             );

src/main/java/com/sollace/fabwork/impl/ModEntriesUtil.java

@@ -0,0 +1,28 @@
+package com.sollace.fabwork.impl;
+
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import com.sollace.fabwork.api.ModEntry;
+
+public interface ModEntriesUtil {
+
+    static Set<String> compare(Stream<ModEntryImpl> provided, List<ModEntryImpl> required) {
+        return provided
+                .map(ModEntry::modId)
+                .filter(id -> required.stream().filter(cc -> cc.modId().equalsIgnoreCase(id)).findAny().isEmpty())
+                .distinct()
+                .collect(Collectors.toSet());
+    }
+
+    static Set<String> ids(List<ModEntryImpl> entries) {
+        return entries.stream().map(ModEntry::modId).distinct().collect(Collectors.toSet());
+    }
+
+    static String stringify(List<ModEntryImpl> entries) {
+        String[] values = entries.stream().map(ModEntry::modId).toArray(String[]::new);
+        return " [" + String.join(", ", values) + "] (" + values.length + ")";
+    }
+}

src/main/java/com/sollace/fabwork/impl/SynchronisationState.java

@@ -2,7 +2,6 @@
 
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.apache.logging.log4j.Logger;
@@ -22,8 +21,8 @@ public SynchronisationState(Stream<ModEntryImpl> installedOnClient, Stream<ModEn
     }
 
     public boolean verify(ClientConnection connection, Logger logger, boolean useTranslation) {
-        Set<String> missingOnServer = getDifference(installedOnClient.stream().filter(c -> c.requirement().isRequiredOnServer()), installedOnServer);
-        Set<String> missingOnClient = getDifference(installedOnServer.stream().filter(c -> c.requirement().isRequiredOnClient()), installedOnClient);
+        Set<String> missingOnServer = ModEntriesUtil.compare(installedOnClient.stream().filter(c -> c.requirement().isRequiredOnServer()), installedOnServer);
+        Set<String> missingOnClient = ModEntriesUtil.compare(installedOnServer.stream().filter(c -> c.requirement().isRequiredOnClient()), installedOnClient);
 
         installedOnServer.stream().forEach(entry -> {
             ModProvisionCallback.EVENT.invoker().onModProvisioned(entry, !missingOnClient.contains(entry.modId()));
@@ -47,14 +46,6 @@ public boolean verify(ClientConnection connection, Logger logger, boolean useTra
         return true;
     }
 
-    private Set<String> getDifference(Stream<ModEntryImpl> provided, List<ModEntryImpl> required) {
-        return provided
-                .map(ModEntry::modId)
-                .filter(id -> required.stream().filter(cc -> cc.modId().equalsIgnoreCase(id)).findAny().isEmpty())
-                .distinct()
-                .collect(Collectors.toSet());
-    }
-
     private Text createErrorMessage(Set<String> missingOnServer, Set<String> missingOnClient, boolean useTranslation) {
         String serverMissing = String.join(", ", missingOnServer.stream().toArray(CharSequence[]::new));
         String clientMissing = String.join(", ", missingOnClient.stream().toArray(CharSequence[]::new));

src/main/java/com/sollace/fabwork/impl/event/ClientConnectionEvents.java

@@ -0,0 +1,18 @@
+package com.sollace.fabwork.impl.event;
+
+import net.fabricmc.fabric.api.client.networking.v1.ClientPlayConnectionEvents.Join;
+import net.fabricmc.fabric.api.event.Event;
+import net.fabricmc.fabric.api.event.EventFactory;
+
+public interface ClientConnectionEvents {
+    /**
+     * An event for notification when a player has successfully connected to the server.
+     * <p>
+     * This event is triggered after JOIN once initial game data has been sent to the client.
+     */
+    Event<Join> CONNECT = EventFactory.createArrayBacked(Join.class, callbacks -> (handler, sender, server) -> {
+        for (Join callback : callbacks) {
+            callback.onPlayReady(handler, sender, server);
+        }
+    });
+}

src/main/java/com/sollace/fabwork/impl/event/ServerConnectionEvents.java

@@ -0,0 +1,18 @@
+package com.sollace.fabwork.impl.event;
+
+import net.fabricmc.fabric.api.event.Event;
+import net.fabricmc.fabric.api.event.EventFactory;
+import net.fabricmc.fabric.api.networking.v1.ServerPlayConnectionEvents.Join;
+
+public interface ServerConnectionEvents {
+    /**
+     * An event for notification when a player has successfully connected to the server.
+     * <p>
+     * This event is triggered after JOIN but before the corresponding event packet is dispatched to the client.
+     */
+    Event<Join> CONNECT = EventFactory.createArrayBacked(Join.class, callbacks -> (handler, sender, server) -> {
+        for (Join callback : callbacks) {
+            callback.onPlayReady(handler, sender, server);
+        }
+    });
+}

src/main/java/com/sollace/fabwork/impl/mixin/PlayerManagerMixin.java

@@ -0,0 +1,21 @@
+package com.sollace.fabwork.impl.mixin;
+
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+import com.sollace.fabwork.impl.event.ServerConnectionEvents;
+
+import net.fabricmc.fabric.api.networking.v1.ServerPlayNetworking;
+import net.minecraft.network.ClientConnection;
+import net.minecraft.server.PlayerManager;
+import net.minecraft.server.network.ServerPlayerEntity;
+
+@Mixin(PlayerManager.class)
+abstract class PlayerManagerMixin {
+    @Inject(method = "onPlayerConnect", at = @At(value = "INVOKE", target = "Lnet/minecraft/network/packet/s2c/play/SynchronizeTagsS2CPacket;<init>(Ljava/util/Map;)V"))
+    private void handlePlayerConnection(ClientConnection connection, ServerPlayerEntity player, CallbackInfo ci) {
+        ServerConnectionEvents.CONNECT.invoker().onPlayReady(player.networkHandler, ServerPlayNetworking.getSender(player), player.getServer());
+    }
+}

src/main/java/com/sollace/fabwork/impl/mixin/client/ClientPlayNetworkHandlerMixin.java

@@ -0,0 +1,22 @@
+package com.sollace.fabwork.impl.mixin.client;
+
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+import com.sollace.fabwork.impl.event.ClientConnectionEvents;
+
+import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking;
+import net.minecraft.client.MinecraftClient;
+import net.minecraft.client.network.ClientPlayNetworkHandler;
+import net.minecraft.network.packet.s2c.play.SynchronizeRecipesS2CPacket;
+
+@Mixin(value = ClientPlayNetworkHandler.class, priority = 999)
+abstract class ClientPlayNetworkHandlerMixin {
+    @Inject(method = "onSynchronizeRecipes", at = @At("RETURN"))
+    private void onOnSynchronizeRecipes(SynchronizeRecipesS2CPacket packet, CallbackInfo cinfo) {
+        final MinecraftClient client = MinecraftClient.getInstance();
+        ClientConnectionEvents.CONNECT.invoker().onPlayReady(client.player.networkHandler, ClientPlayNetworking.getSender(), client);
+    }
+}

src/main/resources/fabwork.mixin.json

@@ -5,9 +5,11 @@
   "refmap": "fabwork.mixin.refmap.json",
   "compatibilityLevel": "JAVA_17",
   "mixins": [
-    "ServerPlayNetworkHandlerMixin"
+    "ServerPlayNetworkHandlerMixin",
+    "PlayerManagerMixin"
   ],
   "client": [
+    "client.ClientPlayNetworkHandlerMixin"
   ],
   "injectors": {
     "defaultRequire": 1