Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mild incompatibility with The Endergetic Expansion #159

Closed
alpharou opened this issue Mar 13, 2021 · 14 comments · Fixed by #161
Closed

Mild incompatibility with The Endergetic Expansion #159

alpharou opened this issue Mar 13, 2021 · 14 comments · Fixed by #161
Assignees
Labels
3rd party issue Issue only arises when some other mod/plugin/whatever is present bug Something isn't working status: resolved Already fixed/implemented in the upcoming update

Comments

@alpharou
Copy link

alpharou commented Mar 13, 2021

Versions:

  • EnigmaticLegacy-2.11.1
  • curios-forge-1.16.5-4.0.5.0
  • Patchouli-1.16.4-50
  • abnormals_core-1.16.5-3.1.1 (not the issue, this library installed is just fine)
  • endergetic-1.16.4-3.0.0
  • forge 36.0.43

Description:

Mild incompatibility with The Endergetic Expansion Mod: https://www.curseforge.com/minecraft/mc-mods/endergetic

All equipped curios added by Enigmatic Legacy are unequipped when quitting and reentering singleplayer or server worlds. If there's no inventory space available the curios will be dropped onto the ground.

The unequipped curios will leave a ghost item in the curios slot that dissapears when it is interacted with.

It only affects Enigmatic Legacy's curios, and more importantly, affects the seven curses ring, so it's possible to get it off without the power of god.

How to Reproduce:

  1. Load a world
  2. Equip any Enigmatic curios, including the ring of seven curses
  3. Log out/save and quit
  4. Reenter the world
  5. Behold!
    image

Logs:

No crashes.

Should I post an issue on the other mod's repository?

@alpharou alpharou added awaits review This issue was not yet reviewed by developer bug Something isn't working labels Mar 13, 2021
@Aizistral Aizistral removed the awaits review This issue was not yet reviewed by developer label Mar 13, 2021
@Aizistral
Copy link
Member

Did this only happen in your instance with Endergetic Expansion installed? Because we just so happen to have #160 which does claim this same bug happens regardless of other mods, simply because of some changes on the side of Curios.

@alpharou
Copy link
Author

I can confirm that the list of mods I listed without the endergetic expansion and with default comfigs do not cause this issue at all. Everything works as intended.

Creating a world with endergetic, then removing the mod and reentering the same world does not cause the problem.

Also, I tested a 80 mod modpack, and Enigmatic Legacy is the only mod that has this issue, amongst other like Eidolon, Artifacts...

What I don't understand is what has Endergetic Expansion to do with curios, as far as I can tell, the mod doesn't add any.

@D-Bigboy-302
Copy link

D-Bigboy-302 commented Mar 14, 2021

I have endergetic as well in my modpack of 100+. I'm pretty sure this is from Curios. Try downgrading to Curios 4.0.4.0 and see if the issue persists. For me, on issue #160, this only happened when I updated to that Curios version. Which does state that it removes invalidated Curio items in the changelog. https://www.curseforge.com/minecraft/mc-mods/curios/files/3231111

@wchen1990
Copy link

Can confirm that this is an incompatibility with Endergetic Expansion.

You can also reproduce this issue by running /reload.

@wchen1990
Copy link

I take that back, it is not an incompatibility with Endergetic Expansion at all.

You can reproduce this issue with just Curios 4.0.5.0 by running /reload.

wchen1990 pushed a commit to wchen1990/Enigmatic-Legacy that referenced this issue Mar 15, 2021
@Aizistral Aizistral linked a pull request Mar 15, 2021 that will close this issue
@alpharou
Copy link
Author

alpharou commented Mar 18, 2021

Versions:

  • EnigmaticLegacy-2.11.2 (9fda5b5)
  • curios-forge-1.16.5-4.0.5.0
  • Patchouli-1.16.4-50
  • abnormals_core-1.16.5-3.1.1 (not the issue, this library installed is just fine)
  • endergetic-1.16.4-3.0.0
  • forge 36.0.43

It still causes the issue.

Reposted to the Endergetic team: team-abnormals/endergetic#162

@wchen1990
Copy link

That's interesting...

I fixed the incompat with curios itself, I wonder what Endergetic is doing that'd trigger this.

@wchen1990
Copy link

wchen1990 commented Mar 18, 2021

Figured it out.

When Endergetic is installed and when you load a world Curios' readNBT is called twice. The code responsible for this in Endergetic:
https://github.com/team-abnormals/endergetic/blob/75d75f684a505b991ea752053280fdf22b278807/src/main/java/com/minecraftabnormals/endergetic/core/mixin/PlayerListMixin.java#L40

The basic breakdown of this is basically:

  • Server starts with a player object with nothing in their curios slots
    (Enigmatic's canEquip returns true because the player has nothing in their curios slots)

  • Server reads and decrypts the NBT data saved for the specific player and equips their Curios accessories
    (Enigmatic's canEquip returns false after this because our slots are filled)

  • Endergetic's mixin makes Curios call readNBT again and since canEquip is false, Curios unequips our stuff

It seems the fix I did only safeguards us from /reload but not from readNBT being called twice. I apologize for assuming that it was just an incompat with just Curios and not both.

EDIT: I'm personally conflicted as to which mod (Enigmatic, Endergetic, or Curios) would be the best place to fix this issue.

@alpharou
Copy link
Author

alpharou commented Mar 18, 2021

I am not qualified for suggesting deep changes into any code, but i'll try my best:

I think one of the major points to consider is, what if equipping multiple enigmatic curios isn't that bad?
That way Curios doesn't have to make critical changes that would propagate to hundreds of other repositories, and thus Enigmatic is more self contained and insensible to other mods messing with the inventory, calling readNBTs and many other inconcevable shenanigans.

  • Maybe a rework of the way curios apply their effect so they are somewhat disjointed from the actual item? Perhaps the item sets up a flag for an overseer (service, daemon, that sort of thing) to actually apply the effect onto the flagged player?

If the problem is that enigmatic curios have to be unique because their nbt contains critical data for the effect to be called suscesfully, maybe implement an effect stack so that the first curio that called the effect is the one that prevails at the end, overwriting any duplicated individual effect (item magnet, floating gravestone, 7 cursed...)

  • Another option would be to set up some way of enigmatic curios disabling themselves or marking their own nbt when they do the duplicate instance check instead of messing with canEquip.

The player could be notified so they don't assume they're getting double the effect of two 7 curses curios (which would be awesome by the way)

If someone in the dev team wants to keep the issue tidy, feel free to delete this reply.

@Aizistral Aizistral added the status: confirmed Bug was confirmed through testing and will be fixed... probably label Mar 18, 2021
@Aizistral Aizistral reopened this Mar 18, 2021
@Aizistral Aizistral added the 3rd party issue Issue only arises when some other mod/plugin/whatever is present label Mar 18, 2021
@D-Bigboy-302
Copy link

Ah so it was also endergetic! Seems I am still having the issue as well.

@wchen1990
Copy link

I made a quick fix as a stop gap measure in fixing Endergetic compat with Enigmatic. However, their license prohibits me from publishing a build of it so here's the branch for the fix: https://github.com/wchen1990/endergetic/tree/EnigmaticQuickFix

If you have JDK installed on your computer you should be able to build from source. Finding resources to build from source isn't too bad either.

@Aizistral
Copy link
Member

I suppose as of Curios update 4.0.5.1 this should be resolved either, but we need someone to confirm that this is indeed the case.

@D-Bigboy-302
Copy link

After the curios update, everything seems to be working for me. Not having the issue anymore

@Aizistral
Copy link
Member

Neat.

@Aizistral Aizistral added status: resolved Already fixed/implemented in the upcoming update and removed status: confirmed Bug was confirmed through testing and will be fixed... probably labels Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party issue Issue only arises when some other mod/plugin/whatever is present bug Something isn't working status: resolved Already fixed/implemented in the upcoming update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants