AllenSeitz / DimDungeons

A Minecraft mod which adds proceedurally generated dungeons.
14 stars 11 forks source link

Dungeons generate outside of the worldborder #28

Closed AfterRebelion closed 3 years ago

AfterRebelion commented 3 years ago

This issue was previously posted on https://www.curseforge.com/minecraft/mc-mods/dimensional-dungeons/issues, however it seems to have been removed since then

Versions: Minecraft: 1.16.5 Forge: 36.1.65 DimDungeons: 1.12 Other mods: None

On singleplayer, the mod works correctly, however on server, you can't interact with anything (Open chest, press buttons, etc) On the original bug report in CurseForge, it was noted that if you copy the server world to singleplayer, the issue can be reproduced. Uploaded the world so it can be tested: https://mega.nz/file/A4R0wJxb#v1iIILFFvbgk7JkG-pfmSVT5so-YWcgDXT7LeNARw_M Update: New world, generated in Singleplayer with WorldBorder, in DimDungeons 1.13: https://mega.nz/file/99hWTLAR#OW4rXNcodgvJoEK8BGIEPHOHJYO6PzrCsKBYxBcCijE

After several testing attempts, was found out that removing level.dat and letting it recreate fixed the issue. Done this on the server, and closed the original issue. However, after updating from DimDungeons 1.11 to DimDungeons 1.12, the same issue apeared again.

So I would ask for a permanent solution, as this is the only dimension mod I tested with this issue.

AllenSeitz commented 3 years ago

I finally found the cause of this issue. Do you have a worldborder set? When you are unable to interact with anything in the dungeon is there a red tint on the edges of your screen? (This indicates that you are outside of the worldborder.)

Unfortunately the worldborder setting is global, not per dimension. I wish Mojang would make it per dimension and I think there might be mods that do this? But in the meantime I have to plan for a worldborder being set. (This also is why deleting level.dat "fixes" the issue, and why the issue only seemed to strike at servers not single player games. I can't believe it took so many bug submissions to figure this out.)

I have a fix implemented in the current 1.13 beta. You're welcome to join my Discord and try it if you like. The way it works is by limiting the coordinates that dungeons can generate in, from 0 to 2 million blocks or the world border, whichever is smaller.

AfterRebelion commented 3 years ago

I confirm the worldborder, and now that you mention it, the red tint should have gave it right away, my bad.

After resetting the configs, got on the test world, generated a new key in the inscriber, however it failed to create a portal. There seems to be some kind of math error calculating the bounding (At commit https://github.com/AllenSeitz/DimDungeons/commit/955d1dc7054335033cb4a17c658f4315df511a94):


java.lang.IllegalArgumentException: bound must be positive
    at java.util.Random.nextInt(Random.java:388) ~[?:1.8.0_282] {}
    at com.catastrophe573.dimdungeons.structure.DungeonBuilderLogic.<init>(DungeonBuilderLogic.java:170) ~[?:?] {re:classloading}
    at com.catastrophe573.dimdungeons.structure.DungeonPlacementLogicBasic.place(DungeonPlacementLogicBasic.java:80) ~[?:?] {re:classloading}
    at com.catastrophe573.dimdungeons.utils.DungeonUtils.buildDungeon(DungeonUtils.java:158) ~[?:?] {re:classloading}
    at com.catastrophe573.dimdungeons.block.BlockPortalKeyhole.use(BlockPortalKeyhole.java:154) ~[?:?] {re:classloading}
    at net.minecraft.block.AbstractBlock$AbstractBlockState.use(AbstractBlock.java:638) ~[forge:?] {re:classloading}
    at net.minecraft.server.management.PlayerInteractionManager.useItemOn(PlayerInteractionManager.java:338) ~[forge:?] {re:classloading}
    at net.minecraft.network.play.ServerPlayNetHandler.handleUseItemOn(ServerPlayNetHandler.java:958) ~[forge:?] {re:classloading}
    at net.minecraft.network.play.client.CPlayerTryUseItemOnBlockPacket.handle(CPlayerTryUseItemOnBlockPacket.java:36) ~[forge:?] {re:classloading}
    at net.minecraft.network.play.client.CPlayerTryUseItemOnBlockPacket.handle(CPlayerTryUseItemOnBlockPacket.java:12) ~[forge:?] {re:classloading}
    at net.minecraft.network.PacketThreadUtil.lambda$ensureRunningOnSameThread$0(PacketThreadUtil.java:19) ~[forge:?] {re:classloading}
    at net.minecraft.util.concurrent.TickDelayedTask.run(TickDelayedTask.java:17) ~[forge:?] {re:classloading}
    at net.minecraft.util.concurrent.ThreadTaskExecutor.doRunTask(ThreadTaskExecutor.java:136) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.util.concurrent.RecursiveEventLoop.doRunTask(RecursiveEventLoop.java:22) ~[forge:?] {re:classloading}
    at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:733) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:159) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.util.concurrent.ThreadTaskExecutor.pollTask(ThreadTaskExecutor.java:109) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.server.MinecraftServer.pollTaskInternal(MinecraftServer.java:716) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.server.MinecraftServer.pollTask(MinecraftServer.java:710) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.util.concurrent.ThreadTaskExecutor.managedBlock(ThreadTaskExecutor.java:119) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.server.MinecraftServer.waitUntilNextTick(MinecraftServer.java:696) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:645) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:232) ~[forge:?] {re:classloading,pl:accesstransformer:B}
    at java.lang.Thread.run(Thread.java:748) [?:1.8.0_282] {}```
AllenSeitz commented 3 years ago

Don't feel bad. This bug has been reported by multiple people since launch for over a year. And none of them noticed either. And every time, the problem magically went away when I downloaded the modpack and used their server configs, or they switched to single player.

That second crash is caused by basicEntrances[] being empty in the common config. This is worrying because I've made many changes to the config files recently. I'll need to test this more before shipping my worldborder fix.

AllenSeitz commented 3 years ago

Omg the commit right before that is where I introduced the blank array in the basicEntrances. (While trying to fix another bug.) I haven't even uploaded a beta with that change yet so you must be building this mod yourself and testing. Thank you!

AfterRebelion commented 3 years ago

Tested on https://github.com/AllenSeitz/DimDungeons/commit/b5ffcd1601a5c6c65172e1789be5f15fa15d97f4, and the config error seems to be fixed However the worldborder issue seems to be still there, as I created a new key, and it was generated outside of the worldborder imagen I have a 20.000 worldborder.

AllenSeitz commented 3 years ago

Thank you. Someone in my Discord recently posted this:

"i took out all of yungs mods, better mineshafts, bettercaves, better strongholds, and extras as well as the api, created a new sp world, made a 10000 block boder, created 10 new dungeons.. all of them within the world border

added back in the yungs api, created a new sp world with a 10000 border, created 8 new duneons, all of them were outside the border."

Do you have Yung's api installed? Their mods were a compatibility thorn for me in Minecraft 1.14 and a second member chimed in to say that Yung also breaks Twilight Forest and Gaia presently in 1.16.

AfterRebelion commented 3 years ago

As indicated in the first post, even though I first detected this issue in a server with other mods, the testing done so far was with only Forge and DimDungeons installed. Later testing was done in a test environment in Intellij, to have a repo with the latest commits. I tested both with the original server world that had the issue, and a new save in singleplayer (It can be seen in the screenshot posted that I received the "Enter the Dungeon" archivement) I also completely remove old configs everytime i pull new commits.

AfterRebelion commented 3 years ago

Update: I know what is happening In your code, you use a get function to pass the limit to the key inscribing process:

// returns the limit of the dungeon space not in blocks, but in dungeon widths (BLOCKS_APART_PER_DUNGEON)
public static int getLimitOfWorldBorder(MinecraftServer server)
{
int block_limit = ItemPortalKey.RANDOM_COORDINATE_RANGE * ItemPortalKey.BLOCKS_APART_PER_DUNGEON;

// I know that the world border setting is global and affects all dimensions, but some mods change this
ServerWorld world = server.getLevel(DimDungeons.DUNGEON_DIMENSION);
double size = world.getWorldBorder().getSize() / 2;
if ( size < block_limit )
{
    return (int)(Math.round(size) / ItemPortalKey.BLOCKS_APART_PER_DUNGEON);
}

However, you do this directly requesting the DimDungeon dimension. If you set the world border in the Overworld, for example: imagen

This does not translate to the DimDungeon dimension, it has it's own worldborder value: imagen

My guess is some forge patch changed how worldborder settings are saved per-world, however is incomplete and it does not affect the border functionality itself.

AfterRebelion commented 3 years ago

Until it's fixed upstream, a workaround for any user affected would be: /execute in dimdungeons:dungeon_dimension run worldborder set <OVERWORLD_WORLDBORDER> I tested this, and works correctly. imagen

A workaround for the mod could be using the getter in the Overworld dimension, as it seems the one the worldborder functionality uses to generate the barrier.

AllenSeitz commented 3 years ago

Wow I did not know that Forge saved worldborders per dimension? I thought that had to be added with another mod.

Now what stumps me is that I never used the /worldborder set command in the dungeon dimension. In my testing I always set the world border in the overworld. So I should have been affected. I'm developing on (and testing with) Forge 36.1.0. Could that have something to do with it?

AllenSeitz commented 3 years ago

I just searched all of Forge's recent changelogs and I couldn't find "border" anywhere. But they did update to 36.2.2 just six days ago and there is a very relevant structure rotation fix for me in 1.51 so I'll update to 36.2.2 now and see what happens.

(But now I'm confused as to what the used who reported Yung's as the problem saw. Unless that person deleted everything and started a new server on a new version of Forge without knowing, maybe?)

AllenSeitz commented 3 years ago

So this just happened: (36.1.0) standing in overworld: /worldborder set 5730 enter the dungeon dimension: /worldborder get -> returns 5730 (update the project to 36.2.2) standing in overworld: /worldborder get -> returns 5730 enter the dungeon dimension: /worldborder get -> returns 60000000

It's like Forge 36.1 was saving the value per dimension all along, but then returning the overworld's value for any calls to get? And then in 36.2 the get command was changed to return whatever was saved to level.dat, even a previously inaccessible value written by 36.1.

I'm going to make a new config value called worldborderToRespect and have it default to minecraft:overworld. That way it works today, but it could also work tomorrow by just changing that one value. Because someone might have a Forge or Sponge mod that makes dimensional worldborders actually work. (Or maybe Forge 36.2.30 will do it, who knows.)

AllenSeitz commented 3 years ago

I think this also means that if Forge did correctly implement separate dimensions in the next minor version that server owners would get a rude surprise. Because I'm sure everyone set their worldborder once in one spot, either with the command or with a chunk pregen mod, and then just left it at that. Imagine if you thought your worldborder for The Nether and Twilight Forest was 10000 but then you update to Forge 36.2.41 (hypothetical) and suddenly your worldborder is 60000000. Lol oops. Sure it's easily fixable by teleporting to each dimension and setting a new worldborder immediately but uh

AllenSeitz commented 3 years ago

Thank you for your help! You've gone above and beyond.