
[1.21] Cyanide ignoring Enchancement's registry disabling
MoriyaShiine opened this issue ยท 3 comments
Loader: Fabric
Minecraft Version: 1.21.1
Cyanide Version: 5.0.0
One of Enchancement's main features is to disable loading certain enchantments, but Cyanide seems to completely overwrite that functionality (specifically here), causing the game to not actually disable anything, and instead crash when the creative tab is opened (see here, isolated crash here). I've tried making a more generic mixin to disable enchantments, but now it errors when attempting to load the enchantments involved in enchantment providers (but presumably more), seen here.
What exactly is Cyanide doing to cause this, and is there anything I can do to work around it?
Very broadly, Cyanide rewrites the flow of registry resource loading in order to provide better error detection, error messages, and exception handling. It essentially has to take control over this process - there is no post-facto way to provide nearly the level of depth or context that Cyanide is currently doing, by just taking vanilla's Map<ResourceKey<?>, Exception>
and using that to extrapolate errors (for example, knowing where something was referenced, not just that an unbound holder was present in the registry). I try and call into as much vanilla logic as I can, without requiring a huge amount of brittle, difficult-to-maintain mixins into the existing vanilla code.
Looking at Enchancement, I'm assuming it's limited to this mixin, as Cyanide would be bypassing that logic completely. From a basic look,
- The first method appears to just be adding an enchantment to the registry, unconditionally. Is this not something that can be provided i.e., as all other enchantments are? By letting it be present as a resource and loading it normally?
- The second method appears to be filtering enchantments from being loaded, at the level of resource loading.
For the second injector, the options I see are:
- Cyanide's
RegistryLoader
needs to be completely rewritten, in order to invokeRegistryDataLoader.loadContentsFromManager()
, which you currently mixin to. This involves a huge amount of work, and makes Cyanide's injections very brittle - I would need to mixin at least one, two, three locations, all of which would need locals access, probably thread-local or shared variable access, and potentially control flow modification. I would greatly, rather avoid this - You move your mixin to somewhere outside of the flow of registry loading, or at least somewhere that Cyanide doesn't take control over. For example, rather than wrapping the call to
FileToIdConverter.listMatchingResources()
within registry loading, inject into that method itself, because Cyanide still will call it - Change your method of disabling these registrations from "patch the map of IDs -> Jsons" to something that hooks Fabric API's existing API for conditional registration. Cyanide explicitly invokes it, and if you moved the injection into
ResourceConditionsImpl.applyResourceConditions()
(from Fabric API), Cyanide would also catch it
Thanks! Option 2 is what I'm currently looking at, I'll let you know if it works.
That, along with fixing another issue related to client-server registry desyncs, managed to do it! Here's the commit if you're curious.