From 72e3a115d660c82f58a4e3d37b180c2fd25f5ca2 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 13:07:34 -0400 Subject: [PATCH 01/23] Move item filling quirk to correct vanilla location on 1.19+ --- .../perf/blast_search_trees/MinecraftMixin.java | 16 ---------------- .../modernfix/searchtree/DummySearchTree.java | 12 +++++++++++- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/blast_search_trees/MinecraftMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/blast_search_trees/MinecraftMixin.java index 252754f2..02443ba0 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/blast_search_trees/MinecraftMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/blast_search_trees/MinecraftMixin.java @@ -3,11 +3,6 @@ package org.embeddedt.modernfix.common.mixin.perf.blast_search_trees; import net.minecraft.client.KeyMapping; import net.minecraft.client.Minecraft; import net.minecraft.client.searchtree.SearchRegistry; -import net.minecraft.core.NonNullList; -import net.minecraft.core.Registry; -import net.minecraft.world.item.CreativeModeTab; -import net.minecraft.world.item.Item; -import net.minecraft.world.item.ItemStack; import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; import org.embeddedt.modernfix.searchtree.DummySearchTree; @@ -32,7 +27,6 @@ public class MinecraftMixin { if(provider == null) return; ModernFix.LOGGER.info("Replacing search trees with '{}' provider", provider.getName()); - mfix$runItemFillingQuirk(); this.searchRegistry.register(SearchRegistry.CREATIVE_NAMES, list -> provider.getSearchTree(false)); this.searchRegistry.register(SearchRegistry.CREATIVE_TAGS, list -> provider.getSearchTree(true)); this.searchRegistry.register(SearchRegistry.RECIPE_COLLECTIONS, list -> new DummySearchTree<>()); @@ -46,14 +40,4 @@ public class MinecraftMixin { GLFW.glfwSetErrorCallback(oldCb); ci.cancel(); } - - private void mfix$runItemFillingQuirk() { - // quirk: call fillItemCategory on all items in the registry in case they do classloading inside it - // see https://github.com/Shadows-of-Fire/GatewaysToEternity/issues/29 for an example of this - NonNullList stacks = NonNullList.create(); - for(Item item : Registry.ITEM) { - stacks.clear(); - item.fillItemCategory(CreativeModeTab.TAB_SEARCH, stacks); - } - } } diff --git a/common/src/main/java/org/embeddedt/modernfix/searchtree/DummySearchTree.java b/common/src/main/java/org/embeddedt/modernfix/searchtree/DummySearchTree.java index a430faaf..0dd6bf3a 100644 --- a/common/src/main/java/org/embeddedt/modernfix/searchtree/DummySearchTree.java +++ b/common/src/main/java/org/embeddedt/modernfix/searchtree/DummySearchTree.java @@ -1,6 +1,10 @@ package org.embeddedt.modernfix.searchtree; import net.minecraft.client.searchtree.RefreshableSearchTree; +import net.minecraft.core.NonNullList; +import net.minecraft.core.Registry; +import net.minecraft.world.item.CreativeModeTab; +import net.minecraft.world.item.Item; import net.minecraft.world.item.ItemStack; import java.util.Collections; @@ -16,7 +20,13 @@ public class DummySearchTree implements RefreshableSearchTree { @Override public void refresh() { - + // quirk: call fillItemCategory on all items in the registry in case they do classloading inside it + // see https://github.com/Shadows-of-Fire/GatewaysToEternity/issues/29 for an example of this + NonNullList stacks = NonNullList.create(); + for(Item item : Registry.ITEM) { + stacks.clear(); + item.fillItemCategory(CreativeModeTab.TAB_SEARCH, stacks); + } } @Override From f469d591b8dc060906ec5df6f7055d6173abdf66 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 13:13:09 -0400 Subject: [PATCH 02/23] Remove item quirk entirely on 1.19+ --- .../modernfix/searchtree/DummySearchTree.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/searchtree/DummySearchTree.java b/common/src/main/java/org/embeddedt/modernfix/searchtree/DummySearchTree.java index 0dd6bf3a..a430faaf 100644 --- a/common/src/main/java/org/embeddedt/modernfix/searchtree/DummySearchTree.java +++ b/common/src/main/java/org/embeddedt/modernfix/searchtree/DummySearchTree.java @@ -1,10 +1,6 @@ package org.embeddedt.modernfix.searchtree; import net.minecraft.client.searchtree.RefreshableSearchTree; -import net.minecraft.core.NonNullList; -import net.minecraft.core.Registry; -import net.minecraft.world.item.CreativeModeTab; -import net.minecraft.world.item.Item; import net.minecraft.world.item.ItemStack; import java.util.Collections; @@ -20,13 +16,7 @@ public class DummySearchTree implements RefreshableSearchTree { @Override public void refresh() { - // quirk: call fillItemCategory on all items in the registry in case they do classloading inside it - // see https://github.com/Shadows-of-Fire/GatewaysToEternity/issues/29 for an example of this - NonNullList stacks = NonNullList.create(); - for(Item item : Registry.ITEM) { - stacks.clear(); - item.fillItemCategory(CreativeModeTab.TAB_SEARCH, stacks); - } + } @Override From eac9edb13af6d352230c6360df9ca4a1cdb7b0d8 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 16:29:13 -0400 Subject: [PATCH 03/23] Fix Forge overriding ResourceKey.equals() with a slower implementation --- .../ResourceKeyMixin.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resource_key_equality/ResourceKeyMixin.java diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resource_key_equality/ResourceKeyMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resource_key_equality/ResourceKeyMixin.java new file mode 100644 index 00000000..4fdf86b7 --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resource_key_equality/ResourceKeyMixin.java @@ -0,0 +1,18 @@ +package org.embeddedt.modernfix.forge.mixin.perf.resource_key_equality; + +import net.minecraft.resources.ResourceKey; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Overwrite; + +@Mixin(ResourceKey.class) +public class ResourceKeyMixin { + /** + * @author embeddedt + * @reason ResourceKeys are interned, so there is no reason to waste time doing any deeper comparison. This override + * is patched in by Forge, it doesn't exist in vanilla + */ + @Overwrite(remap = false) + public boolean equals(Object o) { + return o == this; + } +} From 5853f9b034971a7a01276b24ee82e9ecd2c50b35 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 17:00:30 -0400 Subject: [PATCH 04/23] Fix NPE on Forge error screen --- .../common/mixin/feature/measure_time/MinecraftMixin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/feature/measure_time/MinecraftMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/feature/measure_time/MinecraftMixin.java index 6e7e5df7..f6261313 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/feature/measure_time/MinecraftMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/feature/measure_time/MinecraftMixin.java @@ -40,7 +40,7 @@ public class MinecraftMixin { @Inject(method = "tick", at = @At("HEAD")) private void onClientTick(CallbackInfo ci) { - if(this.overlay == null) { + if(this.overlay == null && ModernFixClient.INSTANCE != null) { ModernFixClient.INSTANCE.onGameLaunchFinish(); } } From 4972081d8a10a7d8d1a84c91e88d92cc2f3e864d Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 17:04:41 -0400 Subject: [PATCH 05/23] Forcefully inject access transformers from mods even if a load error occurs Related: https://github.com/neoforged/NeoForge/issues/43 --- .../core/config/ModernFixEarlyConfig.java | 1 + .../forge/classloading/ATInjector.java | 36 +++++++++++++++++++ .../forge/ModernFixPlatformHooksImpl.java | 4 +++ 3 files changed, 41 insertions(+) create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ATInjector.java diff --git a/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java b/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java index b247fbb4..ba4813f8 100644 --- a/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java +++ b/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java @@ -172,6 +172,7 @@ public class ModernFixEarlyConfig { .put("mixin.devenv", isDevEnv) .put("mixin.perf.remove_spawn_chunks", isDevEnv) .putConditionally(() -> !isFabric, "mixin.bugfix.fix_config_crashes", true) + .putConditionally(() -> !isFabric, "mixin.bugfix.forge_at_inject_error", true) .putConditionally(() -> isFabric, "mixin.perf.clear_fabric_mapping_tables", false) .build(); diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ATInjector.java b/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ATInjector.java new file mode 100644 index 00000000..f86b9623 --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/classloading/ATInjector.java @@ -0,0 +1,36 @@ +package org.embeddedt.modernfix.forge.classloading; + +import net.minecraftforge.fml.loading.FMLLoader; +import net.minecraftforge.fml.loading.moddiscovery.ModFile; +import net.minecraftforge.fml.loading.moddiscovery.ModValidator; +import net.minecraftforge.fml.util.ObfuscationReflectionHelper; +import org.apache.commons.lang3.tuple.Pair; +import org.embeddedt.modernfix.core.ModernFixMixinPlugin; +import org.embeddedt.modernfix.util.CommonModUtil; + +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; + +public class ATInjector { + public static void injectModATs() { + CommonModUtil.runWithoutCrash(() -> { + ModValidator validator = ObfuscationReflectionHelper.getPrivateValue(FMLLoader.class, null, "modValidator"); + List modFiles = ObfuscationReflectionHelper.getPrivateValue(ModValidator.class, validator, "candidateMods"); + List> list = modFiles.stream() + .filter(file -> file.getAccessTransformer().isPresent()) + .map(file -> Pair.of(file, file.getAccessTransformer().get())) + .collect(Collectors.toList()); + if(list.size() > 0) { + ModernFixMixinPlugin.instance.logger.warn("Applying ATs from {} mods despite being in errored state, this might cause a crash!", list.size()); + for(var pair : list) { + try { + FMLLoader.addAccessTransformer(pair.getRight(), pair.getLeft()); + } catch(RuntimeException e) { + ModernFixMixinPlugin.instance.logger.error("Exception occured applying AT from {}", pair.getLeft().getFileName(), e); + } + } + } + }, "applying mod ATs in errored state"); + } +} diff --git a/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java b/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java index 8a53cfbe..6dde752b 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java +++ b/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java @@ -24,6 +24,7 @@ import net.minecraftforge.server.ServerLifecycleHooks; import net.minecraftforge.fml.loading.moddiscovery.ModInfo; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; import org.embeddedt.modernfix.api.constants.IntegrationConstants; +import org.embeddedt.modernfix.forge.classloading.ATInjector; import org.embeddedt.modernfix.forge.classloading.FastAccessTransformerList; import org.embeddedt.modernfix.forge.config.NightConfigFixer; import org.embeddedt.modernfix.forge.packet.PacketHandler; @@ -138,6 +139,9 @@ public class ModernFixPlatformHooksImpl implements ModernFixPlatformHooks { } public void injectPlatformSpecificHacks() { + if(!isEarlyLoadingNormally() && ModernFixMixinPlugin.instance.isOptionEnabled("bugfix.forge_at_inject_error.ATInjector")) { + ATInjector.injectModATs(); + } FastAccessTransformerList.attemptReplace(); /* https://github.com/FabricMC/Mixin/pull/99 */ From 1989f122c60a012a5993bb195bf7bc676678ddbe Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 17:52:50 -0400 Subject: [PATCH 06/23] Remove locking system for Night Config files This can cause deadlocks if mods themselves are also using their own internal locks (Sophisticated Backpacks does this on 1.16 by using a CHM) This system will be replaced by a command/keybind to manually reload configs --- .../modernfix/forge/config/ConfigFixer.java | 23 ++-- .../forge/config/NightConfigFixer.java | 103 ++---------------- 2 files changed, 26 insertions(+), 100 deletions(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java index eaf9621b..49558094 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java @@ -8,6 +8,9 @@ import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; public class ConfigFixer { @@ -35,6 +38,7 @@ public class ConfigFixer { private static class LockingConfigHandler implements Consumer { private final Consumer actualHandler; private final String modId; + private final Lock lock = new ReentrantLock(); LockingConfigHandler(String id, Consumer actualHandler) { this.modId = id; @@ -43,14 +47,17 @@ public class ConfigFixer { @Override public void accept(ModConfig.ModConfigEvent modConfigEvent) { - Object cfgObj = NightConfigFixer.toWriteSyncConfig(modConfigEvent.getConfig().getConfigData()); - if(cfgObj != null) { - // don't synchronize on 'this' as it produces a deadlock when used alongside NightConfigFixer - synchronized (cfgObj) { - this.actualHandler.accept(modConfigEvent); - } - } else { - this.actualHandler.accept(modConfigEvent); + try { + if(lock.tryLock(2, TimeUnit.SECONDS)) { + try { + this.actualHandler.accept(modConfigEvent); + } finally { + lock.unlock(); + } + } else + ModernFix.LOGGER.error("Failed to post config event for {}, someone else is holding the lock", modId); + } catch(InterruptedException e) { + Thread.currentThread().interrupt(); } } diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java index a6993a07..c0cc9432 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java @@ -1,29 +1,22 @@ package org.embeddedt.modernfix.forge.config; -import com.electronwill.nightconfig.core.file.CommentedFileConfig; -import com.electronwill.nightconfig.core.file.FileConfig; import com.electronwill.nightconfig.core.file.FileWatcher; import cpw.mods.modlauncher.api.LamdbaExceptionUtils; -import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet; import net.minecraftforge.fml.common.ObfuscationReflectionHelper; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; import org.embeddedt.modernfix.util.CommonModUtil; import java.lang.reflect.Field; import java.nio.file.Path; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; +import java.util.LinkedHashSet; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; -/** - * Relatively simple patch to wait for config saving to finish, made complex by Night Config classes being package-private, - * and Forge not allowing mixins into libraries. - */ public class NightConfigFixer { + public static final LinkedHashSet configsToReload = new LinkedHashSet<>(); public static void monitorFileWatcher() { + if(!ModernFixMixinPlugin.instance.isOptionEnabled("bugfix.fix_config_crashes.NightConfigFixerMixin")) + return; CommonModUtil.runWithoutCrash(() -> { FileWatcher watcher = FileWatcher.defaultInstance(); Field field = FileWatcher.class.getDeclaredField("watchedFiles"); @@ -38,30 +31,6 @@ public class NightConfigFixer { private static final Class WATCHED_FILE = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.FileWatcher$WatchedFile")); private static final Field CHANGE_HANDLER = ObfuscationReflectionHelper.findField(WATCHED_FILE, "changeHandler"); - public static final Class WRITE_SYNC_CONFIG = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.WriteSyncFileConfig")); - private static final Class AUTOSAVE_CONFIG = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.AutosaveCommentedFileConfig")); - private static final Field AUTOSAVE_FILECONFIG = ObfuscationReflectionHelper.findField(AUTOSAVE_CONFIG, "fileConfig"); - - private static final Field CURRENTLY_WRITING = ObfuscationReflectionHelper.findField(WRITE_SYNC_CONFIG, "currentlyWriting"); - - private static final Map, Field> CONFIG_WATCHER_TO_CONFIG_FIELD = Collections.synchronizedMap(new HashMap<>()); - - private static Field getCurrentlyWritingFieldOnWatcher(Object watcher) { - return CONFIG_WATCHER_TO_CONFIG_FIELD.computeIfAbsent(watcher.getClass(), clz -> { - while(clz != null && clz != Object.class) { - for(Field f : clz.getDeclaredFields()) { - if(CommentedFileConfig.class.isAssignableFrom(f.getType())) { - f.setAccessible(true); - ModernFixMixinPlugin.instance.logger.debug("Found CommentedFileConfig: field '{}' on {}", f.getName(), clz.getName()); - return f; - } - } - clz = clz.getSuperclass(); - } - return null; - }); - } - static class MonitoringMap extends ConcurrentHashMap { public MonitoringMap(ConcurrentHashMap oldMap) { super(oldMap); @@ -82,27 +51,6 @@ public class NightConfigFixer { } } - private static final Set> UNKNOWN_FILE_CONFIG_CLASSES = Collections.synchronizedSet(new ReferenceOpenHashSet<>()); - - public static Object toWriteSyncConfig(Object config) { - if(config == null) - return null; - try { - if(WRITE_SYNC_CONFIG.isAssignableFrom(config.getClass())) { - return config; - } else if(AUTOSAVE_CONFIG.isAssignableFrom(config.getClass())) { - FileConfig fc = (FileConfig)AUTOSAVE_FILECONFIG.get(config); - return toWriteSyncConfig(fc); - } else { - if (UNKNOWN_FILE_CONFIG_CLASSES.add(config.getClass())) - ModernFixMixinPlugin.instance.logger.warn("Unexpected FileConfig class: {}", config.getClass().getName()); - return null; - } - } catch(ReflectiveOperationException e) { - return null; - } - } - static class MonitoringConfigTracker implements Runnable { private final Runnable configTracker; @@ -110,47 +58,18 @@ public class NightConfigFixer { this.configTracker = r; } - private void protectFromSaving(FileConfig config, Runnable runnable) throws ReflectiveOperationException { - Object writeSyncConfig = toWriteSyncConfig(config); - if(writeSyncConfig != null) { - // keep trying to write, releasing the config lock each time in case something else needs to lock it - // for any reason - while(true) { - // acquiring synchronized block here should in theory prevent any other concurrent loads/saves, based - // off WriteSyncFileConfig implementation - synchronized (writeSyncConfig) { - if(CURRENTLY_WRITING.getBoolean(writeSyncConfig)) { - ModernFixMixinPlugin.instance.logger.fatal("Config being written during load!!!"); - try { Thread.sleep(500); } catch(InterruptedException e) { Thread.currentThread().interrupt(); } - continue; - } - // at this point, currentlyWriting is false, and we acquired synchronized lock, should be good to - // go - runnable.run(); - break; - } - } - } else { - runnable.run(); - } - } - /** - * This entrypoint runs when the file watcher has detected a change on the config file. Before passing - * this through to Forge, use reflection hacks to confirm the config system is not still writing to the file. - * If it is, spin until writing finishes. Immediately returning might result in the event never being observed. + * Add the config */ @Override public void run() { - try { - Field theField = getCurrentlyWritingFieldOnWatcher(this.configTracker); - if(theField != null) { - CommentedFileConfig cfg = (CommentedFileConfig)theField.get(this.configTracker); - // will synchronize and check saving flag - protectFromSaving(cfg, configTracker); + synchronized(configsToReload) { + int oldSize = configsToReload.size(); + configsToReload.add(configTracker); + if(oldSize == 0) { + ModernFixMixinPlugin.instance.logger.info("Config file change detected on disk. The Forge feature to watch config files for changes is currently disabled due to random corruption issues."); + ModernFixMixinPlugin.instance.logger.info("This functionality will be restored in a future ModernFix update."); } - } catch(ReflectiveOperationException e) { - e.printStackTrace(); } } } From dbff17a1ffd535efb9f7cbe2376f62540f7ab31e Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 18:08:46 -0400 Subject: [PATCH 07/23] Better fix for config corruption Defer posting of all config reload events to the main thread, and don't process any until after the launch finishes. This should hopefully fix some synchronization issues --- .../forge/config/NightConfigFixer.java | 35 +++++++++++++++---- .../forge/init/ModernFixClientForge.java | 6 ++++ .../modernfix/forge/init/ModernFixForge.java | 11 ++++++ .../forge/ModernFixPlatformHooksImpl.java | 2 ++ 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java index c0cc9432..833e6eb8 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java @@ -3,17 +3,21 @@ package org.embeddedt.modernfix.forge.config; import com.electronwill.nightconfig.core.file.FileWatcher; import cpw.mods.modlauncher.api.LamdbaExceptionUtils; import net.minecraftforge.fml.common.ObfuscationReflectionHelper; +import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; import org.embeddedt.modernfix.util.CommonModUtil; import java.lang.reflect.Field; import java.nio.file.Path; +import java.util.ArrayList; import java.util.LinkedHashSet; +import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; public class NightConfigFixer { public static final LinkedHashSet configsToReload = new LinkedHashSet<>(); + private static int tickCounter = 0; public static void monitorFileWatcher() { if(!ModernFixMixinPlugin.instance.isOptionEnabled("bugfix.fix_config_crashes.NightConfigFixerMixin")) return; @@ -28,6 +32,30 @@ public class NightConfigFixer { }, "replacing Night Config watchedFiles map"); } + /** + * Called by the render thread on the client, and the server thread on the server. Processes all the accumulated + * file watch events. + */ + public static void runReloads() { + if((tickCounter++ % 20) != 0) + return; + List runnablesToRun; + synchronized (configsToReload) { + if(configsToReload.isEmpty()) + return; + runnablesToRun = new ArrayList<>(configsToReload); + configsToReload.clear(); + } + ModernFix.LOGGER.info("Processing {} config reloads", runnablesToRun.size()); + for(Runnable r : runnablesToRun) { + try { + r.run(); + } catch(RuntimeException e) { + e.printStackTrace(); + } + } + } + private static final Class WATCHED_FILE = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.FileWatcher$WatchedFile")); private static final Field CHANGE_HANDLER = ObfuscationReflectionHelper.findField(WATCHED_FILE, "changeHandler"); @@ -59,17 +87,12 @@ public class NightConfigFixer { } /** - * Add the config + * Add the config runnable to the list to be processed by the main thread. */ @Override public void run() { synchronized(configsToReload) { - int oldSize = configsToReload.size(); configsToReload.add(configTracker); - if(oldSize == 0) { - ModernFixMixinPlugin.instance.logger.info("Config file change detected on disk. The Forge feature to watch config files for changes is currently disabled due to random corruption issues."); - ModernFixMixinPlugin.instance.logger.info("This functionality will be restored in a future ModernFix update."); - } } } } diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java index 05b901b0..bc361764 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java @@ -4,6 +4,7 @@ import com.mojang.blaze3d.platform.InputConstants; import net.minecraft.client.KeyMapping; import net.minecraft.client.Minecraft; import net.minecraft.client.gui.components.DebugScreenOverlay; +import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.client.event.RecipesUpdatedEvent; import net.minecraftforge.client.event.RenderGameOverlayEvent; import net.minecraftforge.client.gui.ForgeIngameGui; @@ -20,7 +21,9 @@ import net.minecraftforge.fml.common.ObfuscationReflectionHelper; import net.minecraftforge.fml.event.lifecycle.FMLClientSetupEvent; import net.minecraftforge.fml.event.server.FMLServerStartedEvent; import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext; +import net.minecraftforge.fml.loading.FMLEnvironment; import org.embeddedt.modernfix.ModernFixClient; +import org.embeddedt.modernfix.forge.config.NightConfigFixer; import org.embeddedt.modernfix.screen.ModernFixConfigScreen; import java.util.ArrayList; @@ -51,6 +54,9 @@ public class ModernFixClientForge { if(event.phase == TickEvent.Phase.START && configKey.consumeClick()) { Minecraft.getInstance().setScreen(new ModernFixConfigScreen(Minecraft.getInstance().screen)); } + if(FMLEnvironment.dist == Dist.CLIENT && event.phase == TickEvent.Phase.START && ModernFixForge.launchDone) { + NightConfigFixer.runReloads(); + } } private static final List brandingList = new ArrayList<>(); diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java index 093158e5..72d67ede 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java @@ -6,6 +6,7 @@ import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.event.OnDatapackSyncEvent; import net.minecraftforge.event.RegistryEvent; +import net.minecraftforge.event.TickEvent; import net.minecraftforge.eventbus.api.EventPriority; import net.minecraftforge.eventbus.api.SubscribeEvent; import net.minecraftforge.fml.*; @@ -15,6 +16,7 @@ import net.minecraftforge.fml.event.lifecycle.FMLCommonSetupEvent; import net.minecraftforge.fml.event.server.FMLServerStartedEvent; import net.minecraftforge.fml.event.server.FMLServerStoppedEvent; import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext; +import net.minecraftforge.fml.loading.FMLEnvironment; import net.minecraftforge.fml.loading.FMLLoader; import net.minecraftforge.fml.network.FMLNetworkConstants; import net.minecraftforge.fml.server.ServerLifecycleHooks; @@ -27,6 +29,7 @@ import org.embeddedt.modernfix.forge.classloading.ModFileScanDataDeduplicator; import org.embeddedt.modernfix.forge.ModernFixConfig; import org.embeddedt.modernfix.entity.EntityDataIDSyncHandler; import org.embeddedt.modernfix.forge.config.ConfigFixer; +import org.embeddedt.modernfix.forge.config.NightConfigFixer; import org.embeddedt.modernfix.forge.packet.PacketHandler; import org.embeddedt.modernfix.forge.registry.ObjectHolderClearer; import org.embeddedt.modernfix.forge.util.KubeUtil; @@ -36,6 +39,7 @@ import java.util.List; @Mod(ModernFix.MODID) public class ModernFixForge { private static ModernFix commonMod; + public static boolean launchDone = false; public ModernFixForge() { commonMod = new ModernFix(); @@ -54,6 +58,13 @@ public class ModernFixForge { ConfigFixer.replaceConfigHandlers(); } + @SubscribeEvent + public void onServerTick(TickEvent.ServerTickEvent event) { + if(FMLEnvironment.dist == Dist.DEDICATED_SERVER && event.phase == TickEvent.Phase.END && ModernFixForge.launchDone) { + NightConfigFixer.runReloads(); + } + } + @SubscribeEvent public void onDatapackSync(OnDatapackSyncEvent event) { if(event.getPlayer() != null) { diff --git a/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java b/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java index 44565b96..6463a58c 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java +++ b/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java @@ -30,6 +30,7 @@ import org.embeddedt.modernfix.api.constants.IntegrationConstants; import org.embeddedt.modernfix.forge.classloading.FastAccessTransformerList; import org.embeddedt.modernfix.forge.classloading.ModernFixResourceFinder; import org.embeddedt.modernfix.forge.config.NightConfigFixer; +import org.embeddedt.modernfix.forge.init.ModernFixForge; import org.embeddedt.modernfix.forge.packet.PacketHandler; import org.embeddedt.modernfix.forge.spark.SparkLaunchProfiler; import org.embeddedt.modernfix.platform.ModernFixPlatformHooks; @@ -270,6 +271,7 @@ public class ModernFixPlatformHooksImpl implements ModernFixPlatformHooks { if(ModernFixMixinPlugin.instance.isOptionEnabled("feature.spark_profile_launch.OnForge")) { CommonModUtil.runWithoutCrash(() -> SparkLaunchProfiler.stop("launch"), "Failed to stop profiler"); } + ModernFixForge.launchDone = true; } public String getPlatformName() { From c8749940f73bfd4e17a53b787ce0dceddbce965a Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 18:09:57 -0400 Subject: [PATCH 08/23] Show log message after reloads are processed --- .../org/embeddedt/modernfix/forge/config/NightConfigFixer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java index 833e6eb8..b503a2a5 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java @@ -46,7 +46,6 @@ public class NightConfigFixer { runnablesToRun = new ArrayList<>(configsToReload); configsToReload.clear(); } - ModernFix.LOGGER.info("Processing {} config reloads", runnablesToRun.size()); for(Runnable r : runnablesToRun) { try { r.run(); @@ -54,6 +53,7 @@ public class NightConfigFixer { e.printStackTrace(); } } + ModernFix.LOGGER.info("Processed {} config reloads", runnablesToRun.size()); } private static final Class WATCHED_FILE = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.FileWatcher$WatchedFile")); From 14170ade1feabd4eee75d788814a5908472a0b80 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Thu, 3 Aug 2023 19:45:14 -0400 Subject: [PATCH 09/23] Implement /mfrc and /mfsrc commands to reload configs on client/server respectively --- .../forge/config/NightConfigFixer.java | 12 +++-------- .../forge/init/ModernFixClientForge.java | 12 ++++++++--- .../modernfix/forge/init/ModernFixForge.java | 21 ++++++++++++------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java index b503a2a5..a471e9ac 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java @@ -3,6 +3,7 @@ package org.embeddedt.modernfix.forge.config; import com.electronwill.nightconfig.core.file.FileWatcher; import cpw.mods.modlauncher.api.LamdbaExceptionUtils; import net.minecraftforge.fml.common.ObfuscationReflectionHelper; +import net.minecraftforge.fml.loading.FMLLoader; import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; import org.embeddedt.modernfix.util.CommonModUtil; @@ -17,7 +18,6 @@ import java.util.function.Function; public class NightConfigFixer { public static final LinkedHashSet configsToReload = new LinkedHashSet<>(); - private static int tickCounter = 0; public static void monitorFileWatcher() { if(!ModernFixMixinPlugin.instance.isOptionEnabled("bugfix.fix_config_crashes.NightConfigFixerMixin")) return; @@ -32,17 +32,9 @@ public class NightConfigFixer { }, "replacing Night Config watchedFiles map"); } - /** - * Called by the render thread on the client, and the server thread on the server. Processes all the accumulated - * file watch events. - */ public static void runReloads() { - if((tickCounter++ % 20) != 0) - return; List runnablesToRun; synchronized (configsToReload) { - if(configsToReload.isEmpty()) - return; runnablesToRun = new ArrayList<>(configsToReload); configsToReload.clear(); } @@ -92,6 +84,8 @@ public class NightConfigFixer { @Override public void run() { synchronized(configsToReload) { + if(configsToReload.size() == 0) + ModernFixMixinPlugin.instance.logger.info("Please use /{} to reload any changed mod config files", FMLLoader.getDist().isDedicatedServer() ? "mfsrc" : "mfrc"); configsToReload.add(configTracker); } } diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java index bc361764..60ac7521 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java @@ -4,7 +4,7 @@ import com.mojang.blaze3d.platform.InputConstants; import net.minecraft.client.KeyMapping; import net.minecraft.client.Minecraft; import net.minecraft.client.gui.components.DebugScreenOverlay; -import net.minecraftforge.api.distmarker.Dist; +import net.minecraftforge.client.event.ClientChatEvent; import net.minecraftforge.client.event.RecipesUpdatedEvent; import net.minecraftforge.client.event.RenderGameOverlayEvent; import net.minecraftforge.client.gui.ForgeIngameGui; @@ -21,7 +21,6 @@ import net.minecraftforge.fml.common.ObfuscationReflectionHelper; import net.minecraftforge.fml.event.lifecycle.FMLClientSetupEvent; import net.minecraftforge.fml.event.server.FMLServerStartedEvent; import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext; -import net.minecraftforge.fml.loading.FMLEnvironment; import org.embeddedt.modernfix.ModernFixClient; import org.embeddedt.modernfix.forge.config.NightConfigFixer; import org.embeddedt.modernfix.screen.ModernFixConfigScreen; @@ -54,8 +53,15 @@ public class ModernFixClientForge { if(event.phase == TickEvent.Phase.START && configKey.consumeClick()) { Minecraft.getInstance().setScreen(new ModernFixConfigScreen(Minecraft.getInstance().screen)); } - if(FMLEnvironment.dist == Dist.CLIENT && event.phase == TickEvent.Phase.START && ModernFixForge.launchDone) { + } + + @SubscribeEvent(priority = EventPriority.LOW) + public void onClientChat(ClientChatEvent event) { + if(event.getMessage() != null && event.getMessage().trim().equals("/mfrc")) { NightConfigFixer.runReloads(); + event.setCanceled(true); + // add it to chat history + Minecraft.getInstance().gui.getChat().addRecentChat(event.getMessage()); } } diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java index 72d67ede..844b9f9f 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java @@ -1,12 +1,14 @@ package org.embeddedt.modernfix.forge.init; import com.google.common.collect.ImmutableList; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import net.minecraft.commands.CommandSourceStack; import net.minecraft.world.item.Item; import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.event.OnDatapackSyncEvent; +import net.minecraftforge.event.RegisterCommandsEvent; import net.minecraftforge.event.RegistryEvent; -import net.minecraftforge.event.TickEvent; import net.minecraftforge.eventbus.api.EventPriority; import net.minecraftforge.eventbus.api.SubscribeEvent; import net.minecraftforge.fml.*; @@ -16,7 +18,6 @@ import net.minecraftforge.fml.event.lifecycle.FMLCommonSetupEvent; import net.minecraftforge.fml.event.server.FMLServerStartedEvent; import net.minecraftforge.fml.event.server.FMLServerStoppedEvent; import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext; -import net.minecraftforge.fml.loading.FMLEnvironment; import net.minecraftforge.fml.loading.FMLLoader; import net.minecraftforge.fml.network.FMLNetworkConstants; import net.minecraftforge.fml.server.ServerLifecycleHooks; @@ -24,10 +25,10 @@ import net.minecraftforge.registries.ForgeRegistries; import org.apache.commons.lang3.tuple.Pair; import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; +import org.embeddedt.modernfix.entity.EntityDataIDSyncHandler; +import org.embeddedt.modernfix.forge.ModernFixConfig; import org.embeddedt.modernfix.forge.classloading.ClassLoadHack; import org.embeddedt.modernfix.forge.classloading.ModFileScanDataDeduplicator; -import org.embeddedt.modernfix.forge.ModernFixConfig; -import org.embeddedt.modernfix.entity.EntityDataIDSyncHandler; import org.embeddedt.modernfix.forge.config.ConfigFixer; import org.embeddedt.modernfix.forge.config.NightConfigFixer; import org.embeddedt.modernfix.forge.packet.PacketHandler; @@ -59,9 +60,15 @@ public class ModernFixForge { } @SubscribeEvent - public void onServerTick(TickEvent.ServerTickEvent event) { - if(FMLEnvironment.dist == Dist.DEDICATED_SERVER && event.phase == TickEvent.Phase.END && ModernFixForge.launchDone) { - NightConfigFixer.runReloads(); + public void onCommandRegister(RegisterCommandsEvent event) { + // Register separate commands since redirecting doesn't work without arguments + for(String name : new String[] { "mfrc", "mfsrc"}) { + event.getDispatcher().register(LiteralArgumentBuilder.literal(name) + .requires(source -> source.hasPermission(3)) + .executes(context -> { + NightConfigFixer.runReloads(); + return 1; + })); } } From e04b05dcc8a8668d8497348c2d1a135f34e30ac2 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 4 Aug 2023 09:42:07 -0400 Subject: [PATCH 10/23] Don't return null for models we claim are in the faked model registry --- .../forge/dynresources/ModelBakeEventHelper.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/ModelBakeEventHelper.java b/forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/ModelBakeEventHelper.java index ffe4cc87..f89d1c20 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/ModelBakeEventHelper.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/ModelBakeEventHelper.java @@ -6,6 +6,7 @@ import com.google.common.collect.Sets; import com.google.common.graph.GraphBuilder; import com.google.common.graph.MutableGraph; import net.minecraft.client.resources.model.BakedModel; +import net.minecraft.client.resources.model.ModelBakery; import net.minecraft.resources.ResourceLocation; import net.minecraft.world.item.Item; import net.minecraft.world.level.block.Block; @@ -14,6 +15,7 @@ import net.minecraftforge.fml.ModContainer; import net.minecraftforge.fml.ModList; import net.minecraftforge.forgespi.language.IModInfo; import net.minecraftforge.registries.ForgeRegistries; +import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.dynamicresources.ModelLocationCache; import org.jetbrains.annotations.Nullable; @@ -69,12 +71,23 @@ public class ModelBakeEventHelper { if(modIdsToInclude.stream().noneMatch(INCOMPATIBLE_MODS::contains)) return this.modelRegistry; Set ourModelLocations = Sets.filter(this.topLevelModelLocations, loc -> modIdsToInclude.contains(loc.getNamespace())); + BakedModel missingModel = modelRegistry.get(ModelBakery.MISSING_MODEL_LOCATION); return new ForwardingMap() { @Override protected Map delegate() { return modelRegistry; } + @Override + public BakedModel get(@Nullable Object key) { + BakedModel model = super.get(key); + if(model == null && key != null && modIdsToInclude.contains(((ResourceLocation)key).getNamespace())) { + ModernFix.LOGGER.warn("Model {} is missing, but was requested in model bake event. Returning missing model", key); + return missingModel; + } + return model; + } + @Override public Set keySet() { return ourModelLocations; From 3df41090e4619f9a67d6d2eabdb53a54fc9d5147 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 4 Aug 2023 10:05:55 -0400 Subject: [PATCH 11/23] Remove resource_key_equality patch as NeoForge fixes it in 1.20+ --- .../ResourceKeyMixin.java | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resource_key_equality/ResourceKeyMixin.java diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resource_key_equality/ResourceKeyMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resource_key_equality/ResourceKeyMixin.java deleted file mode 100644 index 4fdf86b7..00000000 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/resource_key_equality/ResourceKeyMixin.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.embeddedt.modernfix.forge.mixin.perf.resource_key_equality; - -import net.minecraft.resources.ResourceKey; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Overwrite; - -@Mixin(ResourceKey.class) -public class ResourceKeyMixin { - /** - * @author embeddedt - * @reason ResourceKeys are interned, so there is no reason to waste time doing any deeper comparison. This override - * is patched in by Forge, it doesn't exist in vanilla - */ - @Overwrite(remap = false) - public boolean equals(Object o) { - return o == this; - } -} From c1277a2bf55f65e88d6ee56c82957cd7abfdb18b Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 5 Aug 2023 11:42:44 -0400 Subject: [PATCH 12/23] Make a copy of the top-level model list when collecting materials This should prevent CMEs if material collection triggers a model load --- .../mixin/perf/dynamic_resources/ModelBakeryMixin.java | 9 +++++++++ .../mixin/perf/dynamic_resources/ModelBakeryMixin.java | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java index d9977015..30281a73 100644 --- a/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/fabric/src/main/java/org/embeddedt/modernfix/fabric/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -230,6 +230,15 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { pack instanceof FilePackResources; } + /** + * Make a copy of the top-level model list and stream that to avoid CMEs if the getMaterials call triggers a model + * load. + */ + @Redirect(method = "", at = @At(value = "INVOKE", target = "Ljava/util/Collection;stream()Ljava/util/stream/Stream;", ordinal = 0)) + private Stream getModelStream(Collection modelCollection) { + return new ArrayList<>(modelCollection).stream(); + } + @Redirect(method = "", at = @At(value = "INVOKE", target = "Ljava/util/stream/Stream;collect(Ljava/util/stream/Collector;)Ljava/lang/Object;", ordinal = 0)) private Object collectExtraTextures(Stream instance, Collector arCollector) { Set materialsSet = new ObjectOpenHashSet<>(instance.collect(Collectors.toSet())); diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java index f3bdb7d6..88fc3a5e 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -53,6 +53,7 @@ import java.util.*; import java.util.concurrent.TimeUnit; import java.util.function.BiConsumer; import java.util.function.Function; +import java.util.stream.Stream; /* high priority so that our injectors are added before other mods' */ @Mixin(value = ModelBakery.class, priority = 600) @@ -162,6 +163,15 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { } } + /** + * Make a copy of the top-level model list and stream that to avoid CMEs if the getMaterials call triggers a model + * load. + */ + @Redirect(method = "processLoading", at = @At(value = "INVOKE", target = "Ljava/util/Collection;stream()Ljava/util/stream/Stream;", ordinal = 0)) + private Stream getModelStream(Collection modelCollection) { + return new ArrayList<>(modelCollection).stream(); + } + @Redirect(method = "processLoading", at = @At(value = "INVOKE", target = "Lnet/minecraftforge/client/ForgeHooksClient;gatherFluidTextures(Ljava/util/Set;)V", remap = false)) private void gatherModelTextures(Set materialSet) { ForgeHooksClient.gatherFluidTextures(materialSet); From 371e5119f10cf6742745fa4601fed506a164e0b9 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 5 Aug 2023 12:07:21 -0400 Subject: [PATCH 13/23] Never return a non-null model if it was top level in vanilla --- .../DynamicBakedModelProvider.java | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicBakedModelProvider.java b/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicBakedModelProvider.java index 9ecb0f2f..40de0d2d 100644 --- a/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicBakedModelProvider.java +++ b/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicBakedModelProvider.java @@ -9,8 +9,11 @@ import net.minecraft.client.renderer.texture.TextureAtlasSprite; import net.minecraft.client.resources.model.BakedModel; import net.minecraft.client.resources.model.BlockModelRotation; import net.minecraft.client.resources.model.ModelBakery; +import net.minecraft.client.resources.model.ModelResourceLocation; import net.minecraft.core.Direction; +import net.minecraft.core.Registry; import net.minecraft.resources.ResourceLocation; +import net.minecraft.world.level.block.Block; import net.minecraft.world.level.block.state.BlockState; import org.apache.commons.lang3.tuple.Triple; import org.embeddedt.modernfix.ModernFix; @@ -103,6 +106,25 @@ public class DynamicBakedModelProvider implements Map blockOpt = Registry.BLOCK.getOptional(registryKey); + if(blockOpt.isPresent()) { + return ModelBakeryHelpers.getBlockStatesForMRL(blockOpt.get().getStateDefinition(), mrl).size() > 0; + } + } catch(RuntimeException ignored) { + // can occur if the MRL is not valid for that blockstate, ignore + } + } + return false; + } @Override public BakedModel get(Object o) { @@ -117,11 +139,11 @@ public class DynamicBakedModelProvider implements Map Date: Sat, 5 Aug 2023 12:26:18 -0400 Subject: [PATCH 14/23] Make sure missing model never becomes null --- .../modernfix/dynamicresources/DynamicBakedModelProvider.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicBakedModelProvider.java b/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicBakedModelProvider.java index 40de0d2d..b0b5b460 100644 --- a/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicBakedModelProvider.java +++ b/common/src/main/java/org/embeddedt/modernfix/dynamicresources/DynamicBakedModelProvider.java @@ -123,6 +123,8 @@ public class DynamicBakedModelProvider implements Map Date: Sat, 5 Aug 2023 19:38:20 -0400 Subject: [PATCH 15/23] Don't trigger full blockstate cache rebuilds when requesting fluid This causes mods to prematurely rebuild the whole cache, which is not what we want --- .../BlockStateBaseMixin.java | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/reduce_blockstate_cache_rebuilds/BlockStateBaseMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/reduce_blockstate_cache_rebuilds/BlockStateBaseMixin.java index 84acec8a..3d3b3acc 100644 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/reduce_blockstate_cache_rebuilds/BlockStateBaseMixin.java +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/reduce_blockstate_cache_rebuilds/BlockStateBaseMixin.java @@ -1,7 +1,14 @@ package org.embeddedt.modernfix.common.mixin.perf.reduce_blockstate_cache_rebuilds; +import com.google.common.collect.ImmutableMap; +import com.mojang.serialization.MapCodec; +import net.minecraft.world.level.block.Block; import net.minecraft.world.level.block.state.BlockBehaviour; +import net.minecraft.world.level.block.state.BlockState; +import net.minecraft.world.level.block.state.StateHolder; +import net.minecraft.world.level.block.state.properties.Property; import net.minecraft.world.level.material.FluidState; +import net.minecraft.world.level.material.Fluids; import org.embeddedt.modernfix.duck.IBlockState; import org.objectweb.asm.Opcodes; import org.spongepowered.asm.mixin.Dynamic; @@ -14,13 +21,21 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; @Mixin(BlockBehaviour.BlockStateBase.class) -public abstract class BlockStateBaseMixin implements IBlockState { +public abstract class BlockStateBaseMixin extends StateHolder implements IBlockState { + protected BlockStateBaseMixin(Block object, ImmutableMap, Comparable> immutableMap, MapCodec mapCodec) { + super(object, immutableMap, mapCodec); + } + + private static final FluidState MFIX$VANILLA_DEFAULT_FLUID = Fluids.EMPTY.defaultFluidState(); + @Shadow public abstract void initCache(); @Shadow private BlockBehaviour.BlockStateBase.Cache cache; @Shadow private FluidState fluidState; @Shadow private boolean isRandomlyTicking; + @Shadow protected abstract BlockState asState(); + private volatile boolean cacheInvalid = false; private static boolean buildingCache = false; @Override @@ -72,7 +87,11 @@ public abstract class BlockStateBaseMixin implements IBlockState { ordinal = 0 ), require = 0) private FluidState genCacheBeforeGettingFluid(BlockBehaviour.BlockStateBase base) { - generateCache(base); + // don't generate the full cache here as mods will iterate for the fluid state a lot + // assume blockstates will not change their contained fluidstate at runtime more than once + // this is how Lithium's implementation used to work, so it should be fine + if(this.cacheInvalid && this.fluidState == MFIX$VANILLA_DEFAULT_FLUID) + this.fluidState = this.owner.getFluidState(this.asState()); return this.fluidState; } @@ -83,7 +102,8 @@ public abstract class BlockStateBaseMixin implements IBlockState { ordinal = 0 )) private boolean genCacheBeforeGettingTicking(BlockBehaviour.BlockStateBase base) { - generateCache(base); + if(this.cacheInvalid) + return this.owner.isRandomlyTicking(this.asState()); return this.isRandomlyTicking; } From 795aca19e031707ae66fc0f011b7d3ad1dd8e744 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 5 Aug 2023 19:42:34 -0400 Subject: [PATCH 16/23] Don't enable blast_search_trees with REI present on 1.16 --- .../embeddedt/modernfix/core/config/ModernFixEarlyConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java b/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java index 353919c6..ae5fa9ee 100644 --- a/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java +++ b/common/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java @@ -141,7 +141,7 @@ public class ModernFixEarlyConfig { private static final boolean isDevEnv = ModernFixPlatformHooks.INSTANCE.isDevEnv(); static { - shouldReplaceSearchTrees = modPresent("jei"); + shouldReplaceSearchTrees = modPresent("jei") && !modPresent("roughlyenoughitems"); } private static class DefaultSettingMapBuilder extends ImmutableMap.Builder { From dc59c9bf0c85dd9863ff21ea68ee9aaa75f49067 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 7 Aug 2023 10:45:59 -0400 Subject: [PATCH 17/23] Update CTM integration for 1.19.4+ --- .../dynamic_resources/ModelBakeryMixin.java | 2 +- .../ctm/TextureMetadataHandlerMixin.java | 39 ++++++++++++++++--- gradle.properties | 2 +- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java index 38e9d2f8..adf3c186 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakeryMixin.java @@ -274,7 +274,7 @@ public abstract class ModelBakeryMixin implements IExtendedModelBakery { public BakedModel bakeDefault(ResourceLocation modelLocation, ModelState state) { ModelBakery self = (ModelBakery) (Object) this; ModelBaker theBaker = self.new ModelBakerImpl(textureGetter, modelLocation); - return theBaker.bake(modelLocation, state); + return theBaker.bake(modelLocation, state, theBaker.getModelTextureGetter()); } @Override diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java index 76c909b7..de8d78ae 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java @@ -1,20 +1,27 @@ package org.embeddedt.modernfix.forge.mixin.perf.dynamic_resources.ctm; +import com.google.common.collect.Multimap; +import com.google.common.collect.Sets; import com.mojang.datafixers.util.Pair; +import net.minecraft.client.Minecraft; +import net.minecraft.client.renderer.texture.TextureAtlasSprite; import net.minecraft.client.resources.model.*; import net.minecraft.resources.ResourceLocation; import org.embeddedt.modernfix.ModernFixClient; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; import org.embeddedt.modernfix.annotation.RequiresMod; import org.embeddedt.modernfix.api.entrypoint.ModernFixClientIntegration; -import org.embeddedt.modernfix.api.helpers.ModelHelpers; +import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import team.chisel.ctm.CTM; +import team.chisel.ctm.api.model.IModelCTM; +import team.chisel.ctm.client.mixin.ModelBakerImplAccessor; import team.chisel.ctm.client.model.AbstractCTMBakedModel; +import team.chisel.ctm.client.model.ModelCTM; import team.chisel.ctm.client.texture.IMetadataSectionCTM; import team.chisel.ctm.client.util.ResourceUtil; import team.chisel.ctm.client.util.TextureMetadataHandler; @@ -22,20 +29,23 @@ import team.chisel.ctm.client.util.TextureMetadataHandler; import javax.annotation.Nonnull; import java.io.IOException; import java.util.*; +import java.util.function.Function; @Mixin(TextureMetadataHandler.class) @RequiresMod("ctm") @ClientOnlyMixin public abstract class TextureMetadataHandlerMixin implements ModernFixClientIntegration { - @Shadow @Nonnull protected abstract BakedModel wrap(ResourceLocation loc, UnbakedModel model, BakedModel object, ModelBaker loader) throws IOException; + @Shadow @Nonnull protected abstract BakedModel wrap(UnbakedModel model, BakedModel object) throws IOException; + + @Shadow @Final public static Multimap TEXTURES_SCRAPED; @Inject(method = "", at = @At("RETURN")) private void subscribeDynamic(CallbackInfo ci) { ModernFixClient.CLIENT_INTEGRATIONS.add(this); } - @Inject(method = "onModelBake", at = @At("HEAD"), cancellable = true, remap = false) + @Inject(method = { "onModelBake(Lnet/minecraftforge/client/event/ModelEvent$ModifyBakingResult;)V", "onModelBake(Lnet/minecraftforge/client/event/ModelEvent$BakingCompleted;)V" }, at = @At("HEAD"), cancellable = true, remap = false) private void noIteration(CallbackInfo ci) { ci.cancel(); } @@ -59,8 +69,7 @@ public abstract class TextureMetadataHandlerMixin implements ModernFixClientInte continue; } - // TODO port - Collection textures = Collections.emptyList(); // model.getMaterials(event.getModelLoader()::getModel, errors); + Collection textures = Sets.newHashSet(TEXTURES_SCRAPED.get(dep)); Collection newDependencies = model.getDependencies(); for (Material tex : textures) { IMetadataSectionCTM meta = null; @@ -82,7 +91,8 @@ public abstract class TextureMetadataHandlerMixin implements ModernFixClientInte } if (shouldWrap) { try { - baked = wrap(rl, rootModel, baked, ModelHelpers.adaptBakery(bakery)); + baked = wrap(rootModel, baked); + handleInit(rl, baked, bakery); dependencies.clear(); } catch (IOException e) { CTM.logger.error("Could not wrap model " + rl + ". Aborting...", e); @@ -91,4 +101,21 @@ public abstract class TextureMetadataHandlerMixin implements ModernFixClientInte } return baked; } + + private void handleInit(ResourceLocation key, BakedModel wrappedModel, ModelBakery bakery) { + if(wrappedModel instanceof AbstractCTMBakedModel baked) { + IModelCTM var10 = baked.getModel(); + if (var10 instanceof ModelCTM ctmModel) { + if (!ctmModel.isInitialized()) { + Function spriteGetter = (m) -> { + return Minecraft.getInstance().getModelManager().getAtlas(m.atlasLocation()).getSprite(m.texture()); + }; + ModelBakery.ModelBakerImpl baker = ModelBakerImplAccessor.createImpl(bakery, ($, m) -> { + return spriteGetter.apply(m); + }, key); + ctmModel.bake(baker, spriteGetter, BlockModelRotation.X0_Y0, key); + } + } + } + } } diff --git a/gradle.properties b/gradle.properties index 8435a3c6..c3ac8c24 100644 --- a/gradle.properties +++ b/gradle.properties @@ -11,7 +11,7 @@ parchment_version=2023.06.26 refined_storage_version=4392788 jei_version=13.1.0.2 rei_version=11.0.597 -ctm_version=1.19.2-1.1.7+11 +ctm_version=1.19.3-1.1.7+14 kubejs_version=1902.6.0-build.142 rhino_version=1902.2.2-build.268 supported_minecraft_versions=1.19.4 From e0a170db928ec21c673dd8287fe72d77e40ace01 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:21:47 -0400 Subject: [PATCH 18/23] Fix CTM block models not working (only item models worked) --- .../forge/dynresources/IModelBakerImpl.java | 5 +++++ .../perf/dynamic_resources/ModelBakerImplMixin.java | 12 ++++++++++-- .../ctm/TextureMetadataHandlerMixin.java | 3 +++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/IModelBakerImpl.java diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/IModelBakerImpl.java b/forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/IModelBakerImpl.java new file mode 100644 index 00000000..b2cced35 --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/IModelBakerImpl.java @@ -0,0 +1,5 @@ +package org.embeddedt.modernfix.forge.dynresources; + +public interface IModelBakerImpl { + void mfix$ignoreCache(); +} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java index ba9ee2e9..9ca72ed5 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ModelBakerImplMixin.java @@ -9,6 +9,7 @@ import org.embeddedt.modernfix.ModernFixClient; import org.embeddedt.modernfix.api.entrypoint.ModernFixClientIntegration; import org.embeddedt.modernfix.duck.IExtendedModelBakery; import org.embeddedt.modernfix.dynamicresources.DynamicBakedModelProvider; +import org.embeddedt.modernfix.forge.dynresources.IModelBakerImpl; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; @@ -19,16 +20,23 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; import java.util.function.Function; @Mixin(ModelBakery.ModelBakerImpl.class) -public abstract class ModelBakerImplMixin { +public abstract class ModelBakerImplMixin implements IModelBakerImpl { private static final boolean debugDynamicModelLoading = Boolean.getBoolean("modernfix.debugDynamicModelLoading"); @Shadow @Final private ModelBakery field_40571; @Shadow public abstract UnbakedModel getModel(ResourceLocation arg); + private boolean mfix$ignoreCache = false; + + @Override + public void mfix$ignoreCache() { + mfix$ignoreCache = true; + } + @Inject(method = "bake(Lnet/minecraft/resources/ResourceLocation;Lnet/minecraft/client/resources/model/ModelState;Ljava/util/function/Function;)Lnet/minecraft/client/resources/model/BakedModel;", at = @At("HEAD"), cancellable = true, remap = false) public void getOrLoadBakedModelDynamic(ResourceLocation arg, ModelState arg2, Function textureGetter, CallbackInfoReturnable cir) { ModelBakery.BakedCacheKey key = new ModelBakery.BakedCacheKey(arg, arg2.getRotation(), arg2.isUvLocked()); - BakedModel existing = this.field_40571.bakedCache.get(key); + BakedModel existing = mfix$ignoreCache ? null : this.field_40571.bakedCache.get(key); if (existing != null) { cir.setReturnValue(existing); } else { diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java index de8d78ae..34e740a5 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/dynamic_resources/ctm/TextureMetadataHandlerMixin.java @@ -11,6 +11,7 @@ import org.embeddedt.modernfix.ModernFixClient; import org.embeddedt.modernfix.annotation.ClientOnlyMixin; import org.embeddedt.modernfix.annotation.RequiresMod; import org.embeddedt.modernfix.api.entrypoint.ModernFixClientIntegration; +import org.embeddedt.modernfix.forge.dynresources.IModelBakerImpl; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; @@ -113,6 +114,8 @@ public abstract class TextureMetadataHandlerMixin implements ModernFixClientInte ModelBakery.ModelBakerImpl baker = ModelBakerImplAccessor.createImpl(bakery, ($, m) -> { return spriteGetter.apply(m); }, key); + // bypass bakedCache so that dependent models get re-baked and thus retrieve their sprites again + ((IModelBakerImpl)baker).mfix$ignoreCache(); ctmModel.bake(baker, spriteGetter, BlockModelRotation.X0_Y0, key); } } From 4ed5e8a434061bb2dd4d921e3a63a3619bc471f2 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:22:29 -0400 Subject: [PATCH 19/23] Update CTM version --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index d0e7122f..21073207 100644 --- a/gradle.properties +++ b/gradle.properties @@ -11,7 +11,7 @@ parchment_version=2023.07.09 refined_storage_version=4392788 jei_version=13.1.0.2 rei_version=11.0.597 -ctm_version=1.19.3-1.1.7+14 +ctm_version=1.20.1-1.1.8+4 kubejs_version=1902.6.0-build.142 rhino_version=1902.2.2-build.268 supported_minecraft_versions=1.20.1 From 8875710f3df33eeb546a884c7e09b89bfde4be41 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Mon, 7 Aug 2023 21:51:54 -0400 Subject: [PATCH 20/23] Use client commands on 1.18+ for /mfrc --- .../forge/init/ModernFixClientForge.java | 17 +++++++++-------- .../modernfix/forge/init/ModernFixForge.java | 3 +-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java index b64c6c0d..6915a9b5 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java @@ -1,13 +1,15 @@ package org.embeddedt.modernfix.forge.init; import com.mojang.blaze3d.platform.InputConstants; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; import net.minecraft.client.KeyMapping; import net.minecraft.client.Minecraft; import net.minecraft.client.gui.components.DebugScreenOverlay; +import net.minecraft.commands.CommandSourceStack; import net.minecraftforge.client.ClientRegistry; import net.minecraftforge.client.ConfigGuiHandler; -import net.minecraftforge.client.event.ClientChatEvent; import net.minecraftforge.client.event.RecipesUpdatedEvent; +import net.minecraftforge.client.event.RegisterClientCommandsEvent; import net.minecraftforge.client.event.RenderGameOverlayEvent; import net.minecraftforge.client.gui.ForgeIngameGui; import net.minecraftforge.client.settings.KeyConflictContext; @@ -55,13 +57,12 @@ public class ModernFixClientForge { } @SubscribeEvent(priority = EventPriority.LOW) - public void onClientChat(ClientChatEvent event) { - if(event.getMessage() != null && event.getMessage().trim().equals("/mfrc")) { - NightConfigFixer.runReloads(); - event.setCanceled(true); - // add it to chat history - Minecraft.getInstance().gui.getChat().addRecentChat(event.getMessage()); - } + public void onClientChat(RegisterClientCommandsEvent event) { + event.getDispatcher().register(LiteralArgumentBuilder.literal("mfrc") + .executes(context -> { + NightConfigFixer.runReloads(); + return 1; + })); } private static final List brandingList = new ArrayList<>(); diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java index f5aa6c11..20696876 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java @@ -58,8 +58,7 @@ public class ModernFixForge { @SubscribeEvent public void onCommandRegister(RegisterCommandsEvent event) { - // Register separate commands since redirecting doesn't work without arguments - for(String name : new String[] { "mfrc", "mfsrc"}) { + for(String name : new String[] { "mfsrc"}) { event.getDispatcher().register(LiteralArgumentBuilder.literal(name) .requires(source -> source.hasPermission(3)) .executes(context -> { From e2aa482187bd623c31a0f7b8806bba1c19b1050d Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Tue, 8 Aug 2023 10:24:28 -0400 Subject: [PATCH 21/23] Try to provide more guidance to users when reloading configs --- .../assets/modernfix/lang/en_us.json | 1 + .../forge/config/NightConfigFixer.java | 24 ++++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/common/src/main/resources/assets/modernfix/lang/en_us.json b/common/src/main/resources/assets/modernfix/lang/en_us.json index 2d56e1be..5f32b072 100644 --- a/common/src/main/resources/assets/modernfix/lang/en_us.json +++ b/common/src/main/resources/assets/modernfix/lang/en_us.json @@ -7,6 +7,7 @@ "modernfix.perf_mod_warning": "It is recommended to install the mods, but the warning(s) can be disabled in the ModernFix config.", "modernfix.config": "ModernFix mixin config", "modernfix.config.done_restart": "Done (restart required)", + "modernfix.message.reload_config": "Run /mfrc after changing configs on disk for them to take effect.", "modernfix.option.on": "on", "modernfix.option.off": "off", "modernfix.option.disabled": "disabled", diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java index a471e9ac..8ae460a7 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java @@ -2,6 +2,8 @@ package org.embeddedt.modernfix.forge.config; import com.electronwill.nightconfig.core.file.FileWatcher; import cpw.mods.modlauncher.api.LamdbaExceptionUtils; +import net.minecraft.client.Minecraft; +import net.minecraft.network.chat.TranslatableComponent; import net.minecraftforge.fml.common.ObfuscationReflectionHelper; import net.minecraftforge.fml.loading.FMLLoader; import org.embeddedt.modernfix.ModernFix; @@ -14,6 +16,7 @@ import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.function.Function; public class NightConfigFixer { @@ -48,10 +51,10 @@ public class NightConfigFixer { ModernFix.LOGGER.info("Processed {} config reloads", runnablesToRun.size()); } - private static final Class WATCHED_FILE = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.FileWatcher$WatchedFile")); - private static final Field CHANGE_HANDLER = ObfuscationReflectionHelper.findField(WATCHED_FILE, "changeHandler"); - static class MonitoringMap extends ConcurrentHashMap { + private static final Class WATCHED_FILE = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.FileWatcher$WatchedFile")); + private static final Field CHANGE_HANDLER = ObfuscationReflectionHelper.findField(WATCHED_FILE, "changeHandler"); + public MonitoringMap(ConcurrentHashMap oldMap) { super(oldMap); } @@ -71,6 +74,19 @@ public class NightConfigFixer { } } + private static long lastConfigTrigger = System.nanoTime(); + + private static void triggerConfigMessage() { + if((System.nanoTime() - lastConfigTrigger) >= TimeUnit.SECONDS.toNanos(5)) { + lastConfigTrigger = System.nanoTime(); + Minecraft.getInstance().execute(() -> { + if(Minecraft.getInstance().level != null) { + Minecraft.getInstance().gui.getChat().addMessage(new TranslatableComponent("modernfix.message.reload_config")); + } + }); + } + } + static class MonitoringConfigTracker implements Runnable { private final Runnable configTracker; @@ -84,6 +100,8 @@ public class NightConfigFixer { @Override public void run() { synchronized(configsToReload) { + if(FMLLoader.getDist().isClient()) + triggerConfigMessage(); if(configsToReload.size() == 0) ModernFixMixinPlugin.instance.logger.info("Please use /{} to reload any changed mod config files", FMLLoader.getDist().isDedicatedServer() ? "mfsrc" : "mfrc"); configsToReload.add(configTracker); From 6c465c7182a3680b82a9d21570e35818be94fcb8 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Tue, 8 Aug 2023 10:26:57 -0400 Subject: [PATCH 22/23] Update use of Component --- .../embeddedt/modernfix/forge/config/NightConfigFixer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java index cf5f6768..f412dae2 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java @@ -3,7 +3,7 @@ package org.embeddedt.modernfix.forge.config; import com.electronwill.nightconfig.core.file.FileWatcher; import cpw.mods.modlauncher.api.LamdbaExceptionUtils; import net.minecraft.client.Minecraft; -import net.minecraft.network.chat.TranslatableComponent; +import net.minecraft.network.chat.Component; import net.minecraftforge.fml.loading.FMLLoader; import net.minecraftforge.fml.util.ObfuscationReflectionHelper; import org.embeddedt.modernfix.ModernFix; @@ -81,7 +81,7 @@ public class NightConfigFixer { lastConfigTrigger = System.nanoTime(); Minecraft.getInstance().execute(() -> { if(Minecraft.getInstance().level != null) { - Minecraft.getInstance().gui.getChat().addMessage(new TranslatableComponent("modernfix.message.reload_config")); + Minecraft.getInstance().gui.getChat().addMessage(Component.translatable("modernfix.message.reload_config")); } }); } From c1182944f3222e7563b17038b818e6b3fe54695e Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Tue, 8 Aug 2023 18:17:01 -0400 Subject: [PATCH 23/23] Redirect fetchChoiceType directly in vanilla code instead of fully disabling it Should address #204 --- .../dynamic_dfu/BlockEntityTypeMixin.java | 19 +++++++++++++++++++ .../dynamic_dfu/EntityTypeBuilderMixin.java | 19 +++++++++++++++++++ .../dynamic_dfu/SharedConstantsMixin.java | 15 --------------- 3 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/BlockEntityTypeMixin.java create mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/EntityTypeBuilderMixin.java delete mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/SharedConstantsMixin.java diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/BlockEntityTypeMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/BlockEntityTypeMixin.java new file mode 100644 index 00000000..23180408 --- /dev/null +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/BlockEntityTypeMixin.java @@ -0,0 +1,19 @@ +package org.embeddedt.modernfix.common.mixin.perf.dynamic_dfu; + +import com.mojang.datafixers.DSL; +import com.mojang.datafixers.types.Type; +import net.minecraft.world.level.block.entity.BlockEntityType; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +/** + * Prevent fetchChoiceType calls from loading DFU early. Vanilla doesn't need the return values here. + */ +@Mixin(BlockEntityType.class) +public class BlockEntityTypeMixin { + @Redirect(method = "register", at = @At(value = "INVOKE", target = "Lnet/minecraft/Util;fetchChoiceType(Lcom/mojang/datafixers/DSL$TypeReference;Ljava/lang/String;)Lcom/mojang/datafixers/types/Type;")) + private static Type skipSchemaCheck(DSL.TypeReference ref, String s) { + return null; + } +} diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/EntityTypeBuilderMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/EntityTypeBuilderMixin.java new file mode 100644 index 00000000..737778f7 --- /dev/null +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/EntityTypeBuilderMixin.java @@ -0,0 +1,19 @@ +package org.embeddedt.modernfix.common.mixin.perf.dynamic_dfu; + +import com.mojang.datafixers.DSL; +import com.mojang.datafixers.types.Type; +import net.minecraft.world.entity.EntityType; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +/** + * Prevent fetchChoiceType calls from loading DFU early. Vanilla doesn't need the return values here. + */ +@Mixin(EntityType.Builder.class) +public class EntityTypeBuilderMixin { + @Redirect(method = "build", at = @At(value = "INVOKE", target = "Lnet/minecraft/Util;fetchChoiceType(Lcom/mojang/datafixers/DSL$TypeReference;Ljava/lang/String;)Lcom/mojang/datafixers/types/Type;")) + private Type skipSchemaCheck(DSL.TypeReference ref, String s) { + return null; + } +} diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/SharedConstantsMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/SharedConstantsMixin.java deleted file mode 100644 index ece6aaf5..00000000 --- a/common/src/main/java/org/embeddedt/modernfix/common/mixin/perf/dynamic_dfu/SharedConstantsMixin.java +++ /dev/null @@ -1,15 +0,0 @@ -package org.embeddedt.modernfix.common.mixin.perf.dynamic_dfu; - -import net.minecraft.SharedConstants; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; - -@Mixin(SharedConstants.class) -public class SharedConstantsMixin { - @Inject(method = "", at = @At("RETURN")) - private static void skipSchemaCheck(CallbackInfo ci) { - SharedConstants.CHECK_DATA_FIXER_SCHEMA = false; - } -}