Scope/world generation - #72
Conversation
its all coming together
Instead of deadlocking, what actually happens is that since synchronized() is reentrant, then calling get() from the same monitor would just reenter the lock, which would effectively recursively call until a StackOverflowError, not a deadlock while other monitors await the release. Updated comments to appropriately describe that instead of incorrectly classifying it as a classic deadlock.
Previously you had to manually define in each json file for each world what biomes and trees to include, and since there are tens to hundreds of biomes and a non-trivial handful of treetypes, this was wildly inefficient and required the server owner to both know all the biomes, write them out (which will make the json file unnecessarily long) and also the same for treetype which imo is ugly and bad. This system now dynamically infers treetype with zero per-world setup, and if you want to override a specific biome's trees (or anything else about it) you can write your own biome json (e.g. swamp.json) and point a world at it with a ref, though heads up the actual override logic in FeaturePopulator isn't wired up yet, just the schema/parser side. Additionally, we can now create custom logical biomes where we handle everything ourselves (terrain, surface, features) except invariable stuff like fog/ambient sound/mob tables, which just get mapped to whatever vanilla biome you pick for that.
…/world-generation
its all coming together
Instead of deadlocking, what actually happens is that since synchronized() is reentrant, then calling get() from the same monitor would just reenter the lock, which would effectively recursively call until a StackOverflowError, not a deadlock while other monitors await the release. Updated comments to appropriately describe that instead of incorrectly classifying it as a classic deadlock.
Previously you had to manually define in each json file for each world what biomes and trees to include, and since there are tens to hundreds of biomes and a non-trivial handful of treetypes, this was wildly inefficient and required the server owner to both know all the biomes, write them out (which will make the json file unnecessarily long) and also the same for treetype which imo is ugly and bad. This system now dynamically infers treetype with zero per-world setup, and if you want to override a specific biome's trees (or anything else about it) you can write your own biome json (e.g. swamp.json) and point a world at it with a ref, though heads up the actual override logic in FeaturePopulator isn't wired up yet, just the schema/parser side. Additionally, we can now create custom logical biomes where we handle everything ourselves (terrain, surface, features) except invariable stuff like fog/ambient sound/mob tables, which just get mapped to whatever vanilla biome you pick for that.
17350e8 to
9fd0baf
Compare
…rg/TotalFreedomMod into scope/world-generation
luke560
left a comment
luke560
left a comment
There was a problem hiding this comment.
The framework itself is great. Most of what I found isn't bad code, but there are some issues. Details in comments.
| catch (final ProfileException ex) | ||
| { | ||
| FLog.severe("Failed to load biome library: " + ExceptionUtils.getRootCauseMessage(ex)); | ||
| Bukkit.getPluginManager().disablePlugin(plugin); // we don't want to load TFM because no worlds can be loaded. |
There was a problem hiding this comment.
onStart runs inside the services.start() loop, which catches per-service and keeps going. So disablePlugin runs onDisable() immediately on the current stack and then the loop continues starting the remaining services against a torn down plugin.
I am also not confident that the premise holds: an empty library only breaks profiles that use "ref" to name a library biome, inline definitions and plain vanilla names parse fine. loadProfile already handles per-world failures and all three templates use plain vanilla names.
| */ | ||
| public Map<String, JsonObject> biomeLibrary() throws ProfileException | ||
| { | ||
| final Map<String, JsonObject> library = new HashMap<>(readBundled(WORLDS_DIRECTORY + "/" + BIOMES_DIRECTORY)); |
There was a problem hiding this comment.
worlds/biomes isnt in the jar; resources only has the three templates. Files.walk throws NoSuchFileException so this warns on every startup and reload, and nothing ships for "ref" to point at
|
|
||
| /** | ||
| * The Optional a profile's own {@link Shape.Caves#floodLevel} lives in, whatever mode the shape is. | ||
| * Flat is intentionally unused because |
| private void validateDestination(final PlayerMoveEvent event) | ||
| { | ||
| final World destination = event.getTo().getWorld(); | ||
| if (destination == null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| get(destination.getName()).validateMovement(event); | ||
| } |
There was a problem hiding this comment.
Movement in a world that TFM doesn't manage logs two lines and throws an exception on every movement event per player.
validateMovement calls getWorld() before reaching canAccessWorld. For an unprofiled world getWorld() never succeeds and it only caches success, so every movement re-runs generateWorld() in full. getSavedFlag(do_wipe_world) throws, the profile lookup comes back empty, then Flog.severe and Flog.warning.
| */ | ||
| private void archiveIfProfileChanged(final String currentFingerprint) | ||
| { | ||
| final File worldFolder = new File("./" + this.name); |
There was a problem hiding this comment.
World folders arent at ./<name> on paper 26, they are under <level>/dimensions/minecraft/<name>. Needs to be fixed here, writeProfileMarker (206), and wipeIfFlagged (237). Currently, the marker never writes, this returns early and never archives, and /manageworld wipe deletes nothing.
Also, the !marker.isFile() guard below means a world generated before this PR is never archived since it has no marker. My test server kept its old CleanroomGenerator flatlands (surface y49) while the profile now specifies y63, so new chunks seam 14 blocks against old ones. It is worth deciding whether no marker should mean "leave alone" or "archive".
Standing on the old y49 terrain, the wall is new chunks generating at y63 from the profile
| @Override | ||
| public Anchor anchor() | ||
| { | ||
| return Anchor.RANGE; |
There was a problem hiding this comment.
Lakes spawn in midair. Since Lake.anchor() returns RANGE, FeaturePopulator picks y from minY + random.nextInt(maxY - minY + 1) with no reference to the ground, and LakeFeature.place places the lake with no ground search.
| "lacunarity": 2.0, | ||
| "ridged": true | ||
| }, | ||
| "threshold": 0.48, |
There was a problem hiding this comment.
This carves about a third of the world away. Ridged noise is 1 - 2|n| per NoiseField:97, so it's highest where the raw noise is near zero. NoiseCarver:42 carves when sample > threshold, so 0.48 means |n| < 0.26. Values in this range are the common case, so a large portion of the world qualifies.
3 participants
Description will be provided upon finalization of the PR. We are making this now so all changes can be seen in one location rather than perusing every single commit in the history.