From 4b234f8c10b6751530062e6d9882f7f358be77c0 Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Wed, 28 Feb 2024 18:05:41 +0100 Subject: [PATCH 1/9] Improved ModMismatchDisconnectedScreen and clientside handling of channel mismatches in general --- ...ConfigurationPacketListenerImpl.java.patch | 4 +- .../gui/ModMismatchDisconnectedScreen.java | 110 +++++++++++++----- 2 files changed, 83 insertions(+), 31 deletions(-) diff --git a/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch b/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch index 2c22884764..036f6874b4 100644 --- a/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch +++ b/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch @@ -10,7 +10,7 @@ public ClientConfigurationPacketListenerImpl(Minecraft p_295262_, Connection p_296339_, CommonListenerCookie p_294706_) { super(p_295262_, p_296339_, p_294706_); -@@ -45,7 +_,41 @@ +@@ -45,7 +_,43 @@ } @Override @@ -38,6 +38,8 @@ + } + if (p_295411_ instanceof net.neoforged.neoforge.network.payload.ModdedNetworkSetupFailedPayload setupFailedPayload) { + failureReasons = setupFailedPayload.failureReasons(); ++ failureReasons.forEach((r, c) -> LOGGER.warn("Channel [{}] failed to connect: {}", r, c.getString())); ++ return; + } + if (!this.connectionType.isNeoForge() && p_295411_ instanceof net.minecraft.network.protocol.common.custom.BrandPayload) { + this.initializedConnection = true; diff --git a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java index fbe90ddadf..b8b4b3549b 100644 --- a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java +++ b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java @@ -10,6 +10,8 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Comparator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -19,6 +21,7 @@ import net.minecraft.client.Minecraft; import net.minecraft.client.gui.GuiGraphics; import net.minecraft.client.gui.components.Button; +import net.minecraft.client.gui.components.MultiLineLabel; import net.minecraft.client.gui.narration.NarrationElementOutput; import net.minecraft.client.gui.screens.Screen; import net.minecraft.network.chat.ClickEvent; @@ -30,6 +33,7 @@ import net.minecraft.network.chat.TextColor; import net.minecraft.resources.ResourceLocation; import net.minecraft.util.FormattedCharSequence; +import net.neoforged.fml.ModContainer; import net.neoforged.fml.ModList; import net.neoforged.fml.loading.FMLPaths; import net.neoforged.neoforge.client.gui.widget.ScrollPanel; @@ -37,6 +41,8 @@ import org.apache.commons.lang3.tuple.Pair; public class ModMismatchDisconnectedScreen extends Screen { + private final Component reason; + private MultiLineLabel message = MultiLineLabel.EMPTY; private final Screen parent; private int textHeight; private final Path modsDir; @@ -44,8 +50,9 @@ public class ModMismatchDisconnectedScreen extends Screen { private final int listHeight = 140; private final Map mismatchedChannelData; - public ModMismatchDisconnectedScreen(Screen parentScreen, Component title, Map mismatchedChannelData) { - super(title); + public ModMismatchDisconnectedScreen(Screen parentScreen, Component reason, Map mismatchedChannelData) { + super(Component.translatable("disconnect.lost")); + this.reason = reason; this.parent = parentScreen; this.modsDir = FMLPaths.MODSDIR.get(); this.logFile = FMLPaths.GAMEDIR.get().resolve(Paths.get("logs", "latest.log")); @@ -56,8 +63,12 @@ public ModMismatchDisconnectedScreen(Screen parentScreen, Component title, Map> collapsedChannelData = collapseChannelData(mismatchedChannelData); record Row(MutableComponent name, MutableComponent reason) {} + //The raw list of the strings in a table row, the components may still be too long for the final table and will be split up later. The first row element may have a style assigned to it that will be used for the whole content row. List rows = new ArrayList<>(); - if (!mismatchedChannelData.isEmpty()) { - //This table section contains the mod name and both mod versions of each mod that has a mismatching client and server version + if (!collapsedChannelData.isEmpty()) { + //Each table row contains the mod name and the reason for the corresponding channel mismatch. rows.add(new Row(Component.translatable("fml.modmismatchscreen.table.channelname"), Component.translatable("fml.modmismatchscreen.table.reason"))); int i = 0; - for (Map.Entry modData : mismatchedChannelData.entrySet()) { - rows.add(new Row(toChannelNameComponent(modData.getKey()), modData.getValue().copy())); - if (++i >= 10) { + for (Map.Entry> channelData : collapsedChannelData.entrySet()) { + rows.add(new Row(toChannelNameComponent(channelData.getKey(), channelData.getValue().getLeft(), i % 2 == 0 ? ChatFormatting.GOLD : ChatFormatting.YELLOW), channelData.getValue().getRight().copy())); + if (++i == 20 && collapsedChannelData.size() > 20) { //If too many mismatched mod entries are present, append a line referencing how to see the full list and stop rendering any more entries - rows.add(new Row(Component.literal(""), Component.translatable("fml.modmismatchscreen.additional", mismatchedChannelData.size() - i).withStyle(ChatFormatting.ITALIC))); + rows.add(new Row(Component.literal(""), Component.translatable("fml.modmismatchscreen.additional", collapsedChannelData.size() - i).withStyle(ChatFormatting.ITALIC))); break; } } @@ -114,7 +126,41 @@ record Row(MutableComponent name, MutableComponent reason) {} } /** - * Splits the raw name and version strings, making them use multiple lines if needed, to fit within the table dimensions. + * Collapses quasi-duplicate channel mismatch entries into single list entries to reduce repetition of entries in the final list. + * Quasi-duplicate channel mismatch entries share the same channel namespace and mismatch reasons, and it is thus very likely that they are caused + * by one and the same mod (that has registered all of these channels) missing/mismatching between client and server. + * + * @param mismatchedChannelData The raw mismatched channel data received from the server, which might contain quasi-duplicate entries + * @return A map containing deduplicated channel mismatch entries. For each quasi-duplicate group, only the first encountered channel id is kept, + * and all other quasi-duplicate channels then increment the associated repetition count that is mapped to that first channel id. + * Finally, the (unchanged) mismatch reason (which is the same for all quasi-duplicate entries) also gets mapped to the channel id. + */ + private Map> collapseChannelData(Map mismatchedChannelData) { + Map> repetitions = new LinkedHashMap<>(); + List sortedChannels = mismatchedChannelData.keySet().stream().sorted(Comparator.comparing(ResourceLocation::toString)).toList(); + for (ResourceLocation channel : sortedChannels) { + Component channelMismatchReason = mismatchedChannelData.get(channel); + List namespaceChannels = repetitions.keySet().stream().filter(r -> r.getNamespace().equals(channel.getNamespace())).toList(); + boolean matched = false; + if (!namespaceChannels.isEmpty()) { + for (ResourceLocation potentialRepetitionChannel : namespaceChannels) { + Pair repetitionData = repetitions.get(potentialRepetitionChannel); + + if (repetitionData.getRight().equals(channelMismatchReason)) { + repetitions.put(potentialRepetitionChannel, Pair.of(repetitionData.getLeft() + 1, repetitionData.getRight())); + matched = true; + } + } + } + if (!matched) + repetitions.put(channel, Pair.of(1, channelMismatchReason)); + } + + return repetitions; + } + + /** + * Splits the raw mod name and mismatch reason strings, making them use multiple lines if needed, to fit within the table dimensions. * The style assigned to the name element is then applied to the entire content row. * * @param name The first element of the content row, usually representing a table section header or the name of a mod entry @@ -123,9 +169,7 @@ record Row(MutableComponent name, MutableComponent reason) {} */ private List> splitLineToWidth(MutableComponent name, MutableComponent reason) { Style style = name.getStyle(); - int versionColumns = 1; - int adaptedNameWidth = nameWidth + versionWidth * (2 - versionColumns) - 4; //the name width may be expanded when the version column string is missing - List nameLines = font.split(name, adaptedNameWidth); + List nameLines = font.split(name, nameWidth - 4); List reasonLines = font.split(reason.setStyle(style), versionWidth - 4); List> splitLines = new ArrayList<>(); @@ -137,25 +181,31 @@ private List> splitLineToWidt } /** - * Adds a style information to the given mod name string. The style assigned to the returned component contains the color of the mod name, - * a hover event containing the given id, and an optional click event, which opens the homepage of mod, if present. + * Uses the given channel id to return a component with the name of the mod that likely owns the channel. If no such mod is found, the namespace of the channel id is used instead. + * The style assigned to the returned component contains the color of the entry, a hover event containing the channel id, and an optional click event which, if present, opens the homepage of the mod. * - * @param id An id that gets displayed in the hover event. Depending on the origin it may only consist of a namespace (the mod id) or a namespace + path (a channel id associated with the mod). - * @return A component with the mod name as the main text component, and an assigned style which will be used for the whole content row. + * @param id The id of the mismatched channel. Used to query the name of the mod that has likely registered the channel in order to use and display its name and homepage URL. + * @param repetitionCount How many other channels of the same mod failed negotiation with the same error message. Displayed in the hover tooltip. + * @param color Defines the color of the returned channel name component. + * @return A component with the mod name (if available) as the main text component, and an assigned color which will be used for the whole content row. */ - private MutableComponent toChannelNameComponent(ResourceLocation id) { + private MutableComponent toChannelNameComponent(ResourceLocation id, int repetitionCount, ChatFormatting color) { String modId = id.getNamespace(); - - String url = ModList.get().getModContainerById(modId) - .flatMap(container -> container.getModInfo().getModURL()) + Optional mod = ModList.get().getModContainerById(modId); + String name = mod.map(m -> m.getModInfo().getDisplayName()).orElse(modId); + String url = mod.flatMap(container -> container.getModInfo().getModURL()) .map(URL::toString) .orElse(""); - MutableComponent result = Component.literal(id.toString()).withStyle(ChatFormatting.YELLOW); + MutableComponent result = Component.literal(name).withStyle(color); + MutableComponent hoverText = Component.literal(id.toString()); + if (repetitionCount > 1) + hoverText.append(Component.literal(" [+%s more]".formatted(repetitionCount)).withStyle(ChatFormatting.GRAY)); if (!url.isEmpty()) { - result = result.withStyle(s -> s.withHoverEvent(new HoverEvent(Action.SHOW_TEXT, Component.translatable("fml.modmismatchscreen.table.visit.mod_page", id.toString())))) - .withStyle(s -> s.withClickEvent(new ClickEvent(ClickEvent.Action.OPEN_URL, url))); + hoverText.append(Component.literal("\n").append(Component.translatable("fml.modmismatchscreen.table.visit.mod_page", id.toString()))); + result = result.withStyle(s -> s.withClickEvent(new ClickEvent(ClickEvent.Action.OPEN_URL, url))); } + result = result.withStyle(s -> s.withHoverEvent(new HoverEvent(Action.SHOW_TEXT, hoverText))); return result; } From be7f25ae3dcde346a6bbdf1f131f6accb297b548 Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Wed, 28 Feb 2024 18:52:11 +0100 Subject: [PATCH 2/9] Ran the spotless task thingy --- .../client/gui/ModMismatchDisconnectedScreen.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java index b8b4b3549b..71c954409d 100644 --- a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java +++ b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java @@ -132,8 +132,8 @@ record Row(MutableComponent name, MutableComponent reason) {} * * @param mismatchedChannelData The raw mismatched channel data received from the server, which might contain quasi-duplicate entries * @return A map containing deduplicated channel mismatch entries. For each quasi-duplicate group, only the first encountered channel id is kept, - * and all other quasi-duplicate channels then increment the associated repetition count that is mapped to that first channel id. - * Finally, the (unchanged) mismatch reason (which is the same for all quasi-duplicate entries) also gets mapped to the channel id. + * and all other quasi-duplicate channels then increment the associated repetition count that is mapped to that first channel id. + * Finally, the (unchanged) mismatch reason (which is the same for all quasi-duplicate entries) also gets mapped to the channel id. */ private Map> collapseChannelData(Map mismatchedChannelData) { Map> repetitions = new LinkedHashMap<>(); @@ -184,9 +184,9 @@ private List> splitLineToWidt * Uses the given channel id to return a component with the name of the mod that likely owns the channel. If no such mod is found, the namespace of the channel id is used instead. * The style assigned to the returned component contains the color of the entry, a hover event containing the channel id, and an optional click event which, if present, opens the homepage of the mod. * - * @param id The id of the mismatched channel. Used to query the name of the mod that has likely registered the channel in order to use and display its name and homepage URL. + * @param id The id of the mismatched channel. Used to query the name of the mod that has likely registered the channel in order to use and display its name and homepage URL. * @param repetitionCount How many other channels of the same mod failed negotiation with the same error message. Displayed in the hover tooltip. - * @param color Defines the color of the returned channel name component. + * @param color Defines the color of the returned channel name component. * @return A component with the mod name (if available) as the main text component, and an assigned color which will be used for the whole content row. */ private MutableComponent toChannelNameComponent(ResourceLocation id, int repetitionCount, ChatFormatting color) { From 11ae1af33e9b57c3f1b0354c9e1e44139a105f3f Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Thu, 29 Feb 2024 16:22:29 +0100 Subject: [PATCH 3/9] Removed code that mapped a channel to the mod with the same namespace also updated some javadoc --- .../gui/ModMismatchDisconnectedScreen.java | 47 +++++++------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java index 71c954409d..bc0b4eba20 100644 --- a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java +++ b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java @@ -6,7 +6,6 @@ package net.neoforged.neoforge.client.gui; import com.mojang.blaze3d.vertex.Tesselator; -import java.net.URL; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -24,7 +23,6 @@ import net.minecraft.client.gui.components.MultiLineLabel; import net.minecraft.client.gui.narration.NarrationElementOutput; import net.minecraft.client.gui.screens.Screen; -import net.minecraft.network.chat.ClickEvent; import net.minecraft.network.chat.Component; import net.minecraft.network.chat.HoverEvent; import net.minecraft.network.chat.HoverEvent.Action; @@ -33,8 +31,6 @@ import net.minecraft.network.chat.TextColor; import net.minecraft.resources.ResourceLocation; import net.minecraft.util.FormattedCharSequence; -import net.neoforged.fml.ModContainer; -import net.neoforged.fml.ModList; import net.neoforged.fml.loading.FMLPaths; import net.neoforged.neoforge.client.gui.widget.ScrollPanel; import net.neoforged.neoforge.common.I18nExtension; @@ -107,7 +103,7 @@ record Row(MutableComponent name, MutableComponent reason) {} //The raw list of the strings in a table row, the components may still be too long for the final table and will be split up later. The first row element may have a style assigned to it that will be used for the whole content row. List rows = new ArrayList<>(); if (!collapsedChannelData.isEmpty()) { - //Each table row contains the mod name and the reason for the corresponding channel mismatch. + //Each table row contains the channel namespace and the reason for the corresponding channel mismatch. rows.add(new Row(Component.translatable("fml.modmismatchscreen.table.channelname"), Component.translatable("fml.modmismatchscreen.table.reason"))); int i = 0; for (Map.Entry> channelData : collapsedChannelData.entrySet()) { @@ -127,13 +123,14 @@ record Row(MutableComponent name, MutableComponent reason) {} /** * Collapses quasi-duplicate channel mismatch entries into single list entries to reduce repetition of entries in the final list. - * Quasi-duplicate channel mismatch entries share the same channel namespace and mismatch reasons, and it is thus very likely that they are caused - * by one and the same mod (that has registered all of these channels) missing/mismatching between client and server. + * Quasi-duplicate channel mismatch entries share the same channel namespace and mismatch reasons. Even though these mismatches do not need to be causally related, + * there is a not inconsiderable likelihood that they are, and to increase usability of the screen, we summarize them into one entry to allow mod users + * to focus on the likely important parts of the channel mismatch data (which is the namespace of the mismatching channels and the exact mismatch reason). * * @param mismatchedChannelData The raw mismatched channel data received from the server, which might contain quasi-duplicate entries * @return A map containing deduplicated channel mismatch entries. For each quasi-duplicate group, only the first encountered channel id is kept, * and all other quasi-duplicate channels then increment the associated repetition count that is mapped to that first channel id. - * Finally, the (unchanged) mismatch reason (which is the same for all quasi-duplicate entries) also gets mapped to the channel id. + * The mismatch reason (which remains unchanged and is the same for all quasi-duplicate entries) also gets mapped to the channel id. */ private Map> collapseChannelData(Map mismatchedChannelData) { Map> repetitions = new LinkedHashMap<>(); @@ -160,11 +157,11 @@ private Map> collapseChannelData(Map< } /** - * Splits the raw mod name and mismatch reason strings, making them use multiple lines if needed, to fit within the table dimensions. + * Splits the raw channel namespace and mismatch reason strings, making them use multiple lines if needed, to fit within the table dimensions. * The style assigned to the name element is then applied to the entire content row. * - * @param name The first element of the content row, usually representing a table section header or the name of a mod entry - * @param reason The second element of the content row, usually representing the reason why the mod is mismatched + * @param name The first element of the content row, usually representing a table section header or a channel namespace entry + * @param reason The second element of the content row, usually representing the reason why the channel is mismatched * @return A list of table rows consisting of 2 elements each which consist of the same content as was given by the parameters, but split up to fit within the table dimensions. */ private List> splitLineToWidth(MutableComponent name, MutableComponent reason) { @@ -181,32 +178,22 @@ private List> splitLineToWidt } /** - * Uses the given channel id to return a component with the name of the mod that likely owns the channel. If no such mod is found, the namespace of the channel id is used instead. - * The style assigned to the returned component contains the color of the entry, a hover event containing the channel id, and an optional click event which, if present, opens the homepage of the mod. + * Returns a component with the namespace of the given channel id. The component is colored in the given color, which will be used for the whole content row, + * and has a hover text mentioning an exemplary mismatching channel id plus how many other channels with the same namespace were mismatching with the same reason. * - * @param id The id of the mismatched channel. Used to query the name of the mod that has likely registered the channel in order to use and display its name and homepage URL. - * @param repetitionCount How many other channels of the same mod failed negotiation with the same error message. Displayed in the hover tooltip. - * @param color Defines the color of the returned channel name component. - * @return A component with the mod name (if available) as the main text component, and an assigned color which will be used for the whole content row. + * @param id The id of the exemplary mismatched channel. Its namespace is used for the component text. + * @param repetitionCount How many channels with the same namespace failed negotiation with the same error message. Displayed in the hover tooltip. + * @param color Defines the color of the returned channel namespace component. + * @return A component with the channel namespace as the main text component, and an assigned color which will be used for the whole content row. */ private MutableComponent toChannelNameComponent(ResourceLocation id, int repetitionCount, ChatFormatting color) { - String modId = id.getNamespace(); - Optional mod = ModList.get().getModContainerById(modId); - String name = mod.map(m -> m.getModInfo().getDisplayName()).orElse(modId); - String url = mod.flatMap(container -> container.getModInfo().getModURL()) - .map(URL::toString) - .orElse(""); - MutableComponent result = Component.literal(name).withStyle(color); + MutableComponent namespaceComponent = Component.literal(id.getNamespace()).withStyle(color); MutableComponent hoverText = Component.literal(id.toString()); if (repetitionCount > 1) hoverText.append(Component.literal(" [+%s more]".formatted(repetitionCount)).withStyle(ChatFormatting.GRAY)); - if (!url.isEmpty()) { - hoverText.append(Component.literal("\n").append(Component.translatable("fml.modmismatchscreen.table.visit.mod_page", id.toString()))); - result = result.withStyle(s -> s.withClickEvent(new ClickEvent(ClickEvent.Action.OPEN_URL, url))); - } - result = result.withStyle(s -> s.withHoverEvent(new HoverEvent(Action.SHOW_TEXT, hoverText))); - return result; + namespaceComponent = namespaceComponent.withStyle(s -> s.withHoverEvent(new HoverEvent(Action.SHOW_TEXT, hoverText))); + return namespaceComponent; } @Override From 0fea0c166d676648b555654a8d178d9f92f8a9da Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Wed, 6 Mar 2024 15:44:51 +0100 Subject: [PATCH 4/9] Reworked merging of MMDS to be based on the mods owning given channels - PlayRegistrations and ConfigurationRegistrations now store the id of the mod they were created from, which is retrieved via the ModLoadingContext - This mod id is used to enhance the channel mismatch reason component to always include the mod name (this enhancing is done on the side that the channel was registered on) - Since some of that reason component enhancing is done in the constructor of the MMDS, the logging of channel mismatch entries has been moved to there as well - There is now also a button that toggles listing only one (default) or all channels with the same reason component --- ...ConfigurationPacketListenerImpl.java.patch | 3 +- .../gui/ModMismatchDisconnectedScreen.java | 131 ++++++++++-------- .../NegotiableNetworkComponent.java | 1 + .../NetworkComponentNegotiator.java | 18 ++- .../ConfigurationRegistration.java | 1 + .../registration/ModdedPacketRegistrar.java | 9 +- .../network/registration/NetworkRegistry.java | 36 +++-- .../registration/PlayRegistration.java | 1 + .../resources/assets/neoforge/lang/en_us.json | 2 + 9 files changed, 129 insertions(+), 73 deletions(-) diff --git a/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch b/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch index 036f6874b4..c616402914 100644 --- a/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch +++ b/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch @@ -10,7 +10,7 @@ public ClientConfigurationPacketListenerImpl(Minecraft p_295262_, Connection p_296339_, CommonListenerCookie p_294706_) { super(p_295262_, p_296339_, p_294706_); -@@ -45,7 +_,43 @@ +@@ -45,7 +_,42 @@ } @Override @@ -38,7 +38,6 @@ + } + if (p_295411_ instanceof net.neoforged.neoforge.network.payload.ModdedNetworkSetupFailedPayload setupFailedPayload) { + failureReasons = setupFailedPayload.failureReasons(); -+ failureReasons.forEach((r, c) -> LOGGER.warn("Channel [{}] failed to connect: {}", r, c.getString())); + return; + } + if (!this.connectionType.isNeoForge() && p_295411_ instanceof net.minecraft.network.protocol.common.custom.BrandPayload) { diff --git a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java index bc0b4eba20..5a1047422c 100644 --- a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java +++ b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java @@ -5,7 +5,9 @@ package net.neoforged.neoforge.client.gui; +import com.google.common.collect.Lists; import com.mojang.blaze3d.vertex.Tesselator; +import com.mojang.logging.LogUtils; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -20,25 +22,31 @@ import net.minecraft.client.Minecraft; import net.minecraft.client.gui.GuiGraphics; import net.minecraft.client.gui.components.Button; +import net.minecraft.client.gui.components.CycleButton; import net.minecraft.client.gui.components.MultiLineLabel; import net.minecraft.client.gui.narration.NarrationElementOutput; import net.minecraft.client.gui.screens.Screen; import net.minecraft.network.chat.Component; -import net.minecraft.network.chat.HoverEvent; -import net.minecraft.network.chat.HoverEvent.Action; +import net.minecraft.network.chat.ComponentUtils; import net.minecraft.network.chat.MutableComponent; import net.minecraft.network.chat.Style; import net.minecraft.network.chat.TextColor; +import net.minecraft.network.chat.contents.TranslatableContents; import net.minecraft.resources.ResourceLocation; import net.minecraft.util.FormattedCharSequence; +import net.neoforged.fml.ModList; import net.neoforged.fml.loading.FMLPaths; import net.neoforged.neoforge.client.gui.widget.ScrollPanel; import net.neoforged.neoforge.common.I18nExtension; +import net.neoforged.neoforge.network.registration.NetworkRegistry; import org.apache.commons.lang3.tuple.Pair; +import org.slf4j.Logger; public class ModMismatchDisconnectedScreen extends Screen { + private static final Logger LOGGER = LogUtils.getLogger(); private final Component reason; private MultiLineLabel message = MultiLineLabel.EMPTY; + private MismatchInfoPanel scrollList; private final Screen parent; private int textHeight; private final Path modsDir; @@ -53,6 +61,12 @@ public ModMismatchDisconnectedScreen(Screen parentScreen, Component reason, Map< this.modsDir = FMLPaths.MODSDIR.get(); this.logFile = FMLPaths.GAMEDIR.get().resolve(Paths.get("logs", "latest.log")); this.mismatchedChannelData = mismatchedChannelData; + this.mismatchedChannelData.replaceAll((id, r) -> { //Enhance the reason components provided by the server with the info of which mod owns the given channel, if such a mod can be found on the client + String owningModId = NetworkRegistry.getInstance().getOwningModId(id); + Optional modDisplayName = ModList.get().getModContainerById(owningModId).map(mod -> mod.getModInfo().getDisplayName()); + return modDisplayName.isPresent() && !(r.getContents() instanceof TranslatableContents c && c.getKey().equals("neoforge.network.negotiation.failure.mod")) ? Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName.get(), r) : r; + }); + this.mismatchedChannelData.forEach((id, r) -> LOGGER.warn("Channel [{}] failed to connect: {}", id, reason.getString())); } @Override @@ -65,17 +79,19 @@ protected void init() { int upperButtonHeight = Math.min((this.height + this.listHeight) / 2 + 25, this.height - 50); int lowerButtonHeight = Math.min((this.height + this.listHeight) / 2 + 50, this.height - 25); - this.addRenderableWidget(new MismatchInfoPanel(minecraft, listWidth, listHeight, (this.height - this.listHeight) / 2, listLeft)); + this.addRenderableWidget(this.scrollList = new MismatchInfoPanel(minecraft, listWidth, listHeight, (this.height - this.listHeight) / 2, listLeft)); int buttonWidth = Math.min(210, this.width / 2 - 20); + this.addRenderableWidget(CycleButton.onOffBuilder(true) + .create(Math.max(this.width / 4 - buttonWidth / 2, listLeft), upperButtonHeight, buttonWidth, 20, Component.translatable("fml.modmismatchscreen.simplifiedview"), (b, v) -> scrollList.toggleSimplifiedView())); this.addRenderableWidget(Button.builder(Component.literal(I18nExtension.parseMessage("fml.button.open.file", logFile.getFileName())), button -> Util.getPlatform().openFile(logFile.toFile())) - .bounds(Math.max(this.width / 4 - buttonWidth / 2, listLeft), upperButtonHeight, buttonWidth, 20) + .bounds(Math.min(this.width * 3 / 4 - buttonWidth / 2, listLeft + listWidth - buttonWidth), upperButtonHeight, buttonWidth, 20) .build()); this.addRenderableWidget(Button.builder(Component.literal(I18nExtension.parseMessage("fml.button.open.mods.folder")), button -> Util.getPlatform().openFile(modsDir.toFile())) - .bounds(Math.min(this.width * 3 / 4 - buttonWidth / 2, listLeft + listWidth - buttonWidth), upperButtonHeight, buttonWidth, 20) + .bounds(Math.max(this.width / 4 - buttonWidth / 2, listLeft), lowerButtonHeight, buttonWidth, 20) .build()); this.addRenderableWidget(Button.builder(Component.translatable("gui.toMenu"), button -> this.minecraft.setScreen(this.parent)) - .bounds((this.width - buttonWidth) / 2, lowerButtonHeight, buttonWidth, 20) + .bounds(Math.min(this.width * 3 / 4 - buttonWidth / 2, listLeft + listWidth - buttonWidth), lowerButtonHeight, buttonWidth, 20) .build()); } @@ -88,29 +104,33 @@ public void render(GuiGraphics guiGraphics, int mouseX, int mouseY, float partia } class MismatchInfoPanel extends ScrollPanel { - private final List> lineTable; - private final int contentSize; private final int nameIndent = 10; private final int tableWidth = width - border * 2 - 6 - nameIndent; private final int nameWidth = tableWidth / 2; private final int versionWidth = tableWidth - nameWidth; + private List> lineTable; + private int contentSize; + private boolean oneChannelPerEntry = true; public MismatchInfoPanel(Minecraft client, int width, int height, int top, int left) { super(client, width, height, top, left); + updateListContent(); + } - Map> collapsedChannelData = collapseChannelData(mismatchedChannelData); + private void updateListContent() { + Map, Component> mergedChannelData = sortAndMergeChannelData(mismatchedChannelData); record Row(MutableComponent name, MutableComponent reason) {} //The raw list of the strings in a table row, the components may still be too long for the final table and will be split up later. The first row element may have a style assigned to it that will be used for the whole content row. List rows = new ArrayList<>(); - if (!collapsedChannelData.isEmpty()) { - //Each table row contains the channel namespace and the reason for the corresponding channel mismatch. + if (!mergedChannelData.isEmpty()) { + //Each table row contains the channel id(s) and the reason for the corresponding channel mismatch. rows.add(new Row(Component.translatable("fml.modmismatchscreen.table.channelname"), Component.translatable("fml.modmismatchscreen.table.reason"))); int i = 0; - for (Map.Entry> channelData : collapsedChannelData.entrySet()) { - rows.add(new Row(toChannelNameComponent(channelData.getKey(), channelData.getValue().getLeft(), i % 2 == 0 ? ChatFormatting.GOLD : ChatFormatting.YELLOW), channelData.getValue().getRight().copy())); - if (++i == 20 && collapsedChannelData.size() > 20) { - //If too many mismatched mod entries are present, append a line referencing how to see the full list and stop rendering any more entries - rows.add(new Row(Component.literal(""), Component.translatable("fml.modmismatchscreen.additional", collapsedChannelData.size() - i).withStyle(ChatFormatting.ITALIC))); + for (Map.Entry, Component> channelData : mergedChannelData.entrySet()) { + rows.add(new Row(toChannelComponent(channelData.getKey(), i % 2 == 0 ? ChatFormatting.GOLD : ChatFormatting.YELLOW), channelData.getValue().copy())); + if (++i == 30 && mergedChannelData.size() > 30) { + //If too many mismatched channel entries are present, append a line referencing how to see the full list and stop rendering any more entries + rows.add(new Row(Component.literal(""), Component.translatable("fml.modmismatchscreen.additional", mergedChannelData.size() - i).withStyle(ChatFormatting.ITALIC))); break; } } @@ -119,48 +139,43 @@ record Row(MutableComponent name, MutableComponent reason) {} this.lineTable = rows.stream().flatMap(p -> splitLineToWidth(p.name(), p.reason()).stream()).collect(Collectors.toList()); this.contentSize = lineTable.size(); + this.scrollDistance = 0; } /** - * Collapses quasi-duplicate channel mismatch entries into single list entries to reduce repetition of entries in the final list. - * Quasi-duplicate channel mismatch entries share the same channel namespace and mismatch reasons. Even though these mismatches do not need to be causally related, - * there is a not inconsiderable likelihood that they are, and to increase usability of the screen, we summarize them into one entry to allow mod users - * to focus on the likely important parts of the channel mismatch data (which is the namespace of the mismatching channels and the exact mismatch reason). + * Iterates over the raw channel mismatch data and merges entries with the same reason component into one channel mismatch entry each. + * Due to the reason component always containing the display name of the mod that owns the associated channel, this step effectively groups channels by their owning mod, + * so users can see more easily which mods are the culprits of the negotiation failure that caused this screen to appear. * - * @param mismatchedChannelData The raw mismatched channel data received from the server, which might contain quasi-duplicate entries - * @return A map containing deduplicated channel mismatch entries. For each quasi-duplicate group, only the first encountered channel id is kept, - * and all other quasi-duplicate channels then increment the associated repetition count that is mapped to that first channel id. - * The mismatch reason (which remains unchanged and is the same for all quasi-duplicate entries) also gets mapped to the channel id. + * @param mismatchedChannelData The raw mismatched channel data received from the server, which might contain entries with duplicate channel mismatch reasons + * @return A map containing channel mismatch entries with unique reasons. Each channel mismatch entry contains the list of all channels that share the same reason component, + * mapped to that reason component. */ - private Map> collapseChannelData(Map mismatchedChannelData) { - Map> repetitions = new LinkedHashMap<>(); + private Map, Component> sortAndMergeChannelData(Map mismatchedChannelData) { + Map> channelsByReason = new LinkedHashMap<>(); List sortedChannels = mismatchedChannelData.keySet().stream().sorted(Comparator.comparing(ResourceLocation::toString)).toList(); for (ResourceLocation channel : sortedChannels) { Component channelMismatchReason = mismatchedChannelData.get(channel); - List namespaceChannels = repetitions.keySet().stream().filter(r -> r.getNamespace().equals(channel.getNamespace())).toList(); - boolean matched = false; - if (!namespaceChannels.isEmpty()) { - for (ResourceLocation potentialRepetitionChannel : namespaceChannels) { - Pair repetitionData = repetitions.get(potentialRepetitionChannel); - - if (repetitionData.getRight().equals(channelMismatchReason)) { - repetitions.put(potentialRepetitionChannel, Pair.of(repetitionData.getLeft() + 1, repetitionData.getRight())); - matched = true; - } - } - } - if (!matched) - repetitions.put(channel, Pair.of(1, channelMismatchReason)); + if (channelsByReason.containsKey(channelMismatchReason)) + channelsByReason.get(channelMismatchReason).add(channel); + else + channelsByReason.put(channelMismatchReason, Lists.newArrayList(channel)); } - return repetitions; + Map, Component> channelMismatchEntries = new LinkedHashMap<>(); + List sortedChannelEntries = channelsByReason.entrySet().stream().sorted(Comparator.comparing(entry -> entry.getValue().get(0).toString())).map(Map.Entry::getKey).toList(); + for (Component mismatchReason : sortedChannelEntries) { + channelMismatchEntries.put(channelsByReason.get(mismatchReason), mismatchReason); + } + + return channelMismatchEntries; } /** * Splits the raw channel namespace and mismatch reason strings, making them use multiple lines if needed, to fit within the table dimensions. * The style assigned to the name element is then applied to the entire content row. * - * @param name The first element of the content row, usually representing a table section header or a channel namespace entry + * @param name The first element of the content row, usually representing a table section header or a channel name entry * @param reason The second element of the content row, usually representing the reason why the channel is mismatched * @return A list of table rows consisting of 2 elements each which consist of the same content as was given by the parameters, but split up to fit within the table dimensions. */ @@ -178,24 +193,32 @@ private List> splitLineToWidt } /** - * Returns a component with the namespace of the given channel id. The component is colored in the given color, which will be used for the whole content row, - * and has a hover text mentioning an exemplary mismatching channel id plus how many other channels with the same namespace were mismatching with the same reason. + * Formats the given list of channel ids to a component which, depending on the current display mode of the list, will list either the first or all channel ids. + * If only one channel id is shown, the amount of channels that have the same reason component will also be displayed next to the channel id. + * The component is colored in the given color, which will be used for the whole content row. * - * @param id The id of the exemplary mismatched channel. Its namespace is used for the component text. - * @param repetitionCount How many channels with the same namespace failed negotiation with the same error message. Displayed in the hover tooltip. - * @param color Defines the color of the returned channel namespace component. - * @return A component with the channel namespace as the main text component, and an assigned color which will be used for the whole content row. + * @param ids The list of channel ids to be formatted to the component. Depending on the current list mode, either the full list or the first entry will be used for the component text. + * @param color Defines the color of the returned component. + * @return A component with one or all entries of the channel id list as the main text component, and an assigned color which will be used for the whole content row. */ - private MutableComponent toChannelNameComponent(ResourceLocation id, int repetitionCount, ChatFormatting color) { - MutableComponent namespaceComponent = Component.literal(id.getNamespace()).withStyle(color); - MutableComponent hoverText = Component.literal(id.toString()); - if (repetitionCount > 1) - hoverText.append(Component.literal(" [+%s more]".formatted(repetitionCount)).withStyle(ChatFormatting.GRAY)); + private MutableComponent toChannelComponent(List ids, ChatFormatting color) { + MutableComponent namespaceComponent; + if (oneChannelPerEntry) { + namespaceComponent = Component.literal(ids.get(0).toString()).withStyle(color); + + if (ids.size() > 1) + namespaceComponent.append(Component.literal("\n[+%s more]".formatted(ids.size() - 1)).withStyle(ChatFormatting.DARK_GRAY)); + } else + namespaceComponent = ComponentUtils.formatList(ids, ComponentUtils.DEFAULT_SEPARATOR, r -> Component.literal(r.toString())).withStyle(color); - namespaceComponent = namespaceComponent.withStyle(s -> s.withHoverEvent(new HoverEvent(Action.SHOW_TEXT, hoverText))); return namespaceComponent; } + public void toggleSimplifiedView() { + this.oneChannelPerEntry = !this.oneChannelPerEntry; + updateListContent(); + } + @Override protected int getContentHeight() { int height = contentSize * (font.lineHeight + 3); diff --git a/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java b/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java index d86545807e..4c54b3f05e 100644 --- a/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java +++ b/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java @@ -21,6 +21,7 @@ @ApiStatus.Internal public record NegotiableNetworkComponent( ResourceLocation id, + Optional modid, Optional version, Optional flow, boolean optional) {} diff --git a/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java b/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java index ae22a2378f..c4d6aa72e6 100644 --- a/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java +++ b/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java @@ -14,6 +14,7 @@ import java.util.Optional; import net.minecraft.network.chat.Component; import net.minecraft.resources.ResourceLocation; +import net.neoforged.fml.ModList; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.VisibleForTesting; @@ -77,13 +78,21 @@ public static NegotiationResult negotiate(List serve if (!client.isEmpty()) { final Map failureReasons = new HashMap<>(); - client.forEach(c -> failureReasons.put(c.id(), Component.translatable("neoforge.network.negotiation.failure.missing.client.server"))); + client.forEach(c -> { + Component channelFailureReason = Component.translatable("neoforge.network.negotiation.failure.missing.client.server"); + String modDisplayName = c.modid().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); + failureReasons.put(c.id(), modDisplayName.isEmpty() ? channelFailureReason : Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName, channelFailureReason)); + }); return new NegotiationResult(List.of(), false, failureReasons); } if (!server.isEmpty()) { final Map failureReasons = new HashMap<>(); - server.forEach(c -> failureReasons.put(c.id(), Component.translatable("neoforge.network.negotiation.failure.missing.server.client"))); + server.forEach(c -> { + Component channelFailureReason = Component.translatable("neoforge.network.negotiation.failure.missing.server.client"); + String modDisplayName = c.modid().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); + failureReasons.put(c.id(), modDisplayName.isEmpty() ? channelFailureReason : Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName, channelFailureReason)); + }); return new NegotiationResult(List.of(), false, failureReasons); } @@ -92,16 +101,17 @@ public static NegotiationResult negotiate(List serve for (Table.Cell match : matches.cellSet()) { final NegotiableNetworkComponent serverComponent = match.getColumnKey(); final NegotiableNetworkComponent clientComponent = match.getValue(); + final String modDisplayName = serverComponent.modid().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); Optional serverToClientComparison = validateComponent(serverComponent, clientComponent, "client"); if (serverToClientComparison.isPresent() && !serverToClientComparison.get().success()) { - failureReasons.put(serverComponent.id(), serverToClientComparison.get().failureReason()); + failureReasons.put(serverComponent.id(), modDisplayName.isEmpty() ? serverToClientComparison.get().failureReason() : Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName, serverToClientComparison.get().failureReason())); continue; } Optional clientToServerComparison = validateComponent(clientComponent, serverComponent, "server"); if (clientToServerComparison.isPresent() && !clientToServerComparison.get().success()) { - failureReasons.put(serverComponent.id(), clientToServerComparison.get().failureReason()); + failureReasons.put(serverComponent.id(), modDisplayName.isEmpty() ? clientToServerComparison.get().failureReason() : Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName, clientToServerComparison.get().failureReason())); continue; } diff --git a/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java b/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java index 59b829a04b..7a3dd25ece 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java @@ -27,6 +27,7 @@ public record ConfigurationRegistration( FriendlyByteBuf.Reader reader, IConfigurationPayloadHandler handler, + String modId, Optional version, Optional flow, boolean optional) implements IConfigurationPayloadHandler, FriendlyByteBuf.Reader { diff --git a/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java b/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java index 4d08a34374..2704eed15c 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java @@ -13,6 +13,7 @@ import net.minecraft.network.FriendlyByteBuf; import net.minecraft.network.protocol.common.custom.CustomPacketPayload; import net.minecraft.resources.ResourceLocation; +import net.neoforged.fml.ModLoadingContext; import net.neoforged.neoforge.network.handling.IConfigurationPayloadHandler; import net.neoforged.neoforge.network.handling.IPayloadHandler; import net.neoforged.neoforge.network.handling.IPlayPayloadHandler; @@ -56,7 +57,7 @@ public Map> getPlayRegistrations() { public IPayloadRegistrar play(ResourceLocation id, FriendlyByteBuf.Reader reader, IPlayPayloadHandler handler) { play( id, new PlayRegistration<>( - reader, handler, version, Optional.empty(), optional)); + reader, handler, ModLoadingContext.get().getActiveNamespace(), version, Optional.empty(), optional)); return this; } @@ -64,7 +65,7 @@ public IPayloadRegistrar play(ResourceLocation i public IPayloadRegistrar configuration(ResourceLocation id, FriendlyByteBuf.Reader reader, IConfigurationPayloadHandler handler) { configuration( id, new ConfigurationRegistration<>( - reader, handler, version, Optional.empty(), optional)); + reader, handler, ModLoadingContext.get().getActiveNamespace(), version, Optional.empty(), optional)); return this; } @@ -75,7 +76,7 @@ public IPayloadRegistrar play(ResourceLocation i final PlayPayloadHandler innerHandler = builder.create(); play( id, new PlayRegistration<>( - reader, innerHandler, version, innerHandler.flow(), optional)); + reader, innerHandler, ModLoadingContext.get().getActiveNamespace(), version, innerHandler.flow(), optional)); return this; } @@ -86,7 +87,7 @@ public IPayloadRegistrar configuration(ResourceL final ConfigurationPayloadHandler innerHandler = builder.create(); configuration( id, new ConfigurationRegistration<>( - reader, innerHandler, version, innerHandler.flow(), optional)); + reader, innerHandler, ModLoadingContext.get().getActiveNamespace(), version, innerHandler.flow(), optional)); return this; } diff --git a/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java b/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java index 9895c0abda..03d0f77343 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java @@ -119,7 +119,7 @@ public void setup() { setup = true; final Map registrarsByNamespace = Collections.synchronizedMap(new HashMap<>()); - ModLoader.get().postEvent(new RegisterPayloadHandlerEvent(namespace -> registrarsByNamespace.computeIfAbsent(namespace, ModdedPacketRegistrar::new))); + ModLoader.get().postEventWrapContainerInModOrder(new RegisterPayloadHandlerEvent(namespace -> registrarsByNamespace.computeIfAbsent(namespace, ModdedPacketRegistrar::new))); registrarsByNamespace.values().forEach(ModdedPacketRegistrar::invalidate); final ImmutableMap.Builder> configurationBuilder = ImmutableMap.builder(); @@ -464,10 +464,10 @@ public boolean onModdedPacketAtClient(ClientCommonPacketListener listener, Clien public void onModdedConnectionDetectedAtServer(ServerConfigurationPacketListener sender, Set configuration, Set play) { final NegotiationResult configurationNegotiationResult = NetworkComponentNegotiator.negotiate( knownConfigurationRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), Optional.of(entry.getValue().modId()), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList(), configuration.stream() - .map(entry -> new NegotiableNetworkComponent(entry.id(), entry.version(), entry.flow(), entry.optional())) + .map(entry -> new NegotiableNetworkComponent(entry.id(), Optional.empty(), entry.version(), entry.flow(), entry.optional())) .toList()); sender.getConnection().channel().attr(ATTRIBUTE_CONNECTION_TYPE).set(sender.getConnectionType()); @@ -486,10 +486,10 @@ public void onModdedConnectionDetectedAtServer(ServerConfigurationPacketListener final NegotiationResult playNegotiationResult = NetworkComponentNegotiator.negotiate( knownPlayRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), Optional.of(entry.getValue().modId()), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList(), play.stream() - .map(entry -> new NegotiableNetworkComponent(entry.id(), entry.version(), entry.flow(), entry.optional())) + .map(entry -> new NegotiableNetworkComponent(entry.id(), Optional.empty(), entry.version(), entry.flow(), entry.optional())) .toList()); //Negotiation failed. Disconnect the client. @@ -533,7 +533,7 @@ public boolean onVanillaOrOtherConnectionDetectedAtServer(ServerConfigurationPac final NegotiationResult configurationNegotiationResult = NetworkComponentNegotiator.negotiate( knownConfigurationRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), Optional.of(entry.getValue().modId()), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList(), List.of()); @@ -550,7 +550,7 @@ public boolean onVanillaOrOtherConnectionDetectedAtServer(ServerConfigurationPac final NegotiationResult playNegotiationResult = NetworkComponentNegotiator.negotiate( knownPlayRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), Optional.of(entry.getValue().modId()), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList(), List.of()); @@ -795,7 +795,7 @@ public boolean onVanillaNetworkConnectionEstablished(ClientConfigurationPacketLi final NegotiationResult configurationNegotiationResult = NetworkComponentNegotiator.negotiate( List.of(), knownConfigurationRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), Optional.of(entry.getValue().modId()), entry.getValue().flow(), entry.getValue().optional())) .toList()); //Because we are in vanilla land, no matter what we are not able to support any custom channels. @@ -812,7 +812,7 @@ public boolean onVanillaNetworkConnectionEstablished(ClientConfigurationPacketLi final NegotiationResult playNegotiationResult = NetworkComponentNegotiator.negotiate( List.of(), knownPlayRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), Optional.of(entry.getValue().modId()), entry.getValue().flow(), entry.getValue().optional())) .toList()); //Negotiation failed. Disconnect the client. @@ -888,6 +888,24 @@ public boolean isConnected(final Connection connection, ConnectionPhase connecti return getKnownAdHocChannelsOfOtherEnd(connection).contains(payloadId); } + /** + * Returns the id of the mod that registered the given payload. This information is not sent over the network, and can thus + * only be retrieved if the payload was registered on the same logical side as this method is called from. + * + * @param payloadId The payload id to check. + * @return The id of the mod that registered the payload with the given id, or an empty string if the payload wasn't registered + * by a mod on the same logical side as this method is called from. + */ + public String getOwningModId(ResourceLocation payloadId) { + String owningModId = ""; + if (knownConfigurationRegistrations.containsKey(payloadId)) + owningModId = knownConfigurationRegistrations.get(payloadId).modId(); + else if (knownPlayRegistrations.containsKey(payloadId)) + owningModId = knownPlayRegistrations.get(payloadId).modId(); + + return owningModId; + } + /** * Filters the given packets for a bundle packet in the game phase of the connection. * diff --git a/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java b/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java index 21c6350ce6..c09cea5f1b 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java @@ -27,6 +27,7 @@ public record PlayRegistration( FriendlyByteBuf.Reader reader, IPlayPayloadHandler handler, + String modId, Optional version, Optional flow, boolean optional) implements IPlayPayloadHandler, FriendlyByteBuf.Reader { diff --git a/src/main/resources/assets/neoforge/lang/en_us.json b/src/main/resources/assets/neoforge/lang/en_us.json index babe2b3144..effb6b9e95 100644 --- a/src/main/resources/assets/neoforge/lang/en_us.json +++ b/src/main/resources/assets/neoforge/lang/en_us.json @@ -58,6 +58,7 @@ "fml.modmismatchscreen.homepage": "Click to get to the homepage of this mod", "fml.modmismatchscreen.table.reason": "Reason", "fml.modmismatchscreen.table.visit.mod_page": "Open the mod page of the mod that registers the channel: %s", + "fml.modmismatchscreen.simplifiedview": "Simplified view", "fml.language.missingversion": "Mod File {5} needs language provider {3}:{4,vr} to load\n\u00a77We have found {6,i18n,fml.messages.artifactversion}", "fml.modloading.missingclasses": "The Mod File {3} has mods that were not found", "fml.modloading.missingmetadata": "mods.toml missing metadata for modid {3}", @@ -239,6 +240,7 @@ "pack.neoforge.description": "NeoForge data/resource pack", + "neoforge.network.negotiation.failure.mod": "Channel of mod \"%1$s\" failed to connect: %2$s", "neoforge.network.negotiation.failure.missing.client.server": "This channel is missing on the server side, but required on the client!", "neoforge.network.negotiation.failure.missing.server.client": "This channel is missing on the client side, but required on the server!", "neoforge.network.negotiation.failure.flow.client.missing": "The client wants the payload to flow: %s, but the server does not support it!", From d72665cdedafc2f885151836891fc3551daa0cf3 Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Wed, 6 Mar 2024 16:45:55 +0100 Subject: [PATCH 5/9] Fixed wrong reason message being logged --- .../neoforge/client/gui/ModMismatchDisconnectedScreen.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java index 5a1047422c..3817d183a8 100644 --- a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java +++ b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java @@ -66,7 +66,7 @@ public ModMismatchDisconnectedScreen(Screen parentScreen, Component reason, Map< Optional modDisplayName = ModList.get().getModContainerById(owningModId).map(mod -> mod.getModInfo().getDisplayName()); return modDisplayName.isPresent() && !(r.getContents() instanceof TranslatableContents c && c.getKey().equals("neoforge.network.negotiation.failure.mod")) ? Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName.get(), r) : r; }); - this.mismatchedChannelData.forEach((id, r) -> LOGGER.warn("Channel [{}] failed to connect: {}", id, reason.getString())); + this.mismatchedChannelData.forEach((id, r) -> LOGGER.warn("Channel [{}] failed to connect: {}", id, r.getString())); } @Override From bbcd82ed4cc1f82be33588f2f81216e0431a92e7 Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Wed, 6 Mar 2024 20:34:28 +0100 Subject: [PATCH 6/9] Added missing param javadoc and renamed record field --- .../network/negotiation/NegotiableNetworkComponent.java | 3 ++- .../network/negotiation/NetworkComponentNegotiator.java | 6 +++--- .../network/registration/ConfigurationRegistration.java | 1 + .../neoforge/network/registration/PlayRegistration.java | 1 + 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java b/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java index 4c54b3f05e..7ca5296781 100644 --- a/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java +++ b/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java @@ -14,6 +14,7 @@ * Represents the input to the negotiation process for a single network payload type. * * @param id The id of the payload type. + * @param modId The id of the mod that registered the payload. * @param version The version of the payload type. * @param flow The flow of the payload type. * @param optional Whether the payload type is optional. @@ -21,7 +22,7 @@ @ApiStatus.Internal public record NegotiableNetworkComponent( ResourceLocation id, - Optional modid, + Optional modId, Optional version, Optional flow, boolean optional) {} diff --git a/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java b/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java index c4d6aa72e6..9bd9649cd8 100644 --- a/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java +++ b/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java @@ -80,7 +80,7 @@ public static NegotiationResult negotiate(List serve final Map failureReasons = new HashMap<>(); client.forEach(c -> { Component channelFailureReason = Component.translatable("neoforge.network.negotiation.failure.missing.client.server"); - String modDisplayName = c.modid().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); + String modDisplayName = c.modId().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); failureReasons.put(c.id(), modDisplayName.isEmpty() ? channelFailureReason : Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName, channelFailureReason)); }); return new NegotiationResult(List.of(), false, failureReasons); @@ -90,7 +90,7 @@ public static NegotiationResult negotiate(List serve final Map failureReasons = new HashMap<>(); server.forEach(c -> { Component channelFailureReason = Component.translatable("neoforge.network.negotiation.failure.missing.server.client"); - String modDisplayName = c.modid().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); + String modDisplayName = c.modId().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); failureReasons.put(c.id(), modDisplayName.isEmpty() ? channelFailureReason : Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName, channelFailureReason)); }); return new NegotiationResult(List.of(), false, failureReasons); @@ -101,7 +101,7 @@ public static NegotiationResult negotiate(List serve for (Table.Cell match : matches.cellSet()) { final NegotiableNetworkComponent serverComponent = match.getColumnKey(); final NegotiableNetworkComponent clientComponent = match.getValue(); - final String modDisplayName = serverComponent.modid().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); + final String modDisplayName = serverComponent.modId().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); Optional serverToClientComparison = validateComponent(serverComponent, clientComponent, "client"); if (serverToClientComparison.isPresent() && !serverToClientComparison.get().success()) { diff --git a/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java b/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java index 7a3dd25ece..b2f26b3580 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java @@ -18,6 +18,7 @@ * * @param reader The reader for the payload * @param handler The handler for the payload + * @param modId The id of the mod that registered the payload * @param version The version of the payload * @param flow The flow of the payload * @param optional Whether the payload is optional diff --git a/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java b/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java index c09cea5f1b..f1bb910049 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java @@ -19,6 +19,7 @@ * @param reader The reader for the payload * @param handler The handler for the payload * @param version The version of the payload + * @param modId The id of the mod that registered the payload * @param flow The flow of the payload * @param optional Whether the payload is optional * @param The type of the payload From e5ced9971f0ef6b954c26dae03e99c46efedb2df Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Fri, 29 Mar 2024 11:27:07 +0100 Subject: [PATCH 7/9] Removed usage of static ModLoadingContext, used supplied modId instead For this solution to work, the verification that a registered payload's namespace needs to match the mod id of the registrar had to be removed, in order to allow mods passing their proper mod id to the registrar while still being able to register payloads for any namespace they desire using that registrar (as has been one of the ideas of the Network Refactor). Suggestion: put a duplication check in NetworkRegistry#setup where the channels are built, since currently duplicate payload ids just seem to be silently overwriting each other, which doesn't feel intentional. --- .../network/registration/ModdedPacketRegistrar.java | 13 ++++--------- .../registration/RegistrationFailedException.java | 4 ---- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java b/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java index 2704eed15c..df0c39908b 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java @@ -13,7 +13,6 @@ import net.minecraft.network.FriendlyByteBuf; import net.minecraft.network.protocol.common.custom.CustomPacketPayload; import net.minecraft.resources.ResourceLocation; -import net.neoforged.fml.ModLoadingContext; import net.neoforged.neoforge.network.handling.IConfigurationPayloadHandler; import net.neoforged.neoforge.network.handling.IPayloadHandler; import net.neoforged.neoforge.network.handling.IPlayPayloadHandler; @@ -57,7 +56,7 @@ public Map> getPlayRegistrations() { public IPayloadRegistrar play(ResourceLocation id, FriendlyByteBuf.Reader reader, IPlayPayloadHandler handler) { play( id, new PlayRegistration<>( - reader, handler, ModLoadingContext.get().getActiveNamespace(), version, Optional.empty(), optional)); + reader, handler, modId, version, Optional.empty(), optional)); return this; } @@ -65,7 +64,7 @@ public IPayloadRegistrar play(ResourceLocation i public IPayloadRegistrar configuration(ResourceLocation id, FriendlyByteBuf.Reader reader, IConfigurationPayloadHandler handler) { configuration( id, new ConfigurationRegistration<>( - reader, handler, ModLoadingContext.get().getActiveNamespace(), version, Optional.empty(), optional)); + reader, handler, modId, version, Optional.empty(), optional)); return this; } @@ -76,7 +75,7 @@ public IPayloadRegistrar play(ResourceLocation i final PlayPayloadHandler innerHandler = builder.create(); play( id, new PlayRegistration<>( - reader, innerHandler, ModLoadingContext.get().getActiveNamespace(), version, innerHandler.flow(), optional)); + reader, innerHandler, modId, version, innerHandler.flow(), optional)); return this; } @@ -87,7 +86,7 @@ public IPayloadRegistrar configuration(ResourceL final ConfigurationPayloadHandler innerHandler = builder.create(); configuration( id, new ConfigurationRegistration<>( - reader, innerHandler, ModLoadingContext.get().getActiveNamespace(), version, innerHandler.flow(), optional)); + reader, innerHandler, modId, version, innerHandler.flow(), optional)); return this; } @@ -120,10 +119,6 @@ private void validatePayload(ResourceLocation id, final Map if (payloads.containsKey(id)) { throw new RegistrationFailedException(id, modId, RegistrationFailedException.Reason.DUPLICATE_ID); } - - if (!id.getNamespace().equals(modId)) { - throw new RegistrationFailedException(id, modId, RegistrationFailedException.Reason.INVALID_NAMESPACE); - } } @Override diff --git a/src/main/java/net/neoforged/neoforge/network/registration/RegistrationFailedException.java b/src/main/java/net/neoforged/neoforge/network/registration/RegistrationFailedException.java index cabf658a1d..d059c43a8d 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/RegistrationFailedException.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/RegistrationFailedException.java @@ -98,10 +98,6 @@ public enum Reason implements ReasonFormatter { * The payload id is already registered. */ DUPLICATE_ID((id, namespace) -> "Duplicate payload id " + id + " for payload in namespace " + namespace + "."), - /** - * The payload id is registered in the wrong namespace. - */ - INVALID_NAMESPACE((id, namespace) -> "Try registering payload in namespace " + namespace + " for payload with id " + id + "."), /** * The registrar is invalid. */ From c8fc63602c57ebe41fb0be83a2dd16859bf5605e Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Sat, 30 Mar 2024 21:58:21 +0100 Subject: [PATCH 8/9] Re-added prior logic of grouping mismatching channels by namespace - The name of the mod which id matches the mismatching channel namespace is displayed in the channel mismatch entry's reason component, so users have a potential mod candidate to add/update/remove - The feature of tracking the mod that registered a channel has been removed again - Re-added the exception that occurs when a channel gets registered to a registrar with a mod id which is different to the channel's namespace --- .../gui/ModMismatchDisconnectedScreen.java | 10 +++--- .../NegotiableNetworkComponent.java | 2 -- .../NetworkComponentNegotiator.java | 6 ++-- .../ConfigurationRegistration.java | 5 +-- .../registration/ModdedPacketRegistrar.java | 12 ++++--- .../network/registration/NetworkRegistry.java | 34 +++++-------------- .../registration/PlayRegistration.java | 5 +-- .../RegistrationFailedException.java | 4 +++ 8 files changed, 33 insertions(+), 45 deletions(-) diff --git a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java index 3817d183a8..202da0cf4e 100644 --- a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java +++ b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java @@ -38,7 +38,6 @@ import net.neoforged.fml.loading.FMLPaths; import net.neoforged.neoforge.client.gui.widget.ScrollPanel; import net.neoforged.neoforge.common.I18nExtension; -import net.neoforged.neoforge.network.registration.NetworkRegistry; import org.apache.commons.lang3.tuple.Pair; import org.slf4j.Logger; @@ -61,9 +60,8 @@ public ModMismatchDisconnectedScreen(Screen parentScreen, Component reason, Map< this.modsDir = FMLPaths.MODSDIR.get(); this.logFile = FMLPaths.GAMEDIR.get().resolve(Paths.get("logs", "latest.log")); this.mismatchedChannelData = mismatchedChannelData; - this.mismatchedChannelData.replaceAll((id, r) -> { //Enhance the reason components provided by the server with the info of which mod owns the given channel, if such a mod can be found on the client - String owningModId = NetworkRegistry.getInstance().getOwningModId(id); - Optional modDisplayName = ModList.get().getModContainerById(owningModId).map(mod -> mod.getModInfo().getDisplayName()); + this.mismatchedChannelData.replaceAll((id, r) -> { //Enhance the reason components provided by the server with the info of which mod likely owns the given channel (based on the channel's namespace), if such a mod can be found on the client + Optional modDisplayName = ModList.get().getModContainerById(id.getNamespace()).map(mod -> mod.getModInfo().getDisplayName()); return modDisplayName.isPresent() && !(r.getContents() instanceof TranslatableContents c && c.getKey().equals("neoforge.network.negotiation.failure.mod")) ? Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName.get(), r) : r; }); this.mismatchedChannelData.forEach((id, r) -> LOGGER.warn("Channel [{}] failed to connect: {}", id, r.getString())); @@ -144,8 +142,8 @@ record Row(MutableComponent name, MutableComponent reason) {} /** * Iterates over the raw channel mismatch data and merges entries with the same reason component into one channel mismatch entry each. - * Due to the reason component always containing the display name of the mod that owns the associated channel, this step effectively groups channels by their owning mod, - * so users can see more easily which mods are the culprits of the negotiation failure that caused this screen to appear. + * Due to the reason component always containing the display name of the mod that likely owns the associated channel, this step effectively groups channels by their most likely owning mod candidate, + * so users can see more easily which mods might be the culprits of the negotiation failure that caused this screen to appear. * * @param mismatchedChannelData The raw mismatched channel data received from the server, which might contain entries with duplicate channel mismatch reasons * @return A map containing channel mismatch entries with unique reasons. Each channel mismatch entry contains the list of all channels that share the same reason component, diff --git a/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java b/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java index 7ca5296781..d86545807e 100644 --- a/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java +++ b/src/main/java/net/neoforged/neoforge/network/negotiation/NegotiableNetworkComponent.java @@ -14,7 +14,6 @@ * Represents the input to the negotiation process for a single network payload type. * * @param id The id of the payload type. - * @param modId The id of the mod that registered the payload. * @param version The version of the payload type. * @param flow The flow of the payload type. * @param optional Whether the payload type is optional. @@ -22,7 +21,6 @@ @ApiStatus.Internal public record NegotiableNetworkComponent( ResourceLocation id, - Optional modId, Optional version, Optional flow, boolean optional) {} diff --git a/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java b/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java index 9bd9649cd8..15abfda186 100644 --- a/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java +++ b/src/main/java/net/neoforged/neoforge/network/negotiation/NetworkComponentNegotiator.java @@ -80,7 +80,7 @@ public static NegotiationResult negotiate(List serve final Map failureReasons = new HashMap<>(); client.forEach(c -> { Component channelFailureReason = Component.translatable("neoforge.network.negotiation.failure.missing.client.server"); - String modDisplayName = c.modId().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); + String modDisplayName = ModList.get().getModContainerById(c.id().getNamespace()).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); failureReasons.put(c.id(), modDisplayName.isEmpty() ? channelFailureReason : Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName, channelFailureReason)); }); return new NegotiationResult(List.of(), false, failureReasons); @@ -90,7 +90,7 @@ public static NegotiationResult negotiate(List serve final Map failureReasons = new HashMap<>(); server.forEach(c -> { Component channelFailureReason = Component.translatable("neoforge.network.negotiation.failure.missing.server.client"); - String modDisplayName = c.modId().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); + String modDisplayName = ModList.get().getModContainerById(c.id().getNamespace()).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); failureReasons.put(c.id(), modDisplayName.isEmpty() ? channelFailureReason : Component.translatable("neoforge.network.negotiation.failure.mod", modDisplayName, channelFailureReason)); }); return new NegotiationResult(List.of(), false, failureReasons); @@ -101,7 +101,7 @@ public static NegotiationResult negotiate(List serve for (Table.Cell match : matches.cellSet()) { final NegotiableNetworkComponent serverComponent = match.getColumnKey(); final NegotiableNetworkComponent clientComponent = match.getValue(); - final String modDisplayName = serverComponent.modId().flatMap(id -> ModList.get().getModContainerById(id)).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); + final String modDisplayName = ModList.get().getModContainerById(serverComponent.id().getNamespace()).map(mc -> mc.getModInfo().getDisplayName()).orElse(""); Optional serverToClientComparison = validateComponent(serverComponent, clientComponent, "client"); if (serverToClientComparison.isPresent() && !serverToClientComparison.get().success()) { diff --git a/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java b/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java index b2f26b3580..0a25aae38f 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/ConfigurationRegistration.java @@ -9,6 +9,7 @@ import net.minecraft.network.FriendlyByteBuf; import net.minecraft.network.protocol.PacketFlow; import net.minecraft.network.protocol.common.custom.CustomPacketPayload; +import net.minecraft.resources.ResourceLocation; import net.neoforged.neoforge.network.handling.ConfigurationPayloadContext; import net.neoforged.neoforge.network.handling.IConfigurationPayloadHandler; import org.jetbrains.annotations.ApiStatus; @@ -18,7 +19,7 @@ * * @param reader The reader for the payload * @param handler The handler for the payload - * @param modId The id of the mod that registered the payload + * @param id The identifier of the payload * @param version The version of the payload * @param flow The flow of the payload * @param optional Whether the payload is optional @@ -28,7 +29,7 @@ public record ConfigurationRegistration( FriendlyByteBuf.Reader reader, IConfigurationPayloadHandler handler, - String modId, + ResourceLocation id, Optional version, Optional flow, boolean optional) implements IConfigurationPayloadHandler, FriendlyByteBuf.Reader { diff --git a/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java b/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java index df0c39908b..849cd75422 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/ModdedPacketRegistrar.java @@ -56,7 +56,7 @@ public Map> getPlayRegistrations() { public IPayloadRegistrar play(ResourceLocation id, FriendlyByteBuf.Reader reader, IPlayPayloadHandler handler) { play( id, new PlayRegistration<>( - reader, handler, modId, version, Optional.empty(), optional)); + reader, handler, id, version, Optional.empty(), optional)); return this; } @@ -64,7 +64,7 @@ public IPayloadRegistrar play(ResourceLocation i public IPayloadRegistrar configuration(ResourceLocation id, FriendlyByteBuf.Reader reader, IConfigurationPayloadHandler handler) { configuration( id, new ConfigurationRegistration<>( - reader, handler, modId, version, Optional.empty(), optional)); + reader, handler, id, version, Optional.empty(), optional)); return this; } @@ -75,7 +75,7 @@ public IPayloadRegistrar play(ResourceLocation i final PlayPayloadHandler innerHandler = builder.create(); play( id, new PlayRegistration<>( - reader, innerHandler, modId, version, innerHandler.flow(), optional)); + reader, innerHandler, id, version, innerHandler.flow(), optional)); return this; } @@ -86,7 +86,7 @@ public IPayloadRegistrar configuration(ResourceL final ConfigurationPayloadHandler innerHandler = builder.create(); configuration( id, new ConfigurationRegistration<>( - reader, innerHandler, modId, version, innerHandler.flow(), optional)); + reader, innerHandler, id, version, innerHandler.flow(), optional)); return this; } @@ -119,6 +119,10 @@ private void validatePayload(ResourceLocation id, final Map if (payloads.containsKey(id)) { throw new RegistrationFailedException(id, modId, RegistrationFailedException.Reason.DUPLICATE_ID); } + + if (!id.getNamespace().equals(modId)) { + throw new RegistrationFailedException(id, modId, RegistrationFailedException.Reason.INVALID_NAMESPACE); + } } @Override diff --git a/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java b/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java index 03d0f77343..4112269262 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/NetworkRegistry.java @@ -464,10 +464,10 @@ public boolean onModdedPacketAtClient(ClientCommonPacketListener listener, Clien public void onModdedConnectionDetectedAtServer(ServerConfigurationPacketListener sender, Set configuration, Set play) { final NegotiationResult configurationNegotiationResult = NetworkComponentNegotiator.negotiate( knownConfigurationRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), Optional.of(entry.getValue().modId()), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList(), configuration.stream() - .map(entry -> new NegotiableNetworkComponent(entry.id(), Optional.empty(), entry.version(), entry.flow(), entry.optional())) + .map(entry -> new NegotiableNetworkComponent(entry.id(), entry.version(), entry.flow(), entry.optional())) .toList()); sender.getConnection().channel().attr(ATTRIBUTE_CONNECTION_TYPE).set(sender.getConnectionType()); @@ -486,10 +486,10 @@ public void onModdedConnectionDetectedAtServer(ServerConfigurationPacketListener final NegotiationResult playNegotiationResult = NetworkComponentNegotiator.negotiate( knownPlayRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), Optional.of(entry.getValue().modId()), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList(), play.stream() - .map(entry -> new NegotiableNetworkComponent(entry.id(), Optional.empty(), entry.version(), entry.flow(), entry.optional())) + .map(entry -> new NegotiableNetworkComponent(entry.id(), entry.version(), entry.flow(), entry.optional())) .toList()); //Negotiation failed. Disconnect the client. @@ -533,7 +533,7 @@ public boolean onVanillaOrOtherConnectionDetectedAtServer(ServerConfigurationPac final NegotiationResult configurationNegotiationResult = NetworkComponentNegotiator.negotiate( knownConfigurationRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), Optional.of(entry.getValue().modId()), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList(), List.of()); @@ -550,7 +550,7 @@ public boolean onVanillaOrOtherConnectionDetectedAtServer(ServerConfigurationPac final NegotiationResult playNegotiationResult = NetworkComponentNegotiator.negotiate( knownPlayRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), Optional.of(entry.getValue().modId()), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList(), List.of()); @@ -795,7 +795,7 @@ public boolean onVanillaNetworkConnectionEstablished(ClientConfigurationPacketLi final NegotiationResult configurationNegotiationResult = NetworkComponentNegotiator.negotiate( List.of(), knownConfigurationRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), Optional.of(entry.getValue().modId()), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList()); //Because we are in vanilla land, no matter what we are not able to support any custom channels. @@ -812,7 +812,7 @@ public boolean onVanillaNetworkConnectionEstablished(ClientConfigurationPacketLi final NegotiationResult playNegotiationResult = NetworkComponentNegotiator.negotiate( List.of(), knownPlayRegistrations.entrySet().stream() - .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), Optional.of(entry.getValue().modId()), entry.getValue().flow(), entry.getValue().optional())) + .map(entry -> new NegotiableNetworkComponent(entry.getKey(), entry.getValue().version(), entry.getValue().flow(), entry.getValue().optional())) .toList()); //Negotiation failed. Disconnect the client. @@ -888,24 +888,6 @@ public boolean isConnected(final Connection connection, ConnectionPhase connecti return getKnownAdHocChannelsOfOtherEnd(connection).contains(payloadId); } - /** - * Returns the id of the mod that registered the given payload. This information is not sent over the network, and can thus - * only be retrieved if the payload was registered on the same logical side as this method is called from. - * - * @param payloadId The payload id to check. - * @return The id of the mod that registered the payload with the given id, or an empty string if the payload wasn't registered - * by a mod on the same logical side as this method is called from. - */ - public String getOwningModId(ResourceLocation payloadId) { - String owningModId = ""; - if (knownConfigurationRegistrations.containsKey(payloadId)) - owningModId = knownConfigurationRegistrations.get(payloadId).modId(); - else if (knownPlayRegistrations.containsKey(payloadId)) - owningModId = knownPlayRegistrations.get(payloadId).modId(); - - return owningModId; - } - /** * Filters the given packets for a bundle packet in the game phase of the connection. * diff --git a/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java b/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java index f1bb910049..aaf359daf3 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/PlayRegistration.java @@ -9,6 +9,7 @@ import net.minecraft.network.FriendlyByteBuf; import net.minecraft.network.protocol.PacketFlow; import net.minecraft.network.protocol.common.custom.CustomPacketPayload; +import net.minecraft.resources.ResourceLocation; import net.neoforged.neoforge.network.handling.IPlayPayloadHandler; import net.neoforged.neoforge.network.handling.PlayPayloadContext; import org.jetbrains.annotations.ApiStatus; @@ -19,7 +20,7 @@ * @param reader The reader for the payload * @param handler The handler for the payload * @param version The version of the payload - * @param modId The id of the mod that registered the payload + * @param id The identifier of the payload * @param flow The flow of the payload * @param optional Whether the payload is optional * @param The type of the payload @@ -28,7 +29,7 @@ public record PlayRegistration( FriendlyByteBuf.Reader reader, IPlayPayloadHandler handler, - String modId, + ResourceLocation id, Optional version, Optional flow, boolean optional) implements IPlayPayloadHandler, FriendlyByteBuf.Reader { diff --git a/src/main/java/net/neoforged/neoforge/network/registration/RegistrationFailedException.java b/src/main/java/net/neoforged/neoforge/network/registration/RegistrationFailedException.java index d059c43a8d..cabf658a1d 100644 --- a/src/main/java/net/neoforged/neoforge/network/registration/RegistrationFailedException.java +++ b/src/main/java/net/neoforged/neoforge/network/registration/RegistrationFailedException.java @@ -98,6 +98,10 @@ public enum Reason implements ReasonFormatter { * The payload id is already registered. */ DUPLICATE_ID((id, namespace) -> "Duplicate payload id " + id + " for payload in namespace " + namespace + "."), + /** + * The payload id is registered in the wrong namespace. + */ + INVALID_NAMESPACE((id, namespace) -> "Try registering payload in namespace " + namespace + " for payload with id " + id + "."), /** * The registrar is invalid. */ From e2f92fca54795765c4af9562c72bb5af9d8a3be5 Mon Sep 17 00:00:00 2001 From: Redstone_Dubstep Date: Wed, 24 Apr 2024 17:24:22 +0200 Subject: [PATCH 9/9] Fixed merge renmant and incorrect blur applied to MMDS --- .../ClientConfigurationPacketListenerImpl.java.patch | 1 - .../neoforge/client/gui/ModMismatchDisconnectedScreen.java | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch b/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch index 3b30a251bb..5d385afbc4 100644 --- a/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch +++ b/patches/net/minecraft/client/multiplayer/ClientConfigurationPacketListenerImpl.java.patch @@ -12,7 +12,6 @@ super(p_295262_, p_296339_, p_294706_); @@ -82,6 +_,11 @@ @Override -+ return; public void handleEnabledFeatures(ClientboundUpdateEnabledFeaturesPacket p_294410_) { this.enabledFeatures = FeatureFlags.REGISTRY.fromNames(p_294410_.features()); + //Fallback detection layer for vanilla servers diff --git a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java index 202da0cf4e..a9ffeb5fb1 100644 --- a/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java +++ b/src/main/java/net/neoforged/neoforge/client/gui/ModMismatchDisconnectedScreen.java @@ -95,10 +95,9 @@ protected void init() { @Override public void render(GuiGraphics guiGraphics, int mouseX, int mouseY, float partialTicks) { - this.renderBackground(guiGraphics, mouseX, mouseY, partialTicks); + super.render(guiGraphics, mouseX, mouseY, partialTicks); guiGraphics.drawCenteredString(this.font, this.title, this.width / 2, (this.height - this.listHeight) / 2 - this.textHeight - 9 * 4, 0xAAAAAA); this.message.renderCentered(guiGraphics, this.width / 2, (this.height - this.listHeight) / 2 - this.textHeight - 9 * 2); - super.render(guiGraphics, mouseX, mouseY, partialTicks); } class MismatchInfoPanel extends ScrollPanel {