From 817e20821d04225e2b39e2ce64be9b70c8ddfa27 Mon Sep 17 00:00:00 2001 From: LexManos Date: Mon, 1 Jan 2024 15:48:44 -0800 Subject: [PATCH] Only add sorted/deduplicated mods to the classpath. Fixes some mods causing the Forge error displays to break. Closes #9833 --- .../minecraftforge/fml/loading/ModSorter.java | 3 ++ .../loading/moddiscovery/ModValidator.java | 45 ++++++++++++++----- settings.gradle | 2 +- .../client/ForgeHooksClient.java | 2 +- .../client/loading/ClientModLoader.java | 26 +---------- .../minecraftforge/common/ForgeConfig.java | 10 +++++ 6 files changed, 50 insertions(+), 38 deletions(-) diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/ModSorter.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/ModSorter.java index cc41dd01b1..4f944f7b66 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/ModSorter.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/ModSorter.java @@ -54,6 +54,9 @@ public class ModSorter ms.buildUniqueList(); } catch (EarlyLoadingException e) { // We cannot build any list with duped mods. We have to abort immediately and report it + // Note this will never actually throw an error because the duplicate checks are done in ModDiscovererer before we get to this phase + // So all this is really doing is wasting time. + // But i'm leaving it here until I rewrite all of cpw's mod loading code because its such a clusterfuck. return LoadingModList.of(ms.systemMods, ms.systemMods.stream().map(mf->(ModInfo)mf.getModInfos().get(0)).collect(toList()), e); } // try and validate dependencies diff --git a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModValidator.java b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModValidator.java index 2483ac4397..ac1765f84b 100644 --- a/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModValidator.java +++ b/fmlloader/src/main/java/net/minecraftforge/fml/loading/moddiscovery/ModValidator.java @@ -6,9 +6,15 @@ package net.minecraftforge.fml.loading.moddiscovery; import com.mojang.logging.LogUtils; + +import cpw.mods.jarhandling.SecureJar; import cpw.mods.modlauncher.api.IModuleLayerManager; import cpw.mods.modlauncher.api.ITransformationService; -import net.minecraftforge.fml.loading.*; +import net.minecraftforge.fml.loading.EarlyLoadingException; +import net.minecraftforge.fml.loading.ImmediateWindowHandler; +import net.minecraftforge.fml.loading.LoadingModList; +import net.minecraftforge.fml.loading.LogMarkers; +import net.minecraftforge.fml.loading.ModSorter; import net.minecraftforge.forgespi.language.IModFileInfo; import net.minecraftforge.forgespi.locating.IModFile; import org.jetbrains.annotations.NotNull; @@ -18,25 +24,24 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; public class ModValidator { private static final Logger LOGGER = LogUtils.getLogger(); - //private final Map> modFiles; private final List candidatePlugins; private final List candidateMods; + private final List gameLibraries; private LoadingModList loadingModList; - private List brokenFiles; + private final List brokenFiles = new ArrayList<>(); private final List discoveryErrorData; - public ModValidator(final Map> modFiles, final List brokenFiles, final List discoveryErrorData) { - //this.modFiles = modFiles; + public ModValidator(Map> modFiles, List brokenFiles, List discoveryErrorData) { this.candidateMods = lst(modFiles.get(IModFile.Type.MOD)); - this.candidateMods.addAll(lst(modFiles.get(IModFile.Type.GAMELIBRARY))); + this.gameLibraries = lst(modFiles.get(IModFile.Type.GAMELIBRARY)); + this.candidateMods.addAll(this.gameLibraries); this.candidatePlugins = lst(modFiles.get(IModFile.Type.LANGPROVIDER)); this.candidatePlugins.addAll(lst(modFiles.get(IModFile.Type.LIBRARY))); this.discoveryErrorData = discoveryErrorData; - this.brokenFiles = brokenFiles.stream().map(IModFileInfo::getFile).collect(Collectors.toList()); // mutable list + brokenFiles.stream().map(IModFileInfo::getFile).forEach(this.brokenFiles::add); } private static List lst(List files) { @@ -54,8 +59,7 @@ public class ModValidator { @NotNull private List validateFiles(final List mods) { final List brokenFiles = new ArrayList<>(); - for (Iterator iterator = mods.iterator(); iterator.hasNext(); ) - { + for (Iterator iterator = mods.iterator(); iterator.hasNext();) { ModFile modFile = iterator.next(); if (!modFile.getProvider().isValid(modFile) || !modFile.identifyMods()) { LOGGER.warn(LogMarkers.SCAN, "File {} has been ignored - it is invalid", modFile.getFilePath()); @@ -71,7 +75,26 @@ public class ModValidator { } public ITransformationService.Resource getModResources() { - return new ITransformationService.Resource(IModuleLayerManager.Layer.GAME, this.candidateMods.stream().map(IModFile::getSecureJar).toList()); + var mods = new ArrayList(); + // Add only the valid mods that we will be attempting to load. + // If any detectable error happens during the sorting process {missing deps, duplicates, + // This helps prevent coremods/mixins from screwing up us displaying the error screens that the sorting/validation is trying to display. + // This won't fix them all as they are still loaded and will still apply but it might help + for (var info : this.loadingModList.getModFiles()) + mods.add(info.getFile().getSecureJar()); + + // Add any game libraries, this *may* be duplicates, depending on the state of the sorting. But until I get around to re-writing + // that clusterfuck of a system, have this simple de-duplication. Order of resource is important, but i'm unsure if duplication is + // so might as well waste a few cycles on checking. Still faster then converting from a set to a list. + // Ideally we would explicitly list out which mods to load {forge and MC being the only ones} but some coremods could require extra things. + // So try game libraries as well + for (var lib : this.gameLibraries) { + var jar = lib.getSecureJar(); + if (!mods.contains(jar)) + mods.add(jar); + } + + return new ITransformationService.Resource(IModuleLayerManager.Layer.GAME, mods); } private List validateLanguages() { diff --git a/settings.gradle b/settings.gradle index a653dbc98f..ed0f0b1607 100644 --- a/settings.gradle +++ b/settings.gradle @@ -72,7 +72,7 @@ dependencyResolutionManagement { bundle('night-config', ['night-config-toml', 'night-config-core']) // Jar in Jar FileSystem - version('jarjar', '0.3.19') + version('jarjar', '0.3.24') library('jarjar-fs', 'net.minecraftforge', 'JarJarFileSystems').versionRef('jarjar') library('jarjar-meta', 'net.minecraftforge', 'JarJarMetadata' ).versionRef('jarjar') library('jarjar-selector', 'net.minecraftforge', 'JarJarSelector' ).versionRef('jarjar') diff --git a/src/main/java/net/minecraftforge/client/ForgeHooksClient.java b/src/main/java/net/minecraftforge/client/ForgeHooksClient.java index e440f42612..aa032e20db 100644 --- a/src/main/java/net/minecraftforge/client/ForgeHooksClient.java +++ b/src/main/java/net/minecraftforge/client/ForgeHooksClient.java @@ -469,7 +469,7 @@ public class ForgeHooksClient { // TODO Do we need this? public static void fillNormal(int[] faceData, Direction facing, boolean calculateNormals) { Vector3f v2; - if (calculateNormals || ForgeConfig.CLIENT.calculateAllNormals.get()) { + if (calculateNormals || ForgeConfig.CLIENT.calculateAllNormals()) { Vector3f v1 = getVertexPos(faceData, 3); Vector3f t1 = getVertexPos(faceData, 1); v2 = getVertexPos(faceData, 2); diff --git a/src/main/java/net/minecraftforge/client/loading/ClientModLoader.java b/src/main/java/net/minecraftforge/client/loading/ClientModLoader.java index c02abc7701..a4e2dbebc1 100644 --- a/src/main/java/net/minecraftforge/client/loading/ClientModLoader.java +++ b/src/main/java/net/minecraftforge/client/loading/ClientModLoader.java @@ -5,24 +5,12 @@ package net.minecraftforge.client.loading; - import java.io.File; -import java.util.ArrayList; import java.util.Collections; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Optional; import java.util.concurrent.*; -import java.util.function.Consumer; - -import net.minecraft.network.chat.Component; -import net.minecraft.server.packs.repository.RepositorySource; -import net.minecraft.server.packs.repository.Pack.Info; import net.minecraft.server.packs.resources.PreparableReloadListener; -import net.minecraft.server.packs.repository.PackSource; import net.minecraft.server.packs.resources.ReloadableResourceManager; import net.minecraft.server.packs.resources.ResourceManager; -import net.minecraft.server.packs.repository.Pack; import net.minecraft.server.packs.repository.PackRepository; import net.minecraft.world.level.DataPackConfig; import net.minecraftforge.event.AddPackFindersEvent; @@ -31,26 +19,19 @@ import net.minecraftforge.fml.loading.ImmediateWindowHandler; import net.minecraftforge.internal.BrandingControl; import net.minecraftforge.logging.CrashReportExtender; import net.minecraftforge.common.util.LogicalSidedProvider; -import net.minecraftforge.forgespi.locating.IModFile; -import net.minecraftforge.resource.PathPackResources; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import net.minecraft.SharedConstants; import net.minecraft.client.Minecraft; import net.minecraft.util.profiling.ProfilerFiller; -import net.minecraft.server.packs.PackResources; import net.minecraft.server.packs.PackType; -import net.minecraft.server.packs.metadata.pack.PackMetadataSection; import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.api.distmarker.OnlyIn; import net.minecraftforge.common.ForgeConfig; import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.client.gui.LoadingErrorScreen; -import net.minecraftforge.resource.DelegatingPackResources; import net.minecraftforge.resource.ResourcePackLoader; import net.minecraftforge.server.LanguageHook; -import net.minecraftforge.forgespi.language.IModInfo; @OnlyIn(Dist.CLIENT) public class ClientModLoader { @@ -117,12 +98,7 @@ public class ClientModLoader { public static boolean completeModLoading() { var warnings = ModLoader.get().getWarnings(); - boolean showWarnings = true; - try { - showWarnings = ForgeConfig.CLIENT.showLoadWarnings.get(); - } catch (NullPointerException | IllegalStateException e) { - // We're in an early error state, config is not available. Assume true. - } + boolean showWarnings = ForgeConfig.CLIENT.showLoadWarnings(); if (!showWarnings) { //User disabled warning screen, as least log them diff --git a/src/main/java/net/minecraftforge/common/ForgeConfig.java b/src/main/java/net/minecraftforge/common/ForgeConfig.java index 0944705bb0..61153442e3 100644 --- a/src/main/java/net/minecraftforge/common/ForgeConfig.java +++ b/src/main/java/net/minecraftforge/common/ForgeConfig.java @@ -158,6 +158,16 @@ public class ForgeConfig { builder.pop(); } + + // Allow these to be called before the config is loaded because its used before loading the error screens. + // Prevents a ton of spam when an error screen is displayed. + public final boolean calculateAllNormals() { + return clientSpec.isLoaded() ? calculateAllNormals.get() : calculateAllNormals.getDefault(); + } + + public final boolean showLoadWarnings() { + return clientSpec.isLoaded() ? showLoadWarnings.get() : showLoadWarnings.getDefault(); + } } static final ForgeConfigSpec clientSpec;