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 d7b505c2..2c9ed884 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 @@ -158,6 +158,9 @@ public class ModernFixEarlyConfig { this.scanForAndBuildMixinOptions(); mixinOptions.addAll(DEFAULT_SETTING_OVERRIDES.keySet()); + if(!isFabric) { + mixinOptions.add("mixin.bugfix.fix_config_crashes"); + } for(String optionName : mixinOptions) { boolean defaultEnabled = DEFAULT_SETTING_OVERRIDES.getOrDefault(optionName, true); this.options.putIfAbsent(optionName, new Option(optionName, defaultEnabled, false)); diff --git a/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java b/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java index 1202500b..352492b1 100644 --- a/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java +++ b/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java @@ -1,157 +1,46 @@ package org.embeddedt.modernfix.util; +import com.google.common.base.Function; import com.google.common.collect.Interner; import com.google.common.collect.Interners; -import it.unimi.dsi.fastutil.objects.*; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; +import com.google.common.collect.Maps; -import java.util.*; -import java.util.function.Function; +import java.util.HashMap; +import java.util.Map; /** - * Replacement backing map for CompoundTags. Uses an array map for tags with 4 or less entries, - * and a hash map for larger tags. + * Replacement backing map for CompoundTags that interns keys. */ -public class CanonizingStringMap implements Map { - private Object2ObjectMap backingMap; - - private static final int GROWTH_THRESHOLD = 4; +public class CanonizingStringMap extends HashMap { private static final Interner KEY_INTERNER = Interners.newStrongInterner(); + private static String intern(String key) { + return key != null ? KEY_INTERNER.intern(key) : null; + } + public CanonizingStringMap() { - this(new Object2ObjectArrayMap<>()); - } - - protected CanonizingStringMap(Object2ObjectMap newMap) { - this.backingMap = newMap; + super(); } @Override - public int size() { - return backingMap.size(); + public T put(String key, T value) { + return super.put(intern(key), value); } @Override - public boolean isEmpty() { - return backingMap.isEmpty(); + public void putAll(Map m) { + HashMap tmp = new HashMap<>(); + m.forEach((k, v) -> tmp.put(intern(k), v)); + super.putAll(tmp); } - @Override - public boolean containsKey(Object o) { - return backingMap.containsKey(o); + private void putAllWithoutInterning(Map m) { + super.putAll(m); } - @Override - public boolean containsValue(Object o) { - return backingMap.containsValue(o); - } - - @Override - public T get(Object o) { - return backingMap.get(o); - } - - @Nullable - @Override - public T put(String s, T t) { - if(backingMap.size() >= GROWTH_THRESHOLD && !(backingMap instanceof Object2ObjectOpenHashMap) && !backingMap.containsKey(s)) { - // map will grow to GROWTH_THRESHOLD + 1 entries, change to hashmap - backingMap = new Object2ObjectOpenHashMap<>(backingMap); - } - s = KEY_INTERNER.intern(s); - return backingMap.put(s, t); - } - - @Override - public T remove(Object o) { - T value = backingMap.remove(o); - // need to shrink to be consistent with new maps - if(backingMap.size() <= GROWTH_THRESHOLD && backingMap instanceof Object2ObjectOpenHashMap) { - backingMap = new Object2ObjectArrayMap<>(backingMap); - } - return value; - } - - @Override - public void putAll(@NotNull Map map) { - if(map.size() == 0) - return; - // grow early if we know there are enough non-overlapping keys - if((map.size() - backingMap.size()) > GROWTH_THRESHOLD && !(backingMap instanceof Object2ObjectOpenHashMap)) { - backingMap = new Object2ObjectOpenHashMap<>(backingMap); - } - map.forEach((String key, T val) -> { - key = KEY_INTERNER.intern(key); - backingMap.put(key, val); - }); - // if it's still an array, and now too big, grow it - if(backingMap.size() > GROWTH_THRESHOLD && !(backingMap instanceof Object2ObjectOpenHashMap)) { - backingMap = new Object2ObjectOpenHashMap<>(backingMap); - } - } - - @Override - public void clear() { - if(!(this.backingMap instanceof Object2ObjectArrayMap)) - this.backingMap = new Object2ObjectArrayMap<>(); - else - this.backingMap.clear(); - } - - @NotNull - @Override - public Set keySet() { - // has to be modifiable because mods (cough, Tinkers) use it to clear the tag - return this.backingMap.keySet(); - } - - @NotNull - @Override - public Collection values() { - return Collections.unmodifiableCollection(this.backingMap.values()); - } - - @NotNull - @Override - public Set> entrySet() { - return Collections.unmodifiableSet(this.backingMap.entrySet()); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CanonizingStringMap that = (CanonizingStringMap)o; - if(that.backingMap.size() != backingMap.size()) - return false; - return backingMap.object2ObjectEntrySet().containsAll(that.backingMap.object2ObjectEntrySet()); - } - - /** - * We deliberately use a hashcode that will be consistent regardless of underlying map type. - */ - @Override - public int hashCode() { - final ObjectIterator> i = Object2ObjectMaps.fastIterator(backingMap); - int h = 0, n = backingMap.size(); - while (n-- != 0) - h += i.next().hashCode(); - return h; - } - - public static CanonizingStringMap deepCopy(CanonizingStringMap inputMap, Function deepCopier) { - Objects.requireNonNull(deepCopier); - Object2ObjectMap copiedBackingMap; - int size = inputMap.backingMap.size(); - if(size > GROWTH_THRESHOLD) { - copiedBackingMap = new Object2ObjectOpenHashMap<>(size); - } else - copiedBackingMap = new Object2ObjectArrayMap<>(size); - inputMap.backingMap.object2ObjectEntrySet().forEach(entry -> { - if(entry.getKey() != null && entry.getValue() != null) - copiedBackingMap.put(entry.getKey(), deepCopier.apply(entry.getValue())); - }); - return new CanonizingStringMap<>(copiedBackingMap); + public static CanonizingStringMap deepCopy(CanonizingStringMap incomingMap, Function deepCopier) { + CanonizingStringMap newMap = new CanonizingStringMap<>(); + newMap.putAllWithoutInterning(Maps.transformValues(incomingMap, deepCopier)); + return newMap; } } 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 new file mode 100644 index 00000000..fb7e0f71 --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java @@ -0,0 +1,56 @@ +package org.embeddedt.modernfix.forge.config; + +import net.minecraftforge.fml.ModContainer; +import net.minecraftforge.fml.ModList; +import net.minecraftforge.fml.common.ObfuscationReflectionHelper; +import net.minecraftforge.fml.config.ModConfig; +import org.embeddedt.modernfix.ModernFix; +import org.embeddedt.modernfix.core.ModernFixMixinPlugin; + +import java.util.Optional; +import java.util.function.Consumer; + +public class ConfigFixer { + /** + * To prevent mods from crashing if their ModConfigEvent is invoked by Night Config's watch thread and Forge + * at the same time, wrap their config handler so that it only executes the event in serial for that mod. + * + * Should have no noticeable performance impact as config handlers are virtually instant. + */ + public static void replaceConfigHandlers() { + if(!ModernFixMixinPlugin.instance.isOptionEnabled("bugfix.fix_config_crashes.ConfigFixerMixin")) + return; + ModList.get().forEachModContainer((id, container) -> { + try { + Optional> configOpt = ObfuscationReflectionHelper.getPrivateValue(ModContainer.class, container, "configHandler"); + if(configOpt.isPresent()) { + ObfuscationReflectionHelper.setPrivateValue(ModContainer.class, container, Optional.of(new LockingConfigHandler(id, configOpt.get())), "configHandler"); + } + } catch(RuntimeException e) { + ModernFix.LOGGER.error("Error replacing config handler", e); + } + }); + } + + private static class LockingConfigHandler implements Consumer { + private final Consumer actualHandler; + private final String modId; + + LockingConfigHandler(String id, Consumer actualHandler) { + this.modId = id; + this.actualHandler = actualHandler; + } + + @Override + public void accept(ModConfig.ModConfigEvent modConfigEvent) { + synchronized (this) { + this.actualHandler.accept(modConfigEvent); + } + } + + @Override + public String toString() { + return "LockingConfigHandler{id=" + modId + "}"; + } + } +} 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 e96181b2..1c443c2b 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 @@ -24,6 +24,7 @@ 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.packet.PacketHandler; import org.embeddedt.modernfix.forge.registry.ObjectHolderClearer; @@ -44,6 +45,7 @@ public class ModernFixForge { PacketHandler.register(); ModFileScanDataDeduplicator.deduplicate(); ClassLoadHack.loadModClasses(); + ConfigFixer.replaceConfigHandlers(); } @SubscribeEvent diff --git a/scripts/autorelease.sh b/scripts/autorelease.sh index bf338308..508143b5 100755 --- a/scripts/autorelease.sh +++ b/scripts/autorelease.sh @@ -27,6 +27,8 @@ do_release() { echo "we think the current tag is $(git describe --tags --abbrev=0)" echo "the current commit head is $(git rev-parse HEAD)" read -p "new tag name: " tag_name + git tag -a $tag_name -m "$tag_name" + git push --tags gh release create $tag_name --target $1 --title "$tag_name" --notes "" } diff --git a/scripts/propagate.sh b/scripts/propagate.sh index 23b8a6a0..86bc3968 100755 --- a/scripts/propagate.sh +++ b/scripts/propagate.sh @@ -47,6 +47,7 @@ for version in "${all_versions[@]}"; do read -rs -n1 fi if (git add . && git commit -m "Merge $our_version into $version" &>/dev/null); then + echo git push -u origin propagations/$version:$version &>/dev/null else echo -e "\b\b\b\bnothing to merge"