No Enchant Cap

No Enchant Cap

Mods
449k Downloads

Tax Free Levels compatibility

Fourmisain opened this issue · 1 comments

commented

Hey, Tax Free Levels author here. 👋
There was a request for mod compatibility and I had a look at what's conflicting.

Conceptually,
No Enchant Cap's "fair level cost" turns the anvil's level cost N into an XP cost (the XP needed to go from level 0 to level N).
Tax Free Levels does a very similar thing, except it is based around player level 30 (it goes from 30-N to 30 if N is small and the player level high).
(Earlier versions actually did the same as you, but I found that it made small anvil costs unreasonably cheap.)

Tax Free Levels also works for enchanting costs (and also changes the anvil rename cost to 1), so it'd make sense running both mods together.
I think it would make the most sense if Tax Free Levels takes precedence in this aspect, since that's basically the whole point of the mod.

On the technical side, this @Redirect

no-enchant-cap/src/main/java/amymialee/noenchantcap/mixin/NECMixin_AnvilScreenHandler.java

Lines 58 to 65 in dedd5a4

@Redirect(method = "onTakeOutput", at = @At(value = "INVOKE", target = "Lnet/minecraft/entity/player/PlayerEntity;addExperienceLevels(I)V"))
private void fairLevelCost(PlayerEntity instance, int levels) {
if (!NoEnchantCap.getConfig().fairLevelCost) {
instance.addExperienceLevels(levels);
} else {
instance.addExperience(-getLevelTotal(-levels));
}
}

conflicts with this @ModifyArg
https://github.com/Fourmisain/TaxFreeLevels/blob/46db6e4682acb62e8f6397a4db894b9a89a23dda/fabric/src/main/java/io/github/fourmisain/taxfreelevels/mixin/FlattenAnvilCostMixin.java#L23-L34
since the latter can't find the addExperienceLevels() call anymore after it has been redirected.

While this mixin conflict can be easily solved by turning your @Redirect into a @ModifyArg as well , this wouldn't really solve the conceptual conflict.
I could change the mixin priority on my end to fix this, but I intentionally left it very low for mod compatibility (ironic, isn't it?), since I knew a mod that changed the level cost there too.

The quickest way to make both mods compatible would be lowering your Mixin priority below that of Tax Free Levels, e.g. by setting priority = 1600.

A more "clean" way would be using a Mixin plugin. The @Redirect can be moved into it's own mixin class and then only loaded if Tax Free Levels is not detected.
(More concrete, implementing IMixinConfigPlugin, adding it to the fabric.mod.json and letting its shouldApplyMixin() return false when FabricLoader.getInstance().isModLoaded("taxfreelevels") && mixinClassName.endsWith("NameOfThatRedirectMixin"))

commented

Sure, whichever method you think is better I'm alright with.

If you want to make a pull request I'll accept it and upload the update, whichever method you prefer.