Fix a number of issues with reload listeners (#1240)

* Fix resource reload listeners not being sorted.
Fix missing/broken reload listener keys.
Add a debug log line for missing reload keys
Add some basic tests.

* Undo adding new keys, we can look at this later.

* use AssertionError

* Fix test on server

(cherry picked from commit 67abea83c2)

fabric-resource-loader-v0/build.gradle

@@ -1,2 +1,7 @@
 archivesBaseName = "fabric-resource-loader-v0"
 version = getSubprojectVersion(project, "0.3.5")
+
+dependencies {
+	testmodCompile project(path: ':fabric-lifecycle-events-v1', configuration: 'dev')
+	testmodCompile project(path: ':fabric-api-base', configuration: 'dev')
+}

fabric-resource-loader-v0/src/main/java/net/fabricmc/fabric/impl/resource/loader/ResourceManagerHelperImpl.java

@@ -51,7 +51,7 @@ public class ResourceManagerHelperImpl implements ResourceManagerHelper {
 	private final Set<Identifier> addedListenerIds = new HashSet<>();
 	private final Set<IdentifiableResourceReloadListener> addedListeners = new LinkedHashSet<>();
 
-	public static ResourceManagerHelper get(ResourceType type) {
+	public static ResourceManagerHelperImpl get(ResourceType type) {
 		return registryMap.computeIfAbsent(type, (t) -> new ResourceManagerHelperImpl());
 	}
 
@@ -114,7 +114,7 @@ public class ResourceManagerHelperImpl implements ResourceManagerHelper {
 	}
 
 	public static void sort(ResourceType type, List<ResourceReloadListener> listeners) {
-		ResourceManagerHelperImpl instance = registryMap.get(type);
+		ResourceManagerHelperImpl instance = get(type);
 
 		if (instance != null) {
 			instance.sort(listeners);

fabric-resource-loader-v0/src/main/java/net/fabricmc/fabric/mixin/resource/loader/KeyedResourceReloadListenerMixin.java

@@ -25,7 +25,7 @@ import org.spongepowered.asm.mixin.Mixin;
 import net.minecraft.loot.LootManager;
 import net.minecraft.recipe.RecipeManager;
 import net.minecraft.server.ServerAdvancementLoader;
-import net.minecraft.server.function.CommandFunctionManager;
+import net.minecraft.server.function.FunctionLoader;
 import net.minecraft.tag.TagManagerLoader;
 import net.minecraft.util.Identifier;
 
@@ -34,7 +34,7 @@ import net.fabricmc.fabric.api.resource.ResourceReloadListenerKeys;
 
 @Mixin({
 		/* public */
-		RecipeManager.class, ServerAdvancementLoader.class, CommandFunctionManager.class, LootManager.class, TagManagerLoader.class
+		RecipeManager.class, ServerAdvancementLoader.class, FunctionLoader.class, LootManager.class, TagManagerLoader.class
 		/* private */
 })
 public abstract class KeyedResourceReloadListenerMixin implements IdentifiableResourceReloadListener {
@@ -51,7 +51,7 @@ public abstract class KeyedResourceReloadListenerMixin implements IdentifiableRe
 				this.fabric$id = ResourceReloadListenerKeys.RECIPES;
 			} else if (self instanceof ServerAdvancementLoader) {
 				this.fabric$id = ResourceReloadListenerKeys.ADVANCEMENTS;
-			} else if (self instanceof CommandFunctionManager) {
+			} else if (self instanceof FunctionLoader) {
 				this.fabric$id = ResourceReloadListenerKeys.FUNCTIONS;
 			} else if (self instanceof LootManager) {
 				this.fabric$id = ResourceReloadListenerKeys.LOOT_TABLES;

fabric-resource-loader-v0/src/main/java/net/fabricmc/fabric/mixin/resource/loader/client/KeyedResourceReloadListenerClientMixin.java

@@ -27,7 +27,7 @@ import net.minecraft.client.render.block.BlockRenderManager;
 import net.minecraft.client.render.item.ItemRenderer;
 import net.minecraft.client.render.model.BakedModelManager;
 import net.minecraft.client.resource.language.LanguageManager;
-import net.minecraft.client.sound.SoundLoader;
+import net.minecraft.client.sound.SoundManager;
 import net.minecraft.client.texture.TextureManager;
 import net.minecraft.util.Identifier;
 
@@ -36,7 +36,7 @@ import net.fabricmc.fabric.api.resource.ResourceReloadListenerKeys;
 
 @Mixin({
 		/* public */
-		SoundLoader.class, BakedModelManager.class, LanguageManager.class, TextureManager.class,
+		SoundManager.class, BakedModelManager.class, LanguageManager.class, TextureManager.class,
 		/* private */
 		WorldRenderer.class, BlockRenderManager.class, ItemRenderer.class
 })
@@ -50,7 +50,7 @@ public abstract class KeyedResourceReloadListenerClientMixin implements Identifi
 		if (this.fabric$id == null) {
 			Object self = this;
 
-			if (self instanceof SoundLoader) {
+			if (self instanceof SoundManager) {
 				this.fabric$id = ResourceReloadListenerKeys.SOUNDS;
 			} else if (self instanceof BakedModelManager) {
 				this.fabric$id = ResourceReloadListenerKeys.MODELS;

fabric-resource-loader-v0/src/testmod/java/net/fabricmc/fabric/test/resource/loader/ResourceReloadListenerTestMod.java

@@ -0,0 +1,120 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.test.resource.loader;
+
+import java.util.Collection;
+import java.util.Collections;
+
+import net.minecraft.resource.ResourceManager;
+import net.minecraft.resource.ResourceType;
+import net.minecraft.util.Identifier;
+
+import net.fabricmc.api.EnvType;
+import net.fabricmc.api.ModInitializer;
+import net.fabricmc.loader.api.FabricLoader;
+import net.fabricmc.fabric.api.event.lifecycle.v1.ServerTickEvents;
+import net.fabricmc.fabric.api.resource.ResourceManagerHelper;
+import net.fabricmc.fabric.api.resource.SimpleSynchronousResourceReloadListener;
+
+public class ResourceReloadListenerTestMod implements ModInitializer {
+	public static final String MODID = "fabric-resource-loader-v0-testmod";
+
+	private static boolean clientResources = false;
+	private static boolean serverResources = false;
+
+	@Override
+	public void onInitialize() {
+		setupClientReloadListeners();
+		setupServerReloadListeners();
+
+		ServerTickEvents.START_WORLD_TICK.register(world -> {
+			if (!clientResources && FabricLoader.getInstance().getEnvironmentType() == EnvType.CLIENT) {
+				throw new AssertionError("Client reload listener was not called.");
+			}
+
+			if (!serverResources) {
+				throw new AssertionError("Server reload listener was not called.");
+			}
+		});
+	}
+
+	private void setupClientReloadListeners() {
+		ResourceManagerHelper.get(ResourceType.CLIENT_RESOURCES).registerReloadListener(new SimpleSynchronousResourceReloadListener() {
+			@Override
+			public Identifier getFabricId() {
+				return new Identifier(MODID, "client_second");
+			}
+
+			@Override
+			public void apply(ResourceManager manager) {
+				if (!clientResources) {
+					throw new AssertionError("Second reload listener was called before the first!");
+				}
+			}
+
+			@Override
+			public Collection<Identifier> getFabricDependencies() {
+				return Collections.singletonList(new Identifier(MODID, "client_first"));
+			}
+		});
+
+		ResourceManagerHelper.get(ResourceType.CLIENT_RESOURCES).registerReloadListener(new SimpleSynchronousResourceReloadListener() {
+			@Override
+			public Identifier getFabricId() {
+				return new Identifier(MODID, "client_first");
+			}
+
+			@Override
+			public void apply(ResourceManager manager) {
+				clientResources = true;
+			}
+		});
+	}
+
+	private void setupServerReloadListeners() {
+		ResourceManagerHelper.get(ResourceType.SERVER_DATA).registerReloadListener(new SimpleSynchronousResourceReloadListener() {
+			@Override
+			public Identifier getFabricId() {
+				return new Identifier(MODID, "server_second");
+			}
+
+			@Override
+			public void apply(ResourceManager manager) {
+				if (!serverResources) {
+					throw new AssertionError("Second reload listener was called before the first!");
+				}
+			}
+
+			@Override
+			public Collection<Identifier> getFabricDependencies() {
+				return Collections.singletonList(new Identifier(MODID, "server_first"));
+			}
+		});
+
+		ResourceManagerHelper.get(ResourceType.SERVER_DATA).registerReloadListener(new SimpleSynchronousResourceReloadListener() {
+			@Override
+			public Identifier getFabricId() {
+				return new Identifier(MODID, "server_first");
+			}
+
+			@Override
+			public void apply(ResourceManager manager) {
+				serverResources = true;
+			}
+		});
+	}
+}

fabric-resource-loader-v0/src/testmod/resources/fabric.mod.json

@@ -10,7 +10,8 @@
   },
   "entrypoints": {
     "main": [
-      "net.fabricmc.fabric.test.resource.loader.BuiltinResourcePackTestMod"
+      "net.fabricmc.fabric.test.resource.loader.BuiltinResourcePackTestMod",
+      "net.fabricmc.fabric.test.resource.loader.ResourceReloadListenerTestMod"
     ]
   }
 }