Virtuoel / Pehkui

Fabric/Forge/NeoForge/Quilt mod that allows for changing the size of any entity.
MIT License
87 stars 33 forks source link

[Bug]: StackOverflowException - TypedScaleModifier #526

Open ByThePowerOfScience opened 1 week ago

ByThePowerOfScience commented 1 week ago

Minecraft version(s)

1.20.1

Mod loader

Forge

Mod loader version

47.2.30

In what kind of world or server did the problem occur?

My multiplayer server that I was running

What went wrong? (Crash logs don't go here)

Hiya, mod dev here. Saw some stackoverflows in the logs: https://mclo.gs/yEODMf5

I checked with Mixin export enabled, and nothing's mixing into it. It just seems like a minor oversight in TypedScaleModifier:

https://github.com/Virtuoel/Pehkui/blob/460c3ec64f20bedd18ef1a5dee37dc4ed17de913/src/main/java/virtuoel/pehkui/api/TypedScaleModifier.java#L44

TypedScaleModifier#modifyScale calls ScaleData#getScale which then, whoops, calls ScaleModifier#modifyScale. 😬

https://github.com/Virtuoel/Pehkui/blob/460c3ec64f20bedd18ef1a5dee37dc4ed17de913/src/main/java/virtuoel/pehkui/api/ScaleData.java#L238-L261 https://github.com/Virtuoel/Pehkui/blob/460c3ec64f20bedd18ef1a5dee37dc4ed17de913/src/main/java/virtuoel/pehkui/api/ScaleData.java#L252

Seems like this is causing an infinite loop of the same scale modifier getting hit every time, eventually resulting in a stack overflow.

Normally I'd offer a suggestion on how to fix it or make a PR myself, but I can't see an obvious solution without knowing more about the internals, so I'm making an issue instead.

Full list of installed mods and their version numbers

Biomancy 2.8.1.0, as well as a very large modpack that somehow doesn't have anything to do with this.

Did the problem cause the game to crash?

No, because it's on another thread, but it does stop the scaling from activating.

Virtuoel commented 1 week ago

TypedScaleModifier prevents a stackoverflow by comparing the modifier's type against the type of the ScaleData argument, and returns the passed scale if they're the same. Seems Biomancy doesn't do that sort of check for their custom scale modifier, so this is likely an issue on their end. A simple fix would be for them to use TypedScaleModifier instead of their custom one.

ByThePowerOfScience commented 1 week ago

Welp, I'll file the report there then. Thanks for looking into it!

ByThePowerOfScience commented 1 week ago

@Virtuoel Wanted to let you know: mixin-patching it to use TypedScaleModifier resulted in the same issue occurring, just without their anonymous class appearing in the stacktrace.

Virtuoel commented 1 week ago

Did that also occur on your server? Does the problem not happen e.g. in singleplayer?