search

SpongePowered / SpongeForge

A Forge mod that implements SpongeAPI
http://www.spongepowered.org/
MIT License
1.14k stars 306 forks source link

[1.12.2] Vanilla-like armor is completely ignored when player takes damage #3231

Open PolyacovYury opened 3 years ago

PolyacovYury commented 3 years ago

I am currently running

Issue Description This commit - 720b576 (build RC4081) - changed some handling of how armor parameters are put into place, but didn't alter any algorithms. As a result - all Vanilla-like armor (that extends ItemArmor) is completely ignored by all damage sources. Restoring prop = new ISpecialArmor.ArmorProperties(0, 0, Integer.MAX_VALUE); to prop = new ISpecialArmor.ArmorProperties(0, armor.damageReduceAmount / 25D, Integer.MAX_VALUE); brings armor back to how it is supposed to work.

While yes, those lines really did change in Forge the way the commit above alters them, there were also other changes to the way things are processed. The complete list of changes can be found in this commit: https://github.com/MinecraftForge/MinecraftForge/commit/e1ddc4315ca3a0ae97a337f08e1f136baa5d0930

I tried to re-implement them myself, but the ominous warning: // Beware all ye who enter here, for there's nothing but black magic here. wasn't as ironic as I thought.

So I suggest to either revert 720b576 and leave the issues fixed by Forge PR unfixed, or to try and figure out, how to alter the rest of the code to process those changes correctly.

embeddedt commented 3 years ago

Any updates here? Was this ever fixed/reviewed?

PolyacovYury commented 3 years ago

Well, it looks like 1.12.2 support has quietly come to an end, and all the work is focused on 1.16 now.

gabizou commented 3 years ago

It's not ended, just most people's time hasn't been as available as previously expected at the end of last year.

embeddedt commented 3 years ago

@gabizou If there isn't time to address this issue at the moment, perhaps the commit should be reverted as @PolyacovYury suggested? I think partially inaccurate armor calculations are better than having armor be ignored.

k1r0s commented 3 years ago

So I guess somebody has to fix this. Has anybody modified and compiled a new version?

embeddedt commented 3 years ago

At the moment, I am just sticking with my older version of Sponge which doesn't have the commit included. The 1.12 modding scene as a whole seems to be wrapping up now, so I am figuring that the older version of Sponge will work just fine since there is really no new development for 1.12.

k1r0s commented 3 years ago

Sure. Kinda sad MP forge is dying like this

embeddedt commented 3 years ago

I suspect it's because the 1.14/15/16 development scene has been highly active since 2020, with a lot of classic mods finally being ported forward. Nonetheless, 1.12 is still the more stable and tested version.

k1r0s commented 3 years ago

Forge should encourage more development on the "LTS" versions. Its crazy to think how much work was wasted on 1.13 - 1.15. Like when 1.7.10 was dropped while 50%+ of the players were there

vediis commented 2 years ago

Running into the same issue on spongeforge-1.12.2-2838-7.3.1-RC4082.jar with ice-and-fire installed as well. Removed all plugins and also sponge, confirmed it is indeed sponge. Will update to 1.12.2-2838-7.4.7 and see if that fixes it.

PolyacovYury commented 2 years ago

The issue was not fixed in newer versions of Sponge (as of writing this comment). You may try and clone/build my fork of SF which has some other fixes as well. Building it is as simple as cloning my SF repo, cloning my SC repo into the SC subfolder of SF, switching to mc4ep branch in both repos and running gradlew clean build.

gabizou commented 2 years ago

You may try and clone/build my fork of SF which has some other fixes as well.

Mind PR'ing some of those fixes? I've not gotten around to ever troubleshooting this particular bug but they'd likely be appreciated.

Regrad commented 2 years ago

The issue was not fixed in newer versions of Sponge (as of writing this comment). You may try and clone/build my fork of SF which has some other fixes as well. Building it is as simple as cloning my SF repo, cloning my SC repo into the SC subfolder of SF, switching to mc4ep branch in both repos and running gradlew clean build.

Can you please give us compiled file? I can't compile by my self, errors and i don't know how i can fix it.

Dockter commented 2 years ago

@gabizou can confirm changing the above lines does fix this issue.

Let me know if you want me to test something; I have my developer environment setup and can easily make new SF builds.

Dockter commented 2 years ago

The issue was not fixed in newer versions of Sponge (as of writing this comment). You may try and clone/build my fork of SF which has some other fixes as well. Building it is as simple as cloning my SF repo, cloning my SC repo into the SC subfolder of SF, switching to mc4ep branch in both repos and running gradlew clean build.

Can you please give us compiled file? I can't compile by my self, errors and i don't know how i can fix it.

Ping me on Sponge Discord; can give you a file to try out.

Regrad commented 2 years ago

Ping me on Sponge Discord; can give you a file to try out.

Done. Engardium, private message in Discord. You afk about 3 days)

KERRIGANRAYNOR commented 1 year ago

I still have this problem in the new versions of Sponge, how can I fix it?

Dockter commented 11 months ago

I still have this problem in the new versions of Sponge, how can I fix it?

Do you still need a copy of this. I can make you a jar. I have a repo here with our changes: https://github.com/AlmuraDev/SpongeForge/commits/stable-7/