Goblin Traders

Goblin Traders

33M Downloads

[1.18.1] Incompatibility with Enchanting Overhauled

Alaberti opened this issue ยท 5 comments

commented

The game crashes during initialization when Goblin Traders and Enchanting Overhauled are used together. Here's the log I get when trying to use this mod and Enchanting Overhauled together: https://pastebin.com/DTyCDazr

After speaking with the mod author on curseforge, here's what he had to say:

JohnSmith_474

I looked into it, Goblin Traders is trying to inject code into the anvil menu after Enchanting Overhauled overwrote the anvil menu class. The changes I make to the anvil are only possible by overwrite, since I need to change things and not just add. The best I can do is add a config option to disable all anvil changes. Unfortunately my mod gets registered first due to alphanumeric ordering where E comes before G. The best option would be to ask MrCrayfish to add a check for my mod and disable his anvil changes if Enchanting Overhauled is detected. I'll look into contacting him about it, but chances are slim. Until then I'll mark my mod as incompatible with Goblin Traders.

I don't know if he's on github, but here's the mod: https://www.curseforge.com/minecraft/mc-mods/enchanting-overhauled

Hope any of that helps!

Goblin Traders: 1.7.0
Enchanting Overhauled: 1.0.2
Forge: 39.0.19

Edit: Double-tapped before I done typing, whoops.

commented

Appreciate the author of Enchanting Overhauled to look into the issue. The problem is that my Mixin is a patch to prevent overlevelled enchantments returning to their max level if merged in an anvil and it also ensures that overlevelled enchantments can't be merged to create even higher levels. Personally I'd recommend author of Enchanting Overhauled to add new blocks with their own new menu. Overriding existing blocks, especially the use of @Overwite Mixins bears the risk of breaking compatibility with other mods. @Overwrite is highly discouraged even by the creators of Mixins. It should only be used when it's not possible any other way to create the same behaviour. I unfortunately can't see code of their mods due to being closed source and can't suggest an alternative. If they can avoid using @Overwrite and @Redirect it may allow the mods to be compatible.

commented

Hey, so I finally got around to creating a github account, I might upload the codebase after cleaning it up a bit.

Anyways, I looked through your code and I understand that you change the anvil via mixin to accommodate overleveled enchantments. And that way I don't see how the overleveled enchanted items of your mod would cause any incompatabilites with the anvil changes of my mod: simply it completely disables enchantments being applied using an anvil, however it keeps the enchantments on the left hand side item exactly the same when processing through the anvil. Neither does my enchanting table update enchantment levels if they are overlevelled, so they should be compatible if Goblin Traders skips the RepairMenuMixin when Enchanting Overhauled is detected.

However, I do understand that it is not your job to accomodate my mod and neither do I want to put all the responsibilties on your shoulders, so I will look into not using the @overwrite annotation if possible. I was using registry overrides in an earlier version, but the anvil rename method requires a server packet validation, and that method calls the wrong anvil menu, so it wouldn't change item names. Also, on several modded minecraft discords I got told to avoid using registry overrides in favor of mixins.

Best regards
John Smith

commented

Thanks for sharing your code. I can't definitely look into disabling my Mixin if your mod replicates the same behaviour fix that my mod resolves.

commented

Hello, been a while. I got around remaking all the anvil related mixins in Enchanting Overhauled, refactoring every @overwrite to something more precise. I tested the two mods in conjunction, the game boots up fine, no errors, and it just works. Certain Items can still exceed the max enchant level when given to one of your Goblin Traders, even though anvil enchanting is disallowed by EO.
So as far as I cen tell, this issue is resolved and can be closed now.

Cheers
JS

PS: I've been meaning to ask if it was intentional or just a typo that "Lure" skips from max level 3 to 5 instantly?

commented

Thanks for taking to time to resolve the issue. I intentionally made it level 5 since there wasn't a huge difference from 3 to 4.