Apotheosis

Apotheosis

79M Downloads

Replacing vanilla registry objects is bad, here's why

Aizistral opened this issue · 1 comments

commented

Okay, look, this is not okay:

((EnchantedTrigger) CriteriaTriggers.ENCHANTED_ITEM).trigger((ServerPlayerEntity) player, enchanted, level, this.eterna.get(), this.quanta.get(), this.arcana.get());

I did not deeply investigate on code behind this yet, but from what I understood you just replaced vanilla EnchantedItemTrigger with your own extension of it like nobody's bussiness. Do I really need to tell why this is a bad idea? Do you understand your mod is not the only mod in the observable Universe, and vanilla enchanting table is therefore not the only block in the observable Universe which might use that trigger?

From my own tests, calling the vanilla version of it causes some of your mod's advancements to go wild, granting the player every and all enchantment advancements that base their conditions on Eterna/Arcana/Quanta with complete disregard of those exact conditions, as if they were all instantly met for whatever reason. But this example is just a tip of the iceberg of every and all scenarios when this practice of replacing vanilla object will horribly break. Have you thought about the instance of some other mod trying to do the same, as an example?

Stop it. Get some help. Make your own independent trigger, after all, I see absolutely no reason why it must replace vanilla one. If you are concerned about advancements using vanilla trigger being unobtainable through your Incredibly Very Custom Enchanting Systemâ„¢, just invoke vanilla trigger from within your EnchantedTrigger#trigger method,.

commented

Also for the anvil, this mod and charm are constantly messing up each other's anvil functionality.