ValkyrienSkies / Valkyrien-Skies-2

Valkyrien Skies 2
https://valkyrienskies.org/
GNU Lesser General Public License v3.0
227 stars 99 forks source link

Beacon Effects May Not Work on Worlds with Increased Height #989

Open EnigmaticWraith opened 3 weeks ago

EnigmaticWraith commented 3 weeks ago

This issue occurs when only Valkyrien Skies and addons are installed and no other mods

Minecraft Version

1.20.x

Mod Loader

Fabric

Issue description

When playing in any world that increases the overall height (logical_height in a dimension_type definition inside a datapack) , beacon effects may no be longer applied.

It appears to be related to this function that stops crashes related to too large bounding boxes limiting the y-height: https://github.com/ValkyrienSkies/Valkyrien-Skies-2/blob/686bd75c6fe996d7c7ec8a0ca33d31a830eb3fc6/common/src/main/kotlin/org/valkyrienskies/mod/util/BugFixUtil.kt#L6-L8

I compiled a test build where I set aabb.ysize > 2048 with a logical world height of 1088 and the beacons work as expected once again. I'm not sure if 1000 was an arbitrarily large value to limit it to or if this is going to lead to instability, however I've yet to experience any crashes in my limited testing.

It also does not appear to be tied 1 to 1 with world height, as I did a second test with a cutoff of 2049 and world height of 2048 and the beacons no longer worked once again. Increasing further to 3000 for the 2048 world height works fine.

Issue reproduction

Make a new world with a datapack that changes logical world height to over 1,000 blocks, create a beacon and attempt to activate an effect. The beacon will appear to function, but no effects will be applied to players in range.

Logs

2024-10-21-1.log

EnigmaticWraith commented 1 week ago

As an update: this fix was unstable, it leads to memory usage that increases over time and eventually crashes the server. For the time being I've reverted that check to how it was originally and added a specific case that matches AABBs of the exact size of a beacon object (101, worldHeight+101, 101). Although still not really a proper solution, so far that's been working fine for almost a full week.