Merge 1.16 into 1.18

Author: embeddedt

common/src/main/java/org/embeddedt/modernfix/util/ClassInfoManager.java

@@ -3,16 +3,22 @@
 import org.embeddedt.modernfix.ModernFix;
 import org.embeddedt.modernfix.core.ModernFixMixinPlugin;
 import org.objectweb.asm.tree.ClassNode;
+import org.spongepowered.asm.logging.ILogger;
+import org.spongepowered.asm.logging.LoggerAdapterDefault;
 import org.spongepowered.asm.mixin.MixinEnvironment;
 import org.spongepowered.asm.mixin.extensibility.IMixinInfo;
 import org.spongepowered.asm.mixin.transformer.ClassInfo;
+import org.spongepowered.asm.service.MixinServiceAbstract;
 
 import java.lang.reflect.Field;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 
 public class ClassInfoManager {
     private static boolean hasRun = false;
+    private static final List<Runnable> loggersToRestore = new ArrayList<>();
     public static void clear() {
         if (!ModernFixMixinPlugin.instance.isOptionEnabled("perf.clear_mixin_classinfo.ClassInfoManager") || hasRun)
             return;
@@ -25,11 +31,33 @@ private static Field accessible(Field f) {
         return f;
     }
 
+    private static void changeLoggerAndRestoreLater(Map<String, ILogger> map, ILogger newLogger) {
+        ILogger oldLogger = map.put("mixin.audit", newLogger);
+        loggersToRestore.add(() -> map.put("mixin.audit", oldLogger));
+    }
+
+    private static void disableLoggers() throws ReflectiveOperationException {
+        // Disable default audit logger
+        Field loggersField = accessible(MixinServiceAbstract.class.getDeclaredField("loggers"));
+        changeLoggerAndRestoreLater((Map<String, ILogger>)loggersField.get(null), new LoggerAdapterDefault("mixin.audit"));
+        Class<?> fabricLogger = null;
+        try {
+            fabricLogger = Class.forName("net.fabricmc.loader.impl.knot.MixinLogger");
+        } catch(Throwable e) {
+            // Probably not Fabric
+            return;
+        }
+        // Disable Fabric audit logger
+        loggersField = accessible(fabricLogger.getDeclaredField("LOGGER_MAP"));
+        changeLoggerAndRestoreLater((Map<String, ILogger>)loggersField.get(null), new LoggerAdapterDefault("mixin.audit"));
+    }
+
     private static void doClear() {
         Map<String, ClassInfo> classInfoCache;
         Field mixinField, stateField, classNodeField, methodsField, fieldsField;
         Class<?> stateClz;
         try {
+            disableLoggers();
             Field field = accessible(ClassInfo.class.getDeclaredField("cache"));
             classInfoCache = (Map<String, ClassInfo>) field.get(null);
             mixinField = accessible(ClassInfo.class.getDeclaredField("mixin"));
@@ -70,6 +98,9 @@ private static void doClear() {
         } catch (RuntimeException e) {
             e.printStackTrace();
         }
+        // Put back the old logger
+        loggersToRestore.forEach(Runnable::run);
+        loggersToRestore.clear();
         ModernFix.LOGGER.warn("Cleared mixin data structures");
     }
 }

forge/src/main/java/org/embeddedt/modernfix/forge/dynresources/ModelBakeEventHelper.java

@@ -153,13 +153,26 @@ public boolean containsKey(@Nullable Object key) {
 
             @Override
             public void replaceAll(BiFunction<? super ResourceLocation, ? super BakedModel, ? extends BakedModel> function) {
-                ModernFix.LOGGER.warn("Mod '{}' is calling replaceAll on the model registry. This requires temporarily loading every model for that mod, which is slow.", modId);
+                ModernFix.LOGGER.warn("Mod '{}' is calling replaceAll on the model registry. Some hacks will be used to keep this fast, but they may not be 100% compatible.", modId);
                 List<ResourceLocation> locations = new ArrayList<>(keySet());
                 for(ResourceLocation location : locations) {
-                    BakedModel existing = get(location);
-                    BakedModel replacement = function.apply(location, existing);
-                    if(replacement != existing) {
-                        put(location, replacement);
+                    /*
+                     * Fetching every model is insanely slow. So we call the function with a null object first, since it
+                     * probably isn't expecting that. If we get an exception thrown, or it returns nonnull, then we know
+                     * it actually cares about the given model.
+                     */
+                    boolean needsReplacement;
+                    try {
+                        needsReplacement = function.apply(location, null) != null;
+                    } catch(Throwable e) {
+                        needsReplacement = true;
+                    }
+                    if(needsReplacement) {
+                        BakedModel existing = get(location);
+                        BakedModel replacement = function.apply(location, existing);
+                        if(replacement != existing) {
+                            put(location, replacement);
+                        }
                     }
                 }
             }

forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/model_data_manager_cme/ModelDataManagerMixin.java

@@ -6,13 +6,15 @@
 import net.minecraft.world.level.Level;
 import net.minecraftforge.client.model.ModelDataManager;
 import org.embeddedt.modernfix.annotation.ClientOnlyMixin;
+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.ModifyArg;
 import org.spongepowered.asm.mixin.injection.Redirect;
 
 import java.util.Collections;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
@@ -27,6 +29,8 @@ public abstract class ModelDataManagerMixin {
         throw new AssertionError();
     }
 
+    @Shadow @Final private static Map<ChunkPos, Set<BlockPos>> needModelDataRefresh;
+
     /**
      * Make the set of positions to refresh a real concurrent hash set rather than relying on synchronizedSet,
      * because the returned iterator won't be thread-safe otherwise. See https://github.com/AppliedEnergistics/Applied-Energistics-2/issues/7511
@@ -40,7 +44,8 @@ private static Function<ChunkPos, Set<BlockPos>> changeTypeOfSetUsed(Function<Ch
     private static void onlyRefreshOnMainThread(Level toUpdate, ChunkPos pos) {
         // Only refresh model data on the main thread. This prevents calling getBlockEntity from worker threads
         // which could cause weird CMEs or other behavior.
-        if(Minecraft.getInstance().isSameThread()) {
+        // Avoid the loop if no model data needs to be refreshed, to prevent unnecessary allocation.
+        if(Minecraft.getInstance().isSameThread() && !needModelDataRefresh.isEmpty()) {
             // Refresh the given chunk, and all its neighbors. This is less efficient than the default code
             // but we have no choice since we need to not do refreshing on workers, and blocks might
             // try to access model data in neighboring chunks.

forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/forge_cap_retrieval/LivingEntityMixin.java

@@ -0,0 +1,24 @@
+package org.embeddedt.modernfix.forge.mixin.perf.forge_cap_retrieval;
+
+import net.minecraft.core.Direction;
+import net.minecraft.world.entity.LivingEntity;
+import net.minecraftforge.common.capabilities.Capability;
+import net.minecraftforge.items.CapabilityItemHandler;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Redirect;
+
+import javax.annotation.Nullable;
+
+@Mixin(LivingEntity.class)
+public class LivingEntityMixin {
+    /**
+     * @author embeddedt (issue noted by XFactHD)
+     * @reason check capability equality before checking that entity is alive, the latter requires a lot more
+     * indirection
+     */
+    @Redirect(method = "getCapability", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/entity/LivingEntity;isAlive()Z"))
+    private <T> boolean checkAliveAfterCap(LivingEntity entity, Capability<T> capability, @Nullable Direction facing) {
+        return capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY && entity.isAlive();
+    }
+}