search

Shadows-of-Fire / Apotheosis

All things that should have been.
Other
155 stars 120 forks source link

[Crash]: Dodge leads to a crash if you hit the person with dodge too fast #1276

Open Satherov opened 5 days ago

Satherov commented 5 days ago

If a player is wearing boots with dodge or a shield with dodge and you hit very quickly (in my test I used a bow from Draconic Evolution with 600% speed) once the dodge triggers, it crashes the server/game. This was reproduced at least three times so far and I tested it in single player with a mob wearing the armor and the crash still happens.

When playing on a Server, it just gives up after ~10 minutes of it being unresponsive and if you play in single player, you have kill your instance or your game will be frozen forever (waiting almost 2 hours did not help). Therefore this does not produce and actual crashlog

Shadows-of-Fire commented 5 days ago

Can you reproduce this without draconic evolution?

Satherov commented 4 days ago

It seems like the issue is not, like previously assumed, the speed at which the person is hit, but armor penetration. When the draconic bow tries to penetrate and dodge tries to prevent that , it leads to dodge triggering thousands of times over until the game crashes.

Log: https://gist.github.com/Satherov/0545a25968bf6f87357dd6692cd1906c

Apparently this issue is already known and supposedly fixed in your own code but it still seems like the crash still occurs

I even found some of your PRs for Forge itself, addressing the issue, but it seems like there hasn't been a true fix implemented yet?

Additionally, it seems like DE uses a different way of penetrating armor, so it doesn't get detected by your function, though I only took a short glance at the DE code so I might be wrong

Shadows-of-Fire commented 4 days ago

If that's the case, it means DE will have this problem with anything that cancels the ProjectileImpactEvent, since it will trigger an infinite loop with the same root cause as the MCF issue.

I think you'll need to raise the issue to DE, and either tell them to utilize piercingIgnoreEntityIds or add some other form of reentrancy checking for their piercing implementation.