In prod, with fabric-content-registries installed, a data pack reload listener doesn't get invoked
sargunv opened this issue · 16 comments
How to reproduce the issue:
- In prod, first install ONLY level-up-hp and open a single player world.
- search the log for
LevellingConfig; it's there, loaded fine
- search the log for
- Now, add the fabric-api jar and open a single player world again
- search the log for
LevellingConfig; it's not there. My resource reload listener was never invoked.
- search the log for
What I've observed so far is that when only the fabric-api modules I depend on are installed, it works just fine, but when ALL fabric-api modules are installed, my SimpleSynchronousResourceReloadListener never runs. I confirmed this by removing the fabric-api modules I DON'T use from the fabric-api jar, and then repeating step 2 above. The results were the same as step 1. I'm still working on determining the exact module and root cause. See https://gitlab.com/sargunv-mc-mods/level-up-hp/issues/20 for details.
Confirmed the culprit is fabric-content-registries. Installing all of fabric-api EXCEPT that module works fine. Installing ONLY level-up-hp (including JIJ) + fabric-content-registries breaks the reload listener.
Updated issue title.
Specifically, this seems to break the apply phase in SimpleResourceReloadListener. The load method gets called as normal, but the apply method never does. My workaround has been to make my data thread safe and apply it during load, but that's very suboptimal.
Confirmed this also affects I Am Very Smart, which is a much more minimal reproduce case. This leads me to believe this affects every mod that relies on loading custom data-pack data.
Poked around by deleting various verticals in fabric-content-registries to see what happens, and I narrowed it down to one of the flammability related APIs in the module. When I delete FlammableBlockRegistry (and Impl), FireBlockHooks, and MixinFireBlock, the bug disappears. When I load up a reduced fabric-content-registries with ONLY those classes, the bug happens. Will investigate more when I get a chance.
Tested with I Am Very Smart.
I believe I've found the cause:
sargunv-desktop: ~/.local/share/multimc/instances/fabric/.minecraft/mods
→ javap -p FlammableBlockRegistryImpl.class
Compiled from "FlammableBlockRegistryImpl.java"
public class net.fabricmc.fabric.impl.registry.FlammableBlockRegistryImpl implements net.fabricmc.fabric.api.registry.FlammableBlockRegistry,net.fabricmc.fabric.api.resource.SimpleSynchronousResourceReloadListener {
private static final net.fabricmc.fabric.api.registry.FlammableBlockRegistry$Entry REMOVED;
private static final java.util.Map<net.minecraft.class_2248, net.fabricmc.fabric.impl.registry.FlammableBlockRegistryImpl> REGISTRIES;
private static final java.util.Collection<net.minecraft.class_2960> RELOAD_DEPS;
private static int idCounter;
private final java.util.Map<net.minecraft.class_2248, net.fabricmc.fabric.api.registry.FlammableBlockRegistry$Entry> registeredEntriesBlock;
private final java.util.Map<net.minecraft.class_3494<net.minecraft.class_2248>, net.fabricmc.fabric.api.registry.FlammableBlockRegistry$Entry> registeredEntriesTag;
private final java.util.Map<net.minecraft.class_2248, net.fabricmc.fabric.api.registry.FlammableBlockRegistry$Entry> computedEntries;
private final net.minecraft.class_2960 id;
private final net.minecraft.class_2248 key;
private boolean tagsPresent;
private net.fabricmc.fabric.impl.registry.FlammableBlockRegistryImpl(net.minecraft.class_2248);
public void apply(net.minecraft.class_3300);
private void reload();
public net.fabricmc.fabric.api.registry.FlammableBlockRegistry$Entry get(net.minecraft.class_2248);
public net.fabricmc.fabric.api.registry.FlammableBlockRegistry$Entry getFabric(net.minecraft.class_2248);
public void add(net.minecraft.class_2248, net.fabricmc.fabric.api.registry.FlammableBlockRegistry$Entry);
public void add(net.minecraft.class_3494<net.minecraft.class_2248>, net.fabricmc.fabric.api.registry.FlammableBlockRegistry$Entry);
public void remove(net.minecraft.class_2248);
public void remove(net.minecraft.class_3494<net.minecraft.class_2248>);
public void clear(net.minecraft.class_2248);
public void clear(net.minecraft.class_3494<net.minecraft.class_2248>);
public static net.fabricmc.fabric.impl.registry.FlammableBlockRegistryImpl getInstance(net.minecraft.class_2248);
public net.minecraft.class_2960 getFabricId();
public java.util.Collection<net.minecraft.class_2960> getFabricDependencies();
public void add(net.minecraft.class_3494, java.lang.Object);
public void add(net.minecraft.class_2248, java.lang.Object);
public java.lang.Object get(net.minecraft.class_2248);
static {};
}
See anything weird there? Specifically this line:
public void apply(net.minecraft.class_3300);
For some reason, the apply method in Fabric's FlammableBlockRegistryImpl didn't get mapped back to its intermediary name method_14491 when the fabric api jar was built.
Compare that to my reload listener:
sargunv-desktop: ~/.local/share/multimc/instances/fabric/.minecraft/mods
→ javap -p ReloadListener.class
Compiled from "ReloadListener.java"
public class me.sargunvohra.mcmods.iamverysmart.config.ReloadListener implements net.fabricmc.fabric.api.resource.SimpleSynchronousResourceReloadListener {
public static final me.sargunvohra.mcmods.iamverysmart.config.ReloadListener INSTANCE;
private com.google.gson.Gson gson;
private me.sargunvohra.mcmods.iamverysmart.match.ComposedMatcher matcher;
private me.sargunvohra.mcmods.iamverysmart.config.ReloadListener();
public net.minecraft.class_2960 getFabricId();
private me.sargunvohra.mcmods.iamverysmart.match.SingleMatcher buildMatcher(net.minecraft.class_3300, java.lang.String);
public void method_14491(net.minecraft.class_3300);
public java.util.concurrent.CompletableFuture<java.lang.Void> reload(net.minecraft.class_3302$class_4045, net.minecraft.class_3300, net.minecraft.class_3695, net.minecraft.class_3695, java.util.concurrent.Executor, java.util.concurrent.Executor);
public me.sargunvohra.mcmods.iamverysmart.match.ComposedMatcher getMatcher();
private void lambda$reload$3(net.minecraft.class_3300);
private static void lambda$buildMatcher$2(me.sargunvohra.mcmods.iamverysmart.match.SingleMatcher, java.util.List);
private java.util.List lambda$buildMatcher$1(net.minecraft.class_3300, net.minecraft.class_2960);
private static boolean lambda$buildMatcher$0(java.lang.String, java.lang.String);
static {};
}
You can see, it's mapped properly. Specifically this line:
public void method_14491(net.minecraft.class_3300);
Now excuse me while I mess around with fabric-loom to find out what went wrong.
Checked the latest build of Fabric API for the 1.14.2 Prerelease (fabric-content-registries-v0-0.1.0+59147463.jar) to see if it was just one bad build, but the problem is present there too. So it's at least a consistently recurring issue in Fabric API's build.
Yes. Apparently the remapping classpath doesn't actually include fabric-resource-loader. Or any of the dependencies. This is bad.
(This is only a problem for Fabric API's build system, but other mods with subprojects would also hit it)
I'm still curious about the exact mechanism of how this broke resource loading. Due to the issue, FlammableBlockRegistryImpl didn't fully implement the SimpleSynchronousResourceReloadListener interface in prod, but absolutely no errors were printed to the log. Is MC catching and silently ignoring some Throwable in the resource loading machinery? Might be interesting to poke around with jdb.
... Fuck, why? It's probably worth writing a mod to fix that. Mind pointing me to where in MC's code this catch is happening?
Is MC catching and silently ignoring some Throwable in the resource loading machinery?
Yes. Any and all non-Exception Throwables, if not all Exceptions, in fact.