bergerhealer / TrainCarts

Minecarts redefined
MIT License
205 stars 63 forks source link

Cannot place MineCarts outside vanilla worldheight #495

Closed Boy0000 closed 8 months ago

Boy0000 commented 8 months ago

Info

Please provide the following information:

Train_Carts: v1.20.4-v1 (build: 1518) BKCommonLib: v1.20.4-v3 (build: 1673) Server: git-Paper-430 (MC: 1.20.4)


Bug

Description

When a server has a datapack that extends the worldheight past the vanilla range, minecarts cannot be placed outside of the vanilla range. For example a server extends the worldheight from 255->-64 to 255->-256 Any placement from -65 and below is cancelled

Expected behaviour

Place like normal

Actual behaviour

It is cancelled

Steps to reproduce

Add a datapack that extends the worldheight above or below the vanilla range. Try and place a minecart

Additional Information

Video clip:

https://github.com/bergerhealer/TrainCarts/assets/62521371/b3c8d88a-67e2-4972-8549-99b737516bf8

Datapack example:

deeper_world.zip

bergerkiller commented 8 months ago

Im wondering, can you place rails below y=-65? Do minecarts continue driving down the rails beyond this point or do they derail / error ?

Boy0000 commented 8 months ago

Rails can be placed below Minecart is killed ones it enters -65

https://github.com/bergerhealer/TrainCarts/assets/62521371/1f569ac9-8f11-4071-b94e-45bb1cc043a0

bergerkiller commented 8 months ago

Weird, this killing logic is set to be < worldBorderKillDistance (config.yml) which is set to 64 by default. This happens when the minecart is 64 blocks outside the world border (or height limits). This would mean it thinks the height limit is at y=0.

If you edit config.yml worldBorderKillDistance and set it to something high like 1000, does the minecart still get killed?

Boy0000 commented 8 months ago

Changing this setting does indeed fix both issues of placing minecarts and riding them 👍

bergerkiller commented 8 months ago

Id call it more a workaround than a fix, but good to know. That means this function is wrong.

https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/java/com/bergerkiller/bukkit/common/internal/logic/RegionHandler_Vanilla_1_17.java#L14

https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/templates/com/bergerkiller/templates/org/bukkit/world.txt#L20

Which is just a Bukkit method (World getMinHeight()). It might be a bug in the server itself?

bergerkiller commented 8 months ago

Can you share this datapack? Maybe its a configuration in the pack that is wrong, causing bukkit to misreport the minimum build height

Boy0000 commented 8 months ago

Shared in the original post From debugging the function that tracks this and kills it Im wondering if the logic for the worldborder stuff is wrong Bukkit should report it correctly as it seems to work in some of our own plugins, but ill ensure that it does

y: -62.895493668068866 | distSqr: 3955.843123750091
y: -63.04736968706467 | distSqr: 3974.9708244574012
y: -63.20660257800347 | distSqr: 3995.074609453675
y: -63.37317027026947 | distSqr: 4016.158710104566
y: -63.54705075945867 | distSqr: 4038.227660225217
y: -63.7282221071803 | distSqr: 4061.286292942104
y: -63.91666244085877 | distSqr: 4085.3397375786863
y: -64.11234995353621 | distSqr: 4110.393416564694
y: -64.3152629036756 | distSqr: 4136.453042368911
y: -64.52537961496458 | distSqr: 4163.524614455287
y: -64.74267847611969 | distSqr: 4191.614416262212
y: -64.96713794069137 | distSqr: 4220.729012204821
y: -65.19873652686931 | distSqr: 4250.8752447001225
y: -65.43745281728872 | distSqr: 4282.060231214887
y: -65.68326545883684 | distSqr: 4314.291361336029
y: -65.93615316246033 | distSqr: 4347.576293863427
y: -66.19609470297297 | distSqr: 4381.922953924966
y: -66.46306891886405 | distSqr: 4417.3395301136725
y: -66.73705471210747 | distSqr: 4453.8344716468255
y: -67.01803104797116 | distSqr: 4491.416485546827
bergerkiller commented 8 months ago

You can also try restarting the server, as it does cache the world border information. Idk if you did?

Boy0000 commented 8 months ago

worldHeightRange: -256..256 Worldheight is returned correctly from entity.getWorld().getMin/maxHeight() for the minecart

Boy0000 commented 8 months ago

You can also try restarting the server, as it does cache the world border information. Idk if you did?

Restarting returns the same result. Been an issue for a few weeks

bergerkiller commented 8 months ago

worldHeightRange: -256..256 Worldheight is returned correctly from entity.getWorld().getMin/maxHeight() for the minecart

Can you also print WorldUtil.getBlockBorder(world)? When printing it shows the min/max bounds of the total space, where the y coordinate is interesting.

Im thinking maybe the min y changes after the world initializes, but that a cached version remains in memory.

Boy0000 commented 8 months ago

{min={-29999984, 0, -29999984}, max={29999985, 256, 29999985}}

Boy0000 commented 8 months ago

so worldBorderKillDistance: 64 works purely because it happens to just cover this, which returns old pre 1.18 worldheight change to -64

bergerkiller commented 8 months ago

Just in case, do you see any errors at startup? This could be explained by the region handler failing to initialize, as y=0 is a default. It might help to print the value of RegionHandler.INSTANCE to know what implementation its using, which should be vanilla_1_17

Boy0000 commented 8 months ago

https://mclo.gs/ITCmpHn log seems to not contain any errors no RegionHandler.INSTANCE.getMinWorldHeight returns 0

bergerkiller commented 8 months ago

And the instance type?

Boy0000 commented 8 months ago

new RegionHandlerSelector().getHandler(getWorld()); returns com.bergerkiller.bukkit.common.internal.logic.RegionHandler_Vanilla_1_17@7f960c3f

Boy0000 commented 8 months ago

new RegionHandlerSelector().getHandler(getWorld()).getMinHeight(getWorld()); returns -256

so im guessing it caches the worldborder before it makes RegionInstance use 1.17 impl? thus no override applied

bergerkiller commented 8 months ago

I found the problem. Thanks for testing btw!

Its doing RegionHandler.INSTANCE.getMinHeight(world), which is calling it on the selector not the actual implementation. There is no getMiNheight and geTMaxHeight impl in the selector, so they stay the default 0.

bergerkiller commented 8 months ago

https://ci.mg-dev.eu/job/BKCommonLib/1680/ that should do it

Boy0000 commented 8 months ago

can confirm it is fixed. thanks for the help 👍