[1.18.1] Crash with Enchanting Overhauled

[MrHazMat]

TFL and EO both modifies the anvil which causes the game to crash on start. A workaround is probably config to disable the cheap renaming

Caused by: org.spongepowered.asm.mixin.injection.throwables.InvalidInjectionException: @At("INVOKE") on net/minecraft/world/inventory/AnvilMenu::taxfreelevels$makeRenamingCheap with priority 1000 cannot inject into net/minecraft/world/inventory/AnvilMenu::m_6640_()V merged by johnsmith.enchantingoverhauled.mixin.anvil.MixinAnvilMenu with priority 1000 [PREINJECT Applicator Phase -> taxfreelevels-common.mixins.json:CheapAnvilRenameMixin -> Prepare Injections -> -> handler$zza000$taxfreelevels$makeRenamingCheap(Lorg/spongepowered/asm/mixin/injection/callback/CallbackInfo;Lnet/minecraft/world/item/ItemStack;III)V -> Prepare]

[Fourmisain]

taxfreelevels (...) cannot inject into net/minecraft/world/inventory/AnvilMenu::m6640()V merged by johnsmith.enchantingoverhauled (...)

Hm... I think this means Enchanting Overhauled overwrites the whole method, which is not great because this usually means the conflict is unfixable and since Enchanting Overhauled is All Rights Reserved I can't read their source or PR any solution either.

So I had a look at the gameplay of Enchanting Overhauled. It seems like the anvil doesn't use any XP or levels (and you can't combine enchanted items anymore?), so the anvil changes from Tax Free Levels actually don't actually matter at all.

The good news is that disabling Tax Free Levels' CheapAnvilRenameMixin and FlattenAnvilCostMixin (which also conflicts) does leave the enchanting table fully functional! Problem solved, right?

Well, the thing is that I have to conditionally disable those 2 mixins when Enchanting Overhauled is installed - which on Fabric is absolutely no issue at all, but I just tried the very same on Forge and at the point Mixins are loaded, Forge doesn't even have the mod list ready, so I can't tell whether to disable the mixins or not.

Of course I could add a config file for this, but it'd be much better if it was automatic and it's already possible to disable mixins manually by removing them from the taxfreelevels-common.mixins.json and taxfreelevels-forge.mixins.json files inside the mod jar.

Maybe there's a way around this, for now I just created a version of Tax Free Levels without the Anvil changes, which is fully compatible with Enchanting Overhauled.

I'll leave this issue open for now, until I find a better solution.

[JohnSmith474]

Hi there! I'm the dev of Enchanting Overhauled. I haven't come around creating a github repo, I'll do that after my exams. Until then here is a pastebin to the MixinAnvilMenu.

I'll also try to come up with a better solution for my anvil changes, but so far I wouldn't know how to do my changes. As you have noticed, right now Enchanting Overhauled is pretty much incompatible with anvil changing mods by design. I had an earlier attempt where I tried to registry overwrride the anvil, but that didn't work as well as I had hoped. That is because the anvil menu needs a serverpacket which calls the vanilla anvil menu which confirms that the player has renamed an item, in short: my overwritten anvil can't rename items. So until I figure that out the best I could do is an anvil without renaming capabilities.

[Fourmisain]

@JohnSmith474 So I did a comparison of your onTake method with the original and (after renaming variable names to match) it looks like this: (Original method on the right)

So there's actually only 3 small changes:

• the removal of the giveExperienceLevels() call
• the removal of cost.set(0)
• the fixed 0.12F break chance is replaced by a config value

Those can easily be done without overwriting the whole method.

The 0.12F constant can be changed with a @ModifyConstant like this:

@ModifyConstant(method = "onTake", constant = @Constant(floatValue = 0.12F))
private float replaceBreakChance(float value) {
  return (float) ModConfig.ANVIL_BREAK_CHANCE.get();
}

The removal of giveExperienceLevels() makes repairing cost nothing and cost.set(0) seems to have no further effect (since the cost is not being used anymore). The same effect should be achievable by setting the cost to 0 before giveExperienceLevels() is reached. The simplest way to do that would probably just set it to 0 at the very beginning of the method like this:

@Inject(method = "onTake", at = @At("HEAD"))
protected void noRepairCost(Player player, ItemStack itemStack) {
  cost.set(0);
}

(It's probably even better to set the cost to 0 somwhere at the end of the createResult - or never set the cost to anything non-zero in the first place - so this inject might not even be needed at all.)

That'd solve the Mixin conflict with the onTake method and the code is a lot simpler too. Note I haven't tested if those two injectors work, I just quickly wrote them down.

---

There might be a way to do something similar with createResult. On first glance it looks like there's a ton of changes and maybe it's not actually feasible to separate the changes and an @Overwrite is the "right" decision afterall - but maybe it's not as hard as it looks. Can you explain what all the changes compared to vanilla are? Stuff like "no anvil cost", "no combining of enchanted items" can probably be achieved with some surgical precision.

Btw, if you wanna know more about Mixins, I'd suggest you check out the small and quick Fabric tutorial for it (it's not Fabric specific) and of course read the official documentation. It can take quite some time to learn how to achieve what you wanna do with Mixin but it's oh so worth it.

[Fourmisain]

I think I might have just found a way to check if a mod is installed before applying Mixins.

While Forge doesn't seem to have the mod list ready at that point, there's a "loading mod list" which I can check for the mod id.

[Fourmisain]

Fixed in https://github.com/Fourmisain/TaxFreeLevels/commit/9662b454bf0d3a079df731f10941fa70c773cbc7, the mod now disables the conflicting mixins when it sees Enchanting Overhauled is installed. New version is up on CurseForge.

[JohnSmith474]

@Fourmisain Apoligies for my very very late response. I had some issues with my email account and couldn't log into github for a while. I just saw your take on my anvil mixin classes and I thank you for the time and effort you put into compiling your findings. At the time I wrote those classes it was my first time using mixins and I had no experience whatsoever, so I'll definetely take look at the tutorial you provided and try to find a way to make the changes I need to make without screwing every other mod over.

The changes I intend to make to the anvil is to remove all of its ties to enchanting, so no enchanted books can be applied using the anvil and to make it so the player can't create items with more than 3 enchantments on them.

I friend of mine made a similar mod with forge anvil events and I tried to implement his approach but eventually chose mixin overwrites because forge anvil events could not remove xp costs in any way.

So I'm thinking now with your approach to remove xp cost it might be possible after all.

Again thank you for pointing me in the right direction.