Cleanup some mixins

src/main/java/malte0811/ferritecore/FastMap.java

@@ -1,5 +1,6 @@
 package malte0811.ferritecore;
 
+import com.google.common.collect.ImmutableList;
 import net.minecraft.state.Property;
 
 import javax.annotation.Nullable;
@@ -29,7 +30,7 @@ public class FastMap<Value> {
             valuesList.set(getIndexOf(state.getKey()), state.getValue());
         }
         this.values = valuesList;
-        this.rawKeys = new ArrayList<>(properties);
+        this.rawKeys = ImmutableList.copyOf(properties);
     }
 
     @Nullable

src/main/java/malte0811/ferritecore/mixin/PropertyValueConditionMixin.java

@@ -1,5 +1,6 @@
 package malte0811.ferritecore.mixin;
 
+import com.google.common.base.Splitter;
 import malte0811.ferritecore.CachedOrPredicates;
 import net.minecraft.block.Block;
 import net.minecraft.block.BlockState;
@@ -11,16 +12,13 @@ import org.spongepowered.asm.mixin.Final;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.Overwrite;
 import org.spongepowered.asm.mixin.Shadow;
- 
-import com.google.common.base.Splitter;
+
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Predicate;
-
-import malte0811.ferritecore.HackyGlobalState;
+import java.util.stream.Collectors;
 
 @Mixin(PropertyValueCondition.class)
 public class PropertyValueConditionMixin {
@@ -38,7 +36,9 @@ public class PropertyValueConditionMixin {
 
    /**
     * @reason Use cached predicates in the case of multiple specified values
-    * TODO this should be possible with a less invasive mixin?
+    * A less invasive Mixin would be preferable (especially since only one line really changes), but that would involve
+    * redirecting a lambda creation (not currently possible as far as I can tell) and capturing locals (possible, but
+    * annoying)
     * @author malte0811
     */
    @Overwrite

src/main/java/malte0811/ferritecore/mixin/StateholderMixin.java

@@ -7,7 +7,9 @@ import net.minecraft.state.Property;
 import net.minecraft.state.StateHolder;
 import org.spongepowered.asm.mixin.*;
 import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Coerce;
 import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.Redirect;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 
 import java.util.Collection;
@@ -23,8 +25,13 @@ public abstract class StateholderMixin<O, S> {
     @Shadow
     @Final
     protected O instance;
-    @Shadow public abstract Collection<Property<?>> getProperties();
-    @Shadow public abstract <T extends Comparable<T>> T get(Property<T> property);
+
+    @Shadow
+    public abstract Collection<Property<?>> getProperties();
+
+    @Shadow
+    public abstract <T extends Comparable<T>> T get(Property<T> property);
+
     private int globalTableIndex;
     private FastMap<S> globalTable;
 
@@ -70,30 +77,19 @@ public abstract class StateholderMixin<O, S> {
 
     // All other Mixins: If the new data structures are initialized, use those. Otherwise (if populateNeighbors didn't
     // run yet) use the vanilla code using `properties`
-    @Inject(method = "get", at = @At("HEAD"), cancellable = true)
-    public <T extends Comparable<T>>
-    void get(Property<T> property, CallbackInfoReturnable<T> cir) {
+    @Redirect(
+            method = {"get", "func_235903_d_"},
+            at = @At(
+                    value = "INVOKE",
+                    target = "Lcom/google/common/collect/ImmutableMap;get(Ljava/lang/Object;)Ljava/lang/Object;"
+            )
+    )
+    @Coerce
+    public Object getValueOfProperty(ImmutableMap<?, ?> vanillaMap, Object key) {
         if (globalTable != null) {
-            T result = globalTable.getValue(globalTableIndex, property);
-            if (result == null) {
-                throw new IllegalArgumentException("Cannot get property " + property + " as it does not exist in " + this.instance);
-            }
-            cir.setReturnValue(result);
-            cir.cancel();
-        }
-    }
-
-    @Inject(method = "func_235903_d_", at = @At("HEAD"), cancellable = true)
-    public <T extends Comparable<T>>
-    void getOptionalHead(Property<T> property, CallbackInfoReturnable<Optional<T>> cir) {
-        if (globalTable != null) {
-            T result = globalTable.getValue(globalTableIndex, property);
-            if (result == null) {
-                cir.setReturnValue(Optional.empty());
-            } else {
-                cir.setReturnValue(Optional.of(result));
-            }
-            cir.cancel();
+            return globalTable.getValue(globalTableIndex, (Property<?>) key);
+        } else {
+            return vanillaMap.get(key);
         }
     }
 

summary.md

@@ -83,7 +83,8 @@ useful as one of the most common usages of multipart models is pipes, where the
 nearly always boolean properties named `north` etc. As a result the number of predicates
 is reduced from between 10s of thousands and millions to a few ten or hundred instances.
 
-Saved memory: 300-400 MB  
+Saved memory: 300-400 MB (relative to the state after the first change, so 100 MB more
+compared to a "clean" instance)  
 CPU impact: Some impact in model loading (but less allocations), zero while playing  
 Side: client  
 Patches: