Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server Thread freezes upon using ChunkGenerator.locateStructure() with "minecraft:fortress" internally #14

Open
JustS-js opened this issue Apr 19, 2024 · 0 comments

Comments

@JustS-js
Copy link

JustS-js commented Apr 19, 2024

I was investigating an issue with Watchdog on my friends modded smp and found one related to your mod.
We play on 1.19.2 Fabric and use Origin and Origin++. When a player with specified "power" (ModifyPlayerSpawnPower.getStructureLocation() from @Apoli library) requests respawning in Nether Fortress, game tries to locate that structure internally with specified structure id, which, if not modified by server manager, is set to vanilla "minecraft:fortress".

If "YUNG's Better Fortresses" is installed on the server and if vanilla fortresses are disabled by config, server will freeze while trying to locate "minecraft:fortress", which usually leads to server thread being terminated by Watchdog.

If i am not mistaken, the problem lies in the method "ChunkGenerator.locateStructure()" (YARN mappings), as it is poorly designed and ineffective by itself. Basically, it spawns a ton of CompletableFutures, which are awaited to join() on server thread. Inside those futures they are trying to read RegionFiles, which get blocked by IO, thus generating giant time consuming workload.
At least that's how i understand the issue, feel free to correct me.

The best solution would be to rewrite BetterFortresses's Structure Registration, so it would substitute the original "minecraft:fortress" if it is disabled by config.
For example, by Mixin Injection inside "ChunkGenerator.locateStructure()", where said substitution could be performed.

For anyone who could be experiencing the same problem, your solution would be either:

  1. Enabling vanilla fortresses spawning
  2. Rewriting your mod/datapack/thingy-that-requests-fortress in a way so it uses "betterfortresses:fortress" id instead of "minecraft:fortress"
  3. Writing something similar to my dynamic "id replacing" (warning: ugly)
...
// ModifyPlayerSpawnPower.getStructureLocation() compatability with BetterFortresses (only if vanilla fortresses are disabled)
if (
    structure != null &&
    structure.getValue().toString().contains("minecraft:fortress") &&
    FabricLoader.getInstance().isModLoaded("betterfortresses")
) {
    Apoli.LOGGER.warn("Trying to respawn player in \"minecraft:fortress\", but should be trying \"betterfortresses:fortress\"");
    structure = registry.getKey(registry.get(new Identifier("betterfortresses", "fortress"))).orElse(null);
}
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant