Up-Mods / CrookedCrooks

Adds crooks for all the vanilla materials. They multiply drops from leaves and grasses and they pull mobs!
MIT License
3 stars 0 forks source link

[1.18.1] Crash with SurvivalPlus, Crooked Crooks, and Numismatic Overhaul #5

Closed coolsimulations closed 2 years ago

coolsimulations commented 2 years ago

Greetings!

crash-2022-02-06_13.31.33-server.txt

This crash seems to be along a similar vein as https://github.com/EnnuiL/CrookedCrooks/issues/4.

I am the developer of SurvivalPlus, MoreThanAPickaxe and SurvivalPlus Lightsabers. I've just received a crash report in this issue https://github.com/coolsimulations/SurvivalPlus/issues/9. It was redirected to me from https://github.com/gliscowo/numismatic-overhaul/issues/8.

Steps to Reproduce:

  1. Use Fabric 1.18 (and API) with SurvivalPlus, MoreThanAPickaxe and Numismatic Overhaul.
  2. Summon a Villager and place a Smithing Table to make him a Tradesmith
  3. Trade until reaching level 3 or 4. There is usually a trade that allows you to trade emerald (or in this case silver coins) for an enchanted Titanium Adze.

As Numismatic Overhaul reads this trade to replace the emeralds with coins it has an issue processing the enchantment values on the Titanium Adze and crashes. This doesn't occur unless you add CrookedCrooks. Even though your thorns enchantment can only be applied to Crook Items it, for some reason, decides to crash here.

I believe it is because we both mixin to the EnchantmentHelper class, mine here and yours here. Since this issue doesn't occur when it's just SurvivalPlus and Numismatic Overhaul, or SurvivalPlus and CrookedCrooks, or CrookedCrooks and Numismatic Overhaul, I assume it's something on your end? As you mentioned in the previous issue it's caused of a missing Enchanting module in Fabric API. I actually copied by solution directly from Forge so I could use it directly in Item classes as in both MoreThanAPickaxe and SurvivalPlus Lightsabers without needing to mixin each time.

I'm not sure how we should go about fixing this issue. Would changing you mixin to Inject as a HEAD type with a higher priority fix it?

Another solution could be adding a condition within the mixin to check if SurvivalPlus is loaded and if it's not then continue. Then you could implement my ItemAccessor interface in a new class that extends CrookItem. Then when you register your Crook items you can again check if SurvivalPlus is loaded and then if so register your items as the extended CrookItem clas that implements ItemAccessor.

Again I'm not certain where the fault lies. Let me know what you think/or if it's something wrong on my end.

EnnuiL commented 2 years ago

Ah, I see why this crash happens; Villagers, like enchanting tables, chooses a random enchantment using EnchantmentHelper, and Crooked Crooks has a mixin in EnchantmentHelper that prevents the NPE from happening by adding it to the list itself and preventing a condition from the original code from happening when it's my enchantment's turn, therefore, not causing the NPE crash A little problem, however, is that your mixin does the equivalent of an @Overwrite by injecting at the HEAD and then cancelling, preventing my mixin from doing its job and causing the NPE;

Looking at your code, I see that what you are doing is a way of including modded items to enchantment categories (mojname), I'd recommend investigating a way to inject directly into them, however, yeah, at a glance, it seems that it's easy said than done

Although perhaps you could do something a little bit dirty that does the same but without the overwriting, like using @ModifyArg in order to change the stack argument of the original method so if it is your custom item, it'll change to a compatible item (ex: WEAPON changes the stack to a wooden sword); I'd also mention the possibility of a @Redirect, but considering the lack of an Enchantment API, it might cause problems with other mods

So, yeah, try the above solution, maybe it'll fix the problem

coolsimulations commented 2 years ago

Thanks for the assistance! I fixed the issue on my end with this commit https://github.com/coolsimulations/SurvivalPlus/commit/e0dd84d9352094d48426abe9f0447818544fcb06