From fa939a890cc80624bf92e813a64c0142fea883ad Mon Sep 17 00:00:00 2001 From: Vincent Lee Date: Fri, 25 Jan 2019 15:14:49 -0600 Subject: [PATCH] [1.13] Make Caps, TESR, Entity renderers, and keybinds thread safe to call during parallel init (#5359) --- .../common/capabilities/CapabilityInject.java | 14 +++++--- .../capabilities/CapabilityManager.java | 31 ++++++++++------ .../fml/client/registry/ClientRegistry.java | 36 ++++++++----------- .../client/registry/RenderingRegistry.java | 8 ++--- 4 files changed, 49 insertions(+), 40 deletions(-) diff --git a/src/main/java/net/minecraftforge/common/capabilities/CapabilityInject.java b/src/main/java/net/minecraftforge/common/capabilities/CapabilityInject.java index 1035b7874..ba885ddab 100644 --- a/src/main/java/net/minecraftforge/common/capabilities/CapabilityInject.java +++ b/src/main/java/net/minecraftforge/common/capabilities/CapabilityInject.java @@ -28,17 +28,23 @@ import java.lang.annotation.*; * of 'Capability' * * Example: - * @CapabilityInject(IExampleCapability.class) - * private static final Capability TEST_CAP = null; + * 
+ * {@literal @}CapabilityInject(IExampleCapability.class)
+ * private static final Capability TEST_CAP = null;
+ *
* * When placed on a METHOD, the method will be invoked once the * capability is registered. This allows you to have a 'enable features' * callback. It MUST have one parameter of type 'Capability; * * Example: - * @CapabilityInject(IExampleCapability.class) - * private static void capRegistered(Capability cap) {} + * 
+ * {@literal @}CapabilityInject(IExampleCapability.class)
+ * private static void capRegistered(Capability cap) {}
+ *
* + * Warning: Capability injections are run in the thread that the capablity is registered. + * Due to parallel mod loading, this can potentially be off of the main thread. */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.METHOD}) diff --git a/src/main/java/net/minecraftforge/common/capabilities/CapabilityManager.java b/src/main/java/net/minecraftforge/common/capabilities/CapabilityManager.java index fedd4a494..9de09e8f4 100644 --- a/src/main/java/net/minecraftforge/common/capabilities/CapabilityManager.java +++ b/src/main/java/net/minecraftforge/common/capabilities/CapabilityManager.java @@ -27,6 +27,7 @@ import java.util.Collection; import java.util.Collections; import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.concurrent.Callable; @@ -50,6 +51,7 @@ public enum CapabilityManager * Registers a capability to be consumed by others. * APIs who define the capability should call this. * To retrieve the Capability instance, use the @CapabilityInject annotation. + * This method is safe to call during parallel mod loading. * * @param type The Interface to be registered * @param storage A default implementation of the storage handler. @@ -61,20 +63,25 @@ public enum CapabilityManager Objects.requireNonNull(storage,"Attempted to register a capability with no storage implementation"); Objects.requireNonNull(factory,"Attempted to register a capability with no default implementation factory"); String realName = type.getName().intern(); - if (providers.containsKey(realName)) { - LOGGER.error(CAPABILITIES, "Cannot register capability implementation multiple times : {}", realName); - throw new IllegalArgumentException("Cannot register a capability implementation multiple times : "+ realName); - } + Capability cap; - Capability cap = new Capability<>(realName, storage, factory); - providers.put(realName, cap); + synchronized (providers) + { + if (providers.containsKey(realName)) { + LOGGER.error(CAPABILITIES, "Cannot register capability implementation multiple times : {}", realName); + throw new IllegalArgumentException("Cannot register a capability implementation multiple times : "+ realName); + } + + cap = new Capability<>(realName, storage, factory); + providers.put(realName, cap); + } callbacks.getOrDefault(realName, Collections.emptyList()).forEach(func -> func.apply(cap)); } // INTERNAL - private IdentityHashMap> providers = new IdentityHashMap<>(); - private IdentityHashMap, Object>>> callbacks = new IdentityHashMap<>(); + private final IdentityHashMap> providers = new IdentityHashMap<>(); + private volatile IdentityHashMap, Object>>> callbacks; public void injectCapabilities(List data) { final List capabilities = data.stream() @@ -82,10 +89,12 @@ public enum CapabilityManager .flatMap(Collection::stream) .filter(a -> CAP_INJECT.equals(a.getAnnotationType())) .collect(Collectors.toList()); - capabilities.forEach(this::attachCapabilityToMethod); + final IdentityHashMap, Object>>> m = new IdentityHashMap<>(); + capabilities.forEach(entry -> attachCapabilityToMethod(m, entry)); + callbacks = m; } - private void attachCapabilityToMethod(ModFileScanData.AnnotationData entry) + private static void attachCapabilityToMethod(Map, Object>>> cbs, ModFileScanData.AnnotationData entry) { final String targetClass = entry.getClassType().getClassName(); final String targetName = entry.getMemberName(); @@ -97,7 +106,7 @@ public enum CapabilityManager } final String capabilityName = type.getInternalName().replace('/', '.').intern(); - List, Object>> list = callbacks.computeIfAbsent(capabilityName, k -> new ArrayList<>()); + List, Object>> list = cbs.computeIfAbsent(capabilityName, k -> new ArrayList<>()); if (entry.getMemberName().indexOf('(') > 0) { diff --git a/src/main/java/net/minecraftforge/fml/client/registry/ClientRegistry.java b/src/main/java/net/minecraftforge/fml/client/registry/ClientRegistry.java index b3a25d627..0dec5c31d 100644 --- a/src/main/java/net/minecraftforge/fml/client/registry/ClientRegistry.java +++ b/src/main/java/net/minecraftforge/fml/client/registry/ClientRegistry.java @@ -19,7 +19,6 @@ package net.minecraftforge.fml.client.registry; -import com.google.common.collect.Maps; import net.minecraft.client.renderer.tileentity.TileEntityRenderer; import net.minecraft.entity.Entity; import net.minecraft.util.ResourceLocation; @@ -29,35 +28,31 @@ import net.minecraft.client.Minecraft; import net.minecraft.client.renderer.tileentity.TileEntityRendererDispatcher; import net.minecraft.client.settings.KeyBinding; import net.minecraft.tileentity.TileEntity; -import net.minecraftforge.fml.common.registry.GameRegistry; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; public class ClientRegistry { - private static Map, ResourceLocation> entityShaderMap = Maps.newHashMap(); + private static Map, ResourceLocation> entityShaderMap = new ConcurrentHashMap<>(); /** - * - * Utility method for registering a tile entity and it's renderer at once - generally you should register them separately - * - * @param tileEntityClass - * @param id - * @param specialRenderer - * / TODO GameRegistry - public static void registerTileEntity(Class tileEntityClass, String id, TileEntityRenderer specialRenderer) - { - GameRegistry.registerTileEntity(tileEntityClass, id); - bindTileEntitySpecialRenderer(tileEntityClass, specialRenderer); - } -*/ - public static void bindTileEntitySpecialRenderer(Class tileEntityClass, TileEntityRenderer specialRenderer) + * Registers a Tile Entity renderer. + * Call this during {@link net.minecraftforge.fml.event.lifecycle.FMLClientSetupEvent}. + * This method is safe to call during parallel mod loading. + */ + public static synchronized void bindTileEntitySpecialRenderer(Class tileEntityClass, TileEntityRenderer specialRenderer) { TileEntityRendererDispatcher.instance.renderers.put(tileEntityClass, specialRenderer); specialRenderer.setRendererDispatcher(TileEntityRendererDispatcher.instance); } - public static void registerKeyBinding(KeyBinding key) + /** + * Registers a KeyBinding. + * Call this during {@link net.minecraftforge.fml.event.lifecycle.FMLClientSetupEvent}. + * This method is safe to call during parallel mod loading. + */ + public static synchronized void registerKeyBinding(KeyBinding key) { Minecraft.getInstance().gameSettings.keyBindings = ArrayUtils.add(Minecraft.getInstance().gameSettings.keyBindings, key); } @@ -65,9 +60,8 @@ public class ClientRegistry /** * Register a shader for an entity. This shader gets activated when a spectator begins spectating an entity. * Vanilla examples of this are the green effect for creepers and the invert effect for endermen. - * - * @param entityClass - * @param shader + * Call this during {@link net.minecraftforge.fml.event.lifecycle.FMLClientSetupEvent}. + * This method is safe to call during parallel mod loading. */ public static void registerEntityShader(Class entityClass, ResourceLocation shader) { diff --git a/src/main/java/net/minecraftforge/fml/client/registry/RenderingRegistry.java b/src/main/java/net/minecraftforge/fml/client/registry/RenderingRegistry.java index 34bdf5ada..fd5a98c4f 100644 --- a/src/main/java/net/minecraftforge/fml/client/registry/RenderingRegistry.java +++ b/src/main/java/net/minecraftforge/fml/client/registry/RenderingRegistry.java @@ -20,23 +20,23 @@ package net.minecraftforge.fml.client.registry; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import net.minecraft.client.renderer.entity.Render; import net.minecraft.client.renderer.entity.RenderManager; import net.minecraft.entity.Entity; -import com.google.common.collect.Maps; - public class RenderingRegistry { private static final RenderingRegistry INSTANCE = new RenderingRegistry(); - private Map, IRenderFactory> entityRenderers = Maps.newHashMap(); + private final Map, IRenderFactory> entityRenderers = new ConcurrentHashMap<>(); /** * Register an entity rendering handler. This will, after mod initialization, be inserted into the main * render map for entities. - * Call this during Preinitialization phase. + * Call this during {@link net.minecraftforge.fml.event.lifecycle.FMLClientSetupEvent}. + * This method is safe to call during parallel mod loading. */ public static void registerEntityRenderingHandler(Class entityClass, IRenderFactory renderFactory) {