feat: per-map horror fog - #208
Open
TheMeinerLP wants to merge 11 commits into
Open
Conversation
The seven fog and sky values move into a new DimensionAtmosphere interface; DimensionPreset now only adds the key on top of it. That lets a per-map atmosphere reuse the same values without inventing a preset name for itself. DimensionFactory gains create(Key, DimensionAtmosphere) so a caller can supply the registry key, and drops the hardcoded SOUL ambient particles: they belonged to one specific look, and would otherwise apply to every map that gets its own dimension. registerAll() and the -Dcygnus.customDimension flag are gone. They registered all twelve presets globally while every instance still ran on OVERWORLD, so nothing of it ever reached a player.
Holds the atmosphere of a single map, keyless because the registry key comes from the map it belongs to. Out-of-range values are clamped and logged rather than thrown: map.json is hand-edited often enough that a typo is a matter of when, and a service refusing to boot over a fog distance is worse than one booting with a corrected value. Components are Color rather than RGBLike so Gson can deserialize them without being told which implementation to pick.
Gson's default turns a Color into an object of three numeric channels, which is unreadable in a file map builders edit by hand and makes noisy diffs. Reading tolerates lower case and a missing hash, since both turn up when values are pasted from a color picker.
GameMap gains a nullable atmosphere, read from an "atmosphere" block by GameMapAdapter. A map file without one keeps running on OVERWORLD, so every existing map is unaffected. The field has to live on GameMap rather than beside it: the setup module rewrites map.json in full from GameMap, and anything stored outside it would be dropped on every save.
GameMapProvider reads the game map in its constructor and registers cygnus:map/<name> from the map's atmosphere there. That timing is the point: registry data only reaches a client during its configuration phase, and the provider is built before the server accepts connections. Registering in loadGameMap() instead would leave everyone already in the lobby without the dimension they are about to be moved into. Also fixes DimensionFactory reaching for DimensionType.OVERWORLD .asValue(), which answers null without a registry to resolve against. The clock is now looked up through the registry, and a failed lookup falls back to the builder default instead of refusing to register.
Adds an ATMOSPHERE category with its own slot, reporting the two numbers a builder actually judges a map by - sight distance and fog color - rather than all seven values. Right-click clears the atmosphere, which puts the map back on the overworld look. The new category goes last in MapDataCategory: InventoryMode holds its categories in an EnumSet, so declaration order decides which slot each one lands in, and appending leaves the existing four untouched.
Pica only offers two-button confirm dialogs, so the flow is a chain rather than one dialog with three buttons: pick a preset, adjust the values, preview, then save or go back and adjust. The preset is only a starting point - what is stored is the resolved set of values, so a map keeps its look even if the preset it started from changes later. AtmospherePreviewService registers a throwaway dimension and walks the builder through a configuration phase into a throwaway instance on the same world directory, because an instance's dimension is fixed at creation and the environment only reaches a client with the registry data sent during configuration. It stays a separate instance on purpose: the setup instance owns live state, an open FalcoAnvilLoader among it, that a failed swap would corrupt. Preview instances are torn down by a task that waits for them to empty, since Minestom refuses to unregister an instance that still holds players and the move out does not finish within the call. The dimension registry entries cannot be taken back - removal is gated behind -Dminestom.registry.unsafe-ops, which is not worth enabling server-wide to tidy up after a dialog - so a noisy session gets a warning instead.
…n phases Returning from a configuration phase looks like a first spawn to Minestom: the player holds no instance while in it, so setInstance sees no previous one and reports firstSpawn. Left alone, the setup's spawn handling would read every preview as a fresh join and teleport the builder to the hub, resetting their items on the way. The session now also outlives the move back out. Until that configuration phase completes the service still has to answer where the builder belongs, or the hub listener claims them instead of their own setup instance. A preview also starts where the builder is standing rather than where the first one did, so comparing two sets of values means comparing the same view twice. Adds the design doc and implementation plan this work followed.
Contributor
Test results237 files 237 suites 1m 24s ⏱️ For more details on these failures, see this check. Results for commit 9d19074. ♻️ This comment has been updated with latest results. |
TheMeinerLP
force-pushed
the
feat/per-map-horror-fog
branch
from
August 30, 2026 15:23
5989e3c to
1ed2228
Compare
A CloudNet service runs one map, and the fog it came up with is otherwise invisible until someone joins and looks at it. Logging the registered dimension key alongside the fog color and distances makes a misread or missing atmosphere block obvious from the service log.
Moving the teams into the game map is asynchronous - Player#setInstance completes when the target chunks have loaded - and since the game map now runs on its own dimension, that move also costs a full respawn. The caller in Cygnus#initPhases releases the lobby one tick after kicking the teleport off, which is nowhere near enough: unregistering an instance that still holds players throws, and the exception surfaced on a real server during the map switch. releasePreviousInstance now unregisters immediately when the instance is already empty and otherwise waits for it, so the fix holds regardless of when the caller fires. The regression test reproduces the original failure: without this change it throws exactly as the server did. Found by running a local server against a real map rather than by the test suite, which only ever called this after polling players into place.
Minestom registers AMBIENT_LIGHT_COLOR with a black default and nothing was setting it, so a lightmap cell with neither block nor sky light came out as exactly vec3(0.0). A shader that normalises by the largest channel then computes 0.0 / 0.0, and the resulting NaN renders unlit surfaces bright instead of dark - the pale streaks seen on a tree trunk in game. Vanilla never reaches that case because it always has some ambient light to fall back on. The colour is now the eighth value of an atmosphere, editable per map alongside the others. A map file that leaves it out gets a dimmed version of its own fog rather than black, so the darkness carries the same tint as the haze, and ColorUtil.dim floors every channel at one: the guard belongs where the value is built, not where it is read. The twelve presets derive it the same way instead of declaring twelve hand-picked colours. Fixing this server-side does not replace the shader guard - a shader should not produce NaN whatever the server sends - but it stops the input that triggered it from occurring.
Contributor
Job Summary for GradleBuild PR :: build
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Every map can now carry its own atmosphere — fog color, fog distances, sky and sky-light colors — set by a builder in the setup module and stored in the map's
map.json.The preset system in
common/.../dimension/already existed but never reached a player:CygnusLoaderregistered all twelve presets globally behind-Dcygnus.customDimension, whileGameMapProviderandInstanceSetupDatacreated every instance onDimensionType.OVERWORLD. This wires it through and replaces the global registration with one dimension per map.How it works
DimensionAtmosphereholds the seven values;DimensionPresetnow only adds a key on top of it, andMapAtmosphereis the keyless per-map implementation.map.jsongains an optionalatmosphereblock with hex colors. Without it a map stays onOVERWORLD, so every existing map is unaffected.GameMapProviderreads the game map in its constructor and registerscygnus:map/<name>there. That timing matters: registry data only reaches a client during its configuration phase, and the provider is built before the server accepts connections.Types of changes
The bugfix:
DimensionFactoryreached forDimensionType.OVERWORLD.asValue(), which answersnullwithout a registry to resolve against. It would have thrown an NPE the first time a custom dimension was actually built. The clock is now resolved through the registry, with the builder default as fallback.Checklist
Tests:
MapAtmosphereTest(value clamping),ColorGsonAdapterTest(hex round-trip),GameMapAdapterTest(atmosphere present/absent/round-trip),GameMapProviderIntegrationTest(instance runs on the per-map dimension, and stays onOVERWORLDwithout one),AtmospherePreviewServiceTest(the preview state machine across both configuration phases).Further comments
Why static per map rather than fog that reacts to the game. Dynamic fog means changing a player's dimension at runtime, which costs a configuration phase plus a chunk reload every time.
TunnelVisionServiceandSlenderGazeServicealready provide reactive visibility without any of that.Two deliberate limitations.
The lobby is out of scope: it loads as an Aves
BaseMapand is saved throughLobbyData, so an atmosphere field would mean changing a class we do not own. A bright lobby against a foggy game map is also the contrast we want.A preview costs two loading screens, and setup markers are not visible inside it. The preview deliberately runs in its own instance rather than rebuilding the setup instance around a new dimension — the setup instance owns live state, an open
FalcoAnvilLoaderamong it, that a failed swap would corrupt.One thing worth a second opinion: each preview holds a dimension registry entry for the life of the server.
DynamicRegistry.removeis gated behind-Dminestom.registry.unsafe-ops, which did not seem worth enabling server-wide just to tidy up after a dialog. Entries are a few hundred bytes, and a session that renders 50 previews gets a warning. Happy to revisit if that trade looks wrong.Design doc and implementation plan are included under
docs/superpowers/.