From 1c4806cd7668445a3ab2da5799b5b3885492aaf0 Mon Sep 17 00:00:00 2001 From: Cat Core <34719527+thecatcore@users.noreply.github.com> Date: Fri, 15 Mar 2024 12:03:47 +0100 Subject: [PATCH] Refactor Registry module code. (#166) * Refactor Registry module code. * Fix some logic flaws in registry remapping --- .../mixin/registry/sync/class_2929Mixin.java | 3 +- .../mixin/registry/sync/class_2929Mixin.java | 3 +- .../mixin/registry/sync/class_2929Mixin.java | 3 +- .../impl/registry/RegistryHelperImpl.java | 120 ++++++++---------- .../sync/remappers/RegistryRemapper.java | 97 ++++++++------ .../mixin/registry/sync/IdListMixin.java | 1 + 6 files changed, 114 insertions(+), 113 deletions(-) diff --git a/legacy-fabric-registry-sync-api-v1/1.10.2/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java b/legacy-fabric-registry-sync-api-v1/1.10.2/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java index 230a0d085..408e79a2a 100644 --- a/legacy-fabric-registry-sync-api-v1/1.10.2/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java +++ b/legacy-fabric-registry-sync-api-v1/1.10.2/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java @@ -20,6 +20,7 @@ import java.util.IdentityHashMap; import java.util.List; +import com.google.common.collect.Lists; import org.jetbrains.annotations.Nullable; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; @@ -63,7 +64,7 @@ public IdentityHashMap getIdMap(SimpleRegistryCompat simpl @Override public List getList() { - return null; + return Lists.newArrayList(this.field_14375); } @Override diff --git a/legacy-fabric-registry-sync-api-v1/1.12.2/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java b/legacy-fabric-registry-sync-api-v1/1.12.2/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java index 230a0d085..408e79a2a 100644 --- a/legacy-fabric-registry-sync-api-v1/1.12.2/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java +++ b/legacy-fabric-registry-sync-api-v1/1.12.2/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java @@ -20,6 +20,7 @@ import java.util.IdentityHashMap; import java.util.List; +import com.google.common.collect.Lists; import org.jetbrains.annotations.Nullable; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; @@ -63,7 +64,7 @@ public IdentityHashMap getIdMap(SimpleRegistryCompat simpl @Override public List getList() { - return null; + return Lists.newArrayList(this.field_14375); } @Override diff --git a/legacy-fabric-registry-sync-api-v1/1.9.4/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java b/legacy-fabric-registry-sync-api-v1/1.9.4/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java index 230a0d085..408e79a2a 100644 --- a/legacy-fabric-registry-sync-api-v1/1.9.4/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java +++ b/legacy-fabric-registry-sync-api-v1/1.9.4/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/class_2929Mixin.java @@ -20,6 +20,7 @@ import java.util.IdentityHashMap; import java.util.List; +import com.google.common.collect.Lists; import org.jetbrains.annotations.Nullable; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; @@ -63,7 +64,7 @@ public IdentityHashMap getIdMap(SimpleRegistryCompat simpl @Override public List getList() { - return null; + return Lists.newArrayList(this.field_14375); } @Override diff --git a/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/impl/registry/RegistryHelperImpl.java b/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/impl/registry/RegistryHelperImpl.java index 7429ec5fe..4ad5ae3c0 100644 --- a/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/impl/registry/RegistryHelperImpl.java +++ b/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/impl/registry/RegistryHelperImpl.java @@ -18,6 +18,7 @@ package net.legacyfabric.fabric.impl.registry; import java.util.IdentityHashMap; +import java.util.function.Consumer; import java.util.function.Supplier; import com.google.common.collect.BiMap; @@ -58,11 +59,36 @@ public class RegistryHelperImpl { public static final boolean bootstrap = VersionUtils.matches(">1.8.9"); public static RegistriesGetter registriesGetter = null; + public static int register(T object, Identifier id, Identifier registryId) { + RegistryRemapper remapper = RegistryRemapper.getRegistryRemapper(registryId); + int rawId = nextId(remapper.getRegistry()); + remapper.register(rawId, id, object); + + return rawId; + } + + public static T register(RegistryHelper.EntryCreator entryCreator, Identifier id, Identifier registryId) { + return register(entryCreator, id, registryId, instance -> { }); + } + + public static T register(RegistryHelper.EntryCreator entryCreator, Identifier id, Identifier registryId, Consumer beforeRegistration) { + RegistryRemapper remapper = RegistryRemapper.getRegistryRemapper(registryId); + SimpleRegistryCompat registry = remapper.getRegistry(); + int rawId = nextId(registry); + + if (registry instanceof ArrayAndMapBasedRegistry) ((ArrayAndMapBasedRegistry) registry).updateArrayLength(rawId); + + T instance = entryCreator.create(rawId); + beforeRegistration.accept(instance); + + remapper.register(rawId, id, instance); + + return instance; + } + public static Block registerBlock(Block block, Identifier id) { block.setTranslationKey(formatTranslationKey(id)); - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.BLOCKS); - int rawId = nextId(registryRemapper.getRegistry()); - registryRemapper.register(rawId, id, block); + int rawId = register(block, id, RegistryIds.BLOCKS); if (hasFlatteningBegun) { for (BlockState blockState : block.getStateManager().getBlockStates()) { @@ -76,9 +102,7 @@ public static Block registerBlock(Block block, Identifier id) { public static Item registerItem(Item item, Identifier id) { item.setTranslationKey(formatTranslationKey(id)); - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.ITEMS); - int rawId = nextId(registryRemapper.getRegistry()); - registryRemapper.register(rawId, id, item); + register(item, id, RegistryIds.ITEMS); if (hasFlatteningBegun) { if (item instanceof BlockItem) { @@ -90,83 +114,47 @@ public static Item registerItem(Item item, Identifier id) { } public static Class registerBlockEntityType(Class blockEntityClass, Identifier id) { - RegistryRemapper> registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.BLOCK_ENTITY_TYPES); - int rawId = nextId(registryRemapper.getRegistry()); - registryRemapper.register(rawId, id, blockEntityClass); + register(blockEntityClass, id, RegistryIds.BLOCK_ENTITY_TYPES); return blockEntityClass; } public static Class registerEntityType(Class entityTypeClass, Identifier id) { - RegistryRemapper> registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.ENTITY_TYPES); - int rawId = nextId(registryRemapper.getRegistry()); - registryRemapper.register(rawId, id, entityTypeClass); + register(entityTypeClass, id, RegistryIds.ENTITY_TYPES); return entityTypeClass; } public static StatusEffect registerStatusEffect(StatusEffect statusEffect, Identifier id) { statusEffect.setTranslationKey(formatTranslationKey(id)); - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.STATUS_EFFECTS); - int rawId = nextId(registryRemapper.getRegistry()); - registryRemapper.register(rawId, id, statusEffect); + register(statusEffect, id, RegistryIds.STATUS_EFFECTS); return statusEffect; } public static StatusEffect registerStatusEffect(RegistryHelper.EntryCreator statusEffectCreator, Identifier id) { - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.STATUS_EFFECTS); - int rawId = nextId(registryRemapper.getRegistry()); - - ((ArrayAndMapBasedRegistry) registryRemapper.getRegistry()).updateArrayLength(rawId); - - StatusEffect statusEffect = statusEffectCreator.create(rawId); - statusEffect.setTranslationKey(formatTranslationKey(id)); - registryRemapper.register(rawId, id, statusEffect); - - return statusEffect; + return register(statusEffectCreator, id, RegistryIds.STATUS_EFFECTS, effect -> effect.setTranslationKey(formatTranslationKey(id))); } public static Enchantment registerEnchantment(Enchantment enchantment, Identifier id) { enchantment.setName(formatTranslationKey(id)); - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.ENCHANTMENTS); - int rawId = nextId(registryRemapper.getRegistry()); - registryRemapper.register(rawId, id, enchantment); + register(enchantment, id, RegistryIds.ENCHANTMENTS); return enchantment; } public static Enchantment registerEnchantment(RegistryHelper.EntryCreator enchantmentCreator, Identifier id) { - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.ENCHANTMENTS); - int rawId = nextId(registryRemapper.getRegistry()); - - ((ArrayAndMapBasedRegistry) registryRemapper.getRegistry()).updateArrayLength(rawId); - - Enchantment enchantment = enchantmentCreator.create(rawId); - enchantment.setName(formatTranslationKey(id)); - registryRemapper.register(rawId, id, enchantment); - - return enchantment; + return register(enchantmentCreator, id, RegistryIds.ENCHANTMENTS, enchantment -> enchantment.setName(formatTranslationKey(id))); } public static Biome registerBiome(Biome biome, Identifier id) { - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.BIOMES); - int rawId = nextId(registryRemapper.getRegistry()); - registryRemapper.register(rawId, id, biome); + register(biome, id, RegistryIds.BIOMES); return biome; } public static Biome registerBiome(RegistryHelper.EntryCreator biomeCreator, Identifier id) { - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.BIOMES); - int rawId = nextId(registryRemapper.getRegistry()); - - ((ArrayAndMapBasedRegistry) registryRemapper.getRegistry()).updateArrayLength(rawId); - - Biome biome = biomeCreator.create(rawId); - registryRemapper.register(rawId, id, biome); - - return biome; + return register(biomeCreator, id, RegistryIds.BIOMES); } public static BiomePair registerBiomeWithMutatedVariant( @@ -176,7 +164,7 @@ public static BiomePair registerBiomeWithMutatedVariant( RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(RegistryIds.BIOMES); NumericalIdPair rawIds = nextIds(registryRemapper.getRegistry(), 128); - ((ArrayAndMapBasedRegistry) registryRemapper.getRegistry()).updateArrayLength(rawIds.getSecondary()); + ((ArrayAndMapBasedRegistry) registryRemapper.getRegistry()).updateArrayLength(rawIds.getSecondary()); Biome parentBiome = parentBiomeCreator.create(rawIds.getMain()); registryRemapper.register(rawIds.getMain(), parentId, parentBiome); @@ -216,25 +204,11 @@ private static String formatTranslationKey(Identifier key) { } public static int nextId(SimpleRegistryCompat registry) { - return nextId(registry, 0); + return nextId(registry.getIds(), registry); } public static int nextId(SimpleRegistryCompat registry, int minId) { - int id = minId; - - RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(registry); - - if (registryRemapper == null) { - registryRemapper = RegistryRemapper.DEFAULT_CLIENT_INSTANCE; - } - - while (getIdList(registry).fromInt(id) != null - || id < registryRemapper.getMinId() - ) { - id++; - } - - return id; + return nextId(registry.getIds(), registry, minId); } public static NumericalIdPair nextIds(SimpleRegistryCompat registry, int offset) { @@ -255,12 +229,20 @@ public static NumericalIdPair nextIds(SimpleRegistryCompat registry, int o return new NumericalIdPair(id, id + offset); } + public static int nextId(IdListCompat idList, SimpleRegistryCompat registry, int minId) { + return nextId(idList, registry, HashBiMap.create(), minId); + } + public static int nextId(IdListCompat idList, SimpleRegistryCompat registry) { - return nextId(idList, registry, HashBiMap.create()); + return nextId(idList, registry, 0); } public static int nextId(IdListCompat idList, SimpleRegistryCompat registry, BiMap missingMap) { - int id = 0; + return nextId(idList, registry, missingMap, 0); + } + + public static int nextId(IdListCompat idList, SimpleRegistryCompat registry, BiMap missingMap, int minId) { + int id = minId; RegistryRemapper registryRemapper = RegistryRemapper.getRegistryRemapper(registry); diff --git a/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/impl/registry/sync/remappers/RegistryRemapper.java b/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/impl/registry/sync/remappers/RegistryRemapper.java index a66f11cab..cd7fab552 100644 --- a/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/impl/registry/sync/remappers/RegistryRemapper.java +++ b/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/impl/registry/sync/remappers/RegistryRemapper.java @@ -20,6 +20,7 @@ import java.util.HashMap; import java.util.Map; import java.util.function.IntSupplier; +import java.util.stream.Collectors; import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; @@ -112,14 +113,7 @@ public void readNbt(NbtCompound tag) { } } - // Type erasure, ily - public void remap() { - LOGGER.info("Remapping registry %s", this.registryId.toString()); - - if (this.entryDump == null || this.entryDump.isEmpty()) { - this.dump(); - } - + private IdListCompat getExistingFromDump() { IdListCompat newList = this.registry.createIdList(); this.entryDump.forEach((id, rawId) -> { @@ -134,43 +128,26 @@ public void remap() { } }); - IntSupplier currentSize = () -> RegistryHelperImpl.getIdMap(newList, this.registry).size(); - IntSupplier previousSize = () -> RegistryHelperImpl.getObjects(this.registry).size(); - - if (currentSize.getAsInt() > previousSize.getAsInt()) { - if (this.missingMap.size() == 0) { - throw new IllegalStateException("Registry size increased from " + previousSize.getAsInt() + " to " + currentSize.getAsInt() + " after remapping! This is not possible!"); - } - } else if (currentSize.getAsInt() < previousSize.getAsInt()) { - LOGGER.info("Adding " + (previousSize.getAsInt() - currentSize.getAsInt()) + " missing entries to registry"); - - RegistryHelperImpl.getObjects(this.registry).keySet().stream().filter(obj -> newList.getInt(obj) == -1).forEach(missing -> { - int id = RegistryHelperImpl.nextId(this.registry); - - while (newList.fromInt(id) != null || this.missingMap.containsValue(id)) { - id = RegistryHelperImpl.nextId(newList, this.registry, this.missingMap); - - V currentBlock = RegistryHelperImpl.getIdList(this.registry).fromInt(id); + return newList; + } - if (currentBlock == null) { - break; - } - } + private void addNewEntries(IdListCompat newList, IntSupplier currentSize, IntSupplier previousSize) { + LOGGER.info("Adding " + (previousSize.getAsInt() - currentSize.getAsInt()) + " missing entries to registry"); - if (newList.getInt(missing) == -1) { - newList.setValue(missing, id); - } else { - id = newList.getInt(missing); - } + RegistryHelperImpl.getObjects(this.registry) + .keySet().stream() + .filter(obj -> newList.getInt(obj) == -1) + .collect(Collectors.toList()) + .forEach(missingValue -> { + int newId = RegistryHelperImpl.nextId(newList, this.registry, this.missingMap); - LOGGER.info("Adding %s %s with numerical id %d to registry", this.type, this.registry.getKey(missing), id); - }); - } + newList.setValue(missingValue, newId); - if (currentSize.getAsInt() != previousSize.getAsInt() && this.missingMap.size() == 0) { - throw new IllegalStateException("An error occured during remapping"); - } + LOGGER.info("Adding %s %s with numerical id %d to registry", this.type, this.registry.getKey(missingValue), newId); + }); + } + private void invokeRemapListeners(IdListCompat newList) { for (V value : newList) { int oldId = this.registry.getIds().getInt(value); int newId = newList.getInt(value); @@ -180,12 +157,50 @@ public void remap() { this.registry.getEventHolder().getRemapEvent().invoker().onEntryAdded(oldId, newId, new Identifier(this.registry.getKey(value)), value); } } + } + private void updateRegistry(IdListCompat newList) { this.registry.setIds(newList); if (this.registry instanceof ArrayBasedRegistry) { - ((ArrayBasedRegistry) this.registry).syncArrayWithIdList(); + ((ArrayBasedRegistry) this.registry).syncArrayWithIdList(); } + } + + private IntSupplier normalizeRegistryEntryList(IdListCompat newList) { + IntSupplier currentSize = () -> RegistryHelperImpl.getIdMap(newList, this.registry).size(); + IntSupplier previousSize = () -> RegistryHelperImpl.getObjects(this.registry).size(); + + if (currentSize.getAsInt() > previousSize.getAsInt()) { + if (this.missingMap.isEmpty()) { + throw new IllegalStateException("Registry size increased from " + previousSize.getAsInt() + " to " + currentSize.getAsInt() + " after remapping! This is not possible!"); + } + } else if (currentSize.getAsInt() < previousSize.getAsInt()) { + addNewEntries(newList, currentSize, previousSize); + } + + if (currentSize.getAsInt() != previousSize.getAsInt() && this.missingMap.isEmpty()) { + throw new IllegalStateException("An error occured during remapping"); + } + + return previousSize; + } + + // Type erasure, ily + public void remap() { + LOGGER.info("Remapping registry %s", this.registryId.toString()); + + if (this.entryDump == null || this.entryDump.isEmpty()) { + this.dump(); + } + + IdListCompat newList = getExistingFromDump(); + + IntSupplier previousSize = normalizeRegistryEntryList(newList); + + invokeRemapListeners(newList); + + updateRegistry(newList); this.dump(); LOGGER.info("Remapped " + previousSize.getAsInt() + " entries"); diff --git a/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/IdListMixin.java b/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/IdListMixin.java index 48f7480ca..d4b23e6a3 100644 --- a/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/IdListMixin.java +++ b/legacy-fabric-registry-sync-api-v1/common/src/main/java/net/legacyfabric/fabric/mixin/registry/sync/IdListMixin.java @@ -48,6 +48,7 @@ public abstract class IdListMixin implements IdListCompat { @Shadow public abstract int getId(V value); + @Override public IdentityHashMap getIdMap(SimpleRegistryCompat simpleRegistry) { return this.idMap; }