CleverNucleus / RelicEx

Adds compatible trinket items for PlayerEx attributes.
MIT License
5 stars 10 forks source link

RelicEx removes attributes from unrelated armor pieces #37

Closed ZsoltMolnarrr closed 1 year ago

ZsoltMolnarrr commented 1 year ago

Hello!

The problem RelicEx seems to be removing attributes from items those are not related to it in any way. My example is a set of wizard robes, those come with custom magical attributes.

Without RelicEx Screenshot 2023-02-15 at 22 55 20

With RelicEx Screenshot 2023-02-15 at 22 51 24

Way to reproduce

  1. Use CurseForge launcher, create Fabric 1.19.2 instance
  2. Install Wizards
  3. Install RelicEx
  4. Enter the game

Expected RelicEx not mutating/overriding attributes out of its own scope.

Additional information The magical attributes on the screenshot are provided by Spell Power Attributes. They are registered on mod initializer, using vanilla API (so nothing out of ordinary). The wizard robes are implemented in Wizards. Nothing in the Fabric API does indicate a faulty implementation on this end. Please feel free to point out if there is any. https://github.com/ZsoltMolnarrr/Wizards/blob/1.19.2/common/src/main/java/net/wizards/item/WizardArmor.java

cleannrooster commented 1 year ago

Here I believe is the offending mixin: https://github.com/CleverNucleus/RelicEx/blob/65845619a0064544a5bfbffcb2c7b92463c65d69/src/main/java/com/github/clevernucleus/relicex/mixin/ArmorItemMixin.java See getAttributeModifiers, at the end, it checks if "modifiers" is empty. If it is, it defaults to attributemodifiers, which is empty anyways, resulting in an item with empty attribute modifiers. If you put this.getDefaultAttributeModifiers() instead of this.attributemodifiers, i believe it will fix the issue.

CleverNucleus commented 1 year ago

Hi there @ZsoltMolnarrr @cleannrooster I appreciate the alert and detailed insight; I've received similar reports - it looks like I need to refactor this implementation. Thanks.

ZsoltMolnarrr commented 1 year ago

Hello @CleverNucleus ! Just for your information, I would like to express the severity of this issue. Wizard armor sets are designed to give 50% of the total magical attributes. This issue renders all wizard armor sets useless, causing the wizard combat specs to be 50% weaker than intended, compared melee or range combat.

The 2 mods are both present in MedievalMC modpack, affecting thousands of players.

Please don't take this as pushing you, I would just be very grateful for a fix as soon as possible.🙏

CleverNucleus commented 1 year ago

Hi @ZsoltMolnarrr, I understand, and I appreciate your response. I have not had the opportunity to work on it; however, if nothing unexpected comes up I will output a fix sometime between the 28th Feb and 3rd Mar. Thanks.

ZsoltMolnarrr commented 1 year ago

Thank you for the heads up!

CleverNucleus commented 1 year ago

Hi @ZsoltMolnarrr @cleannrooster, Latest release has fixed the issue - available on Modrinth, pending for approval on Curseforge. Thanks again for bringing awareness to this bug.