Give the gas planet surface its own background shader #51

Closed
opened 2026-08-15 20:29:58 +00:00 by metaheavens · 2 comments
Owner

A landed gas giant currently looks like a purple rock planet, because GasSurfaceProfile.tres reuses lunar_surface.gdshader and only recolours its uniforms. The crater field is still there - it reads as regolith, not atmosphere.

Scope

  1. New gas_surface.gdshader, keeping the same uniform interface as lunar_surface.gdshader so LunarBackground.gd can feed it unchanged: camera_cell, camera_frac, camera_zoom, viewport_size, cell_size, base_color, crater_color, highlight_color.
    • Render horizontal banded cloud layers with slow lateral drift and some turbulence, not craters. base_color becomes the deep atmosphere, crater_color the shadowed band, highlight_color the lit band crest - so the profile resource keeps meaning something.
    • Keep the camera cell/frac split. Read the long note at the top of lunar_surface.gdshader and in Starfield.gd first: absolute world coordinates reach the tens of thousands, where mobile float precision collapses and no highp helps. Never reconstruct an absolute position in the shader.
    • Reuse the hash21/hash22 formulation already in lunar_surface.gdshader (Dave Hoskins, all multipliers < 1). Do not introduce sin()-based hashing - it is precisely what those functions exist to avoid.
    • Use TIME directly for the drift rather than adding a uniform, so LunarBackground.gd needs no change at all.
  2. PlanetSurfaceProfile gains @export var background_shader: Shader (default null, meaning "keep the scene's existing lunar shader"). PlanetSurfaceController._compose_surface() assigns background.material.shader when the profile names one, before setting the colour parameters.
  3. Point GasSurfaceProfile.tres at the new shader and retune its three colours for the banded look.

Performance constraint

Screenshots go through Mesa softpipe (llvmpipe SIGILLs in this environment). lunar_surface.gdshader's 5x5 neighbour loop already takes minutes to render a frame at 1280x720. Keep the gas shader cheaper than that - banded noise should be far cheaper than a cellular distance field, so this is achievable, but verify a screenshot actually completes rather than assuming.

Out of scope

GasGiantVisual (the space-side planet body) - it is already visually distinct.

Acceptance criteria

  • The landed gas surface shows drifting banded cloud layers with no craters.
  • The landed rocky surface is unchanged - same shader, same appearance.
  • Both render at 720x1280 within the screenshot harness.
  • bash tests/run_tests.sh atmosphere and run_tests.sh gas pass.

Tests

New tests/test_surface_profiles.gd: the gas profile resolves a different background_shader than the rocky profile, _compose_surface() actually applies it to the background material, and a null background_shader leaves the scene's own shader in place. Plus 720x1280 screenshots of both surfaces.

Repo notes (read before starting)

  • Godot 4.7.1, portrait 720x1280, touch UI. Project dir: /storage/emulated/0/GodotProjects/SpaceshipTest.
  • After editing any project file, run touch on it. Android FUSE does not bump mtime under ~/storage/shared, and Godot's import cache will silently keep serving the old version, making a correct fix look like it did nothing.
  • Headless tests: bash tests/run_tests.sh <substring>; full suite with no argument. Tests are SceneTree scripts printing TEST_NAME: PASS / TEST_NAME: FAIL (reason) and calling quit(0|1) - copy the shape of tests/test_capture_points.gd.
  • Real screenshots: bash tests/screenshot.sh <res://Scene.tscn> <out.png> <frames> 720x1280. Use few frames (~15); the software rasterizer is slow. --headless renders nothing at all, so it cannot validate visuals.
  • Drawing goes through NeonDraw / NeonPiece; do not introduce a different visual idiom.
  • The repo has no type switches (tickets #31-#35): behaviour comes from *Definition resources plus capability component scenes. Do not add if type == "..." branches.
  • There is no persistence layer (no FileAccess, no user://). GameState is an in-memory autoload. Do not invent a save system.
  • Commit this ticket separately and run the affected test groups plus the full suite before pushing.

Traps that have already cost hours on this codebase

  1. Never put a top-level const X = preload(...) in a test file that is itself the -s entry point. It evaluates during that file's own early compilation, which can run before autoloads are registered; any script it pulls in that references GameState inside a function body then fails to compile with Identifier not found: GameState. Use var X: Script plus load() inside _initialize() instead. See the header comment in tests/test_capture_points.gd.
  2. Assigning a plain [] to a typed Array[Resource] export from another script fails at runtime - the compiler only sees the target as Resource and cannot confirm the element type. Leave the declared default alone, or build the typed array inside the owning script.
  3. Fleet.spawn_ships(0, ...) does not empty the fleet - it falls back to one default interceptor by design. Clear fleet.ships explicitly if a test or preview needs genuinely zero ships.
  4. Setting ship.position without also setting ship.move_target lets Ship._physics_process steer it straight back toward the stale target within a few frames.
  5. Physics ticks can run ahead of a SceneTree test's own _process() clock under device load, even with --fixed-fps 60. Gate waits on the system's own state (a component's timer or flag), not on elapsed >= duration.
  6. A node that queue_free()s itself (projectiles, shells) must not be re-read inside a per-frame wait condition - cache the scalar you need before the wait.
  7. Preview scenes for screenshots are added to the tree from inside _screenshot_tool.gd's _initialize(), so their _ready() races the game scene's own _ready(). Do setup on the first _process() tick, and get_tree().paused = true once everything is placed if the scene must hold still.
A landed gas giant currently looks like a purple rock planet, because `GasSurfaceProfile.tres` reuses `lunar_surface.gdshader` and only recolours its uniforms. The crater field is still there - it reads as regolith, not atmosphere. ### Scope 1. New `gas_surface.gdshader`, keeping the **same uniform interface** as `lunar_surface.gdshader` so `LunarBackground.gd` can feed it unchanged: `camera_cell`, `camera_frac`, `camera_zoom`, `viewport_size`, `cell_size`, `base_color`, `crater_color`, `highlight_color`. - Render horizontal banded cloud layers with slow lateral drift and some turbulence, not craters. `base_color` becomes the deep atmosphere, `crater_color` the shadowed band, `highlight_color` the lit band crest - so the profile resource keeps meaning something. - **Keep the camera cell/frac split.** Read the long note at the top of `lunar_surface.gdshader` and in `Starfield.gd` first: absolute world coordinates reach the tens of thousands, where mobile float precision collapses and no `highp` helps. Never reconstruct an absolute position in the shader. - Reuse the `hash21`/`hash22` formulation already in `lunar_surface.gdshader` (Dave Hoskins, all multipliers < 1). Do **not** introduce `sin()`-based hashing - it is precisely what those functions exist to avoid. - Use `TIME` directly for the drift rather than adding a uniform, so `LunarBackground.gd` needs no change at all. 2. `PlanetSurfaceProfile` gains `@export var background_shader: Shader` (default `null`, meaning "keep the scene's existing lunar shader"). `PlanetSurfaceController._compose_surface()` assigns `background.material.shader` when the profile names one, before setting the colour parameters. 3. Point `GasSurfaceProfile.tres` at the new shader and retune its three colours for the banded look. ### Performance constraint Screenshots go through Mesa **softpipe** (llvmpipe SIGILLs in this environment). `lunar_surface.gdshader`'s 5x5 neighbour loop already takes minutes to render a frame at 1280x720. Keep the gas shader cheaper than that - banded noise should be far cheaper than a cellular distance field, so this is achievable, but verify a screenshot actually completes rather than assuming. ### Out of scope `GasGiantVisual` (the space-side planet body) - it is already visually distinct. ### Acceptance criteria - The landed gas surface shows drifting banded cloud layers with no craters. - The landed rocky surface is **unchanged** - same shader, same appearance. - Both render at 720x1280 within the screenshot harness. - `bash tests/run_tests.sh atmosphere` and `run_tests.sh gas` pass. ### Tests New `tests/test_surface_profiles.gd`: the gas profile resolves a different `background_shader` than the rocky profile, `_compose_surface()` actually applies it to the background material, and a null `background_shader` leaves the scene's own shader in place. Plus 720x1280 screenshots of both surfaces. ### Repo notes (read before starting) - Godot 4.7.1, portrait 720x1280, touch UI. Project dir: `/storage/emulated/0/GodotProjects/SpaceshipTest`. - **After editing any project file, run `touch` on it.** Android FUSE does not bump mtime under `~/storage/shared`, and Godot's import cache will silently keep serving the old version, making a correct fix look like it did nothing. - Headless tests: `bash tests/run_tests.sh <substring>`; full suite with no argument. Tests are `SceneTree` scripts printing `TEST_NAME: PASS` / `TEST_NAME: FAIL (reason)` and calling `quit(0|1)` - copy the shape of `tests/test_capture_points.gd`. - Real screenshots: `bash tests/screenshot.sh <res://Scene.tscn> <out.png> <frames> 720x1280`. Use few frames (~15); the software rasterizer is slow. `--headless` renders nothing at all, so it cannot validate visuals. - Drawing goes through `NeonDraw` / `NeonPiece`; do not introduce a different visual idiom. - The repo has **no type switches** (tickets #31-#35): behaviour comes from `*Definition` resources plus capability component scenes. Do not add `if type == "..."` branches. - There is **no persistence layer** (no `FileAccess`, no `user://`). `GameState` is an in-memory autoload. Do not invent a save system. - Commit this ticket separately and run the affected test groups plus the full suite before pushing. ### Traps that have already cost hours on this codebase 1. **Never put a top-level `const X = preload(...)` in a test file that is itself the `-s` entry point.** It evaluates during that file's own early compilation, which can run before autoloads are registered; any script it pulls in that references `GameState` inside a function body then fails to compile with `Identifier not found: GameState`. Use `var X: Script` plus `load()` inside `_initialize()` instead. See the header comment in `tests/test_capture_points.gd`. 2. **Assigning a plain `[]` to a typed `Array[Resource]` export from another script fails at runtime** - the compiler only sees the target as `Resource` and cannot confirm the element type. Leave the declared default alone, or build the typed array inside the owning script. 3. **`Fleet.spawn_ships(0, ...)` does not empty the fleet** - it falls back to one default interceptor by design. Clear `fleet.ships` explicitly if a test or preview needs genuinely zero ships. 4. **Setting `ship.position` without also setting `ship.move_target`** lets `Ship._physics_process` steer it straight back toward the stale target within a few frames. 5. **Physics ticks can run ahead of a `SceneTree` test's own `_process()` clock** under device load, even with `--fixed-fps 60`. Gate waits on the system's own state (a component's timer or flag), not on `elapsed >= duration`. 6. **A node that `queue_free()`s itself** (projectiles, shells) must not be re-read inside a per-frame wait condition - cache the scalar you need before the wait. 7. **Preview scenes for screenshots** are added to the tree from inside `_screenshot_tool.gd`'s `_initialize()`, so their `_ready()` races the game scene's own `_ready()`. Do setup on the first `_process()` tick, and `get_tree().paused = true` once everything is placed if the scene must hold still.
Author
Owner

Done in commit 4606c34, pushed to main.

New gas_surface.gdshader replaces the recoloured lunar_surface.gdshader for gas profiles only: a nearest-row-wins horizontal band field (same single-scalar discipline as lunar's nearest-crater field), with drifting per-row turbulence so the bands ripple rather than reading as ruled lines. Keeps the exact same uniform interface as lunar (camera_cell/camera_frac/camera_zoom/viewport_size/cell_size/base_color/crater_color/highlight_color), so LunarBackground.gd needed no changes.

PlanetSurfaceProfile gains background_shader (default null = keep the scene's existing lunar shader); PlanetSurfaceController._compose_surface() swaps the background material's shader in only when a profile names one. GasSurfaceProfile.tres points at the new shader and retunes its three colours to a deep-atmosphere/shadowed-band/lit-crest read. RockySurfaceProfile.tres is untouched, so the rocky surface renders exactly as before - proven both by the new test (same Shader resource identity) and by the fact that nothing in the rocky code path was touched.

Precision: every hash input is either an exact integer cell/row index or a bounded [0,1) local fraction, the same split lunar_surface.gdshader/Starfield.gd use and for the same reason. TIME-based lateral drift is folded into the lattice coordinate via the same floor/remainder trick rather than ever added to a large cell index directly.

Tests: new tests/test_surface_profiles.gd - rocky resolves no background_shader and keeps the lunar shader; gas resolves and applies its own, different from lunar's. run_tests.sh atmosphere, run_tests.sh gas, and the full suite (78/78) all pass.

Screenshot of the gas surface (shots/gas_surface_banded.png) confirms drifting bands with no craters, well inside the performance budget: 20 frames rendered in ~2m40s on Mesa softpipe, versus lunar's "minutes per frame" baseline the ticket warned about.

Two things worth keeping in mind for later tickets:

  1. Never reference a plain (non-autoload) class_name class by its bare global name from a -s entry-point test script - load() it by path into a local var instead, same as PlanetSurfaceController.gd/PlanetSetups.gd already do. I hit this directly: referencing PlanetSurfaceProfiles.get_profile(...) by bare name didn't fail to compile - it corrupted resource-loading state badly enough that an unrelated scene's Rock.tscn lost its script attachment and RockField's spawn loop exhausted the physics server's RID pool. If a crash trace implicates code you never touched, check every bare global class_name reference in the test first.
  2. Running a tests/screenshot.sh render (genuinely minutes for a crater-shader scene on softpipe) at the same time as a run_tests.sh full-suite run starves both for CPU on this device - kill one before waiting on the other.
Done in commit 4606c34, pushed to main. New `gas_surface.gdshader` replaces the recoloured `lunar_surface.gdshader` for gas profiles only: a nearest-row-wins horizontal band field (same single-scalar discipline as lunar's nearest-crater field), with drifting per-row turbulence so the bands ripple rather than reading as ruled lines. Keeps the exact same uniform interface as lunar (`camera_cell`/`camera_frac`/`camera_zoom`/`viewport_size`/`cell_size`/`base_color`/`crater_color`/`highlight_color`), so `LunarBackground.gd` needed no changes. `PlanetSurfaceProfile` gains `background_shader` (default null = keep the scene's existing lunar shader); `PlanetSurfaceController._compose_surface()` swaps the background material's shader in only when a profile names one. `GasSurfaceProfile.tres` points at the new shader and retunes its three colours to a deep-atmosphere/shadowed-band/lit-crest read. `RockySurfaceProfile.tres` is untouched, so the rocky surface renders exactly as before - proven both by the new test (same `Shader` resource identity) and by the fact that nothing in the rocky code path was touched. Precision: every hash input is either an exact integer cell/row index or a bounded [0,1) local fraction, the same split `lunar_surface.gdshader`/`Starfield.gd` use and for the same reason. TIME-based lateral drift is folded into the lattice coordinate via the same floor/remainder trick rather than ever added to a large cell index directly. Tests: new `tests/test_surface_profiles.gd` - rocky resolves no `background_shader` and keeps the lunar shader; gas resolves and applies its own, different from lunar's. `run_tests.sh atmosphere`, `run_tests.sh gas`, and the full suite (78/78) all pass. Screenshot of the gas surface (`shots/gas_surface_banded.png`) confirms drifting bands with no craters, well inside the performance budget: 20 frames rendered in ~2m40s on Mesa softpipe, versus lunar's "minutes per frame" baseline the ticket warned about. Two things worth keeping in mind for later tickets: 1. Never reference a plain (non-autoload) `class_name` class by its bare global name from a `-s` entry-point test script - `load()` it by path into a local var instead, same as `PlanetSurfaceController.gd`/`PlanetSetups.gd` already do. I hit this directly: referencing `PlanetSurfaceProfiles.get_profile(...)` by bare name didn't fail to compile - it corrupted resource-loading state badly enough that an unrelated scene's `Rock.tscn` lost its script attachment and `RockField`'s spawn loop exhausted the physics server's RID pool. If a crash trace implicates code you never touched, check every bare global class_name reference in the test first. 2. Running a `tests/screenshot.sh` render (genuinely minutes for a crater-shader scene on softpipe) at the same time as a `run_tests.sh` full-suite run starves both for CPU on this device - kill one before waiting on the other.
Author
Owner

Correction to the repo note above, after re-testing: the guidance as I wrote it ("never reference a plain class_name class by its bare global name from a -s script") is overgeneralized and contradicted by existing code - tests/test_enemy_composition.gd calls EnemyDefinitions.get_definition(...) by bare class name from _initialize() and passes reliably, and I confirmed a fresh isolated probe doing the same (bare EnemyDefinitions + loading PlanetSurface.tscn) is fine too.

The real, narrower trigger, isolated by a further probe: it's specifically when the SAME script gets loaded two different ways in close succession within one process - a bare global-class-name reference AND a preload("res://Script.gd") by path elsewhere (here, PlanetSurfaceController.gd, which is part of PlanetSurface.tscn's own tree, already does const SURFACE_PROFILES = preload("res://PlanetSurfaceProfiles.gd")). Referencing PlanetSurfaceProfiles bare right before instantiating PlanetSurface.tscn collides with that; referencing EnemyDefinitions bare doesn't, because nothing in PlanetSurface.tscn's subtree preloads EnemyDefinitions.gd by path.

So the safe rule is narrower than I originally wrote: bare global class_name references are fine in general (matches existing test conventions like WorldGen.has_planet_at_chunk, EnemyDefinitions.get_definition). Only worth avoiding - preload by path into a local instead - when the SAME script is also preloaded by path somewhere in a scene your test is about to instantiate in that same process. If a future test crashes in code you didn't touch, that's the specific thing to check, not bare class-name usage in general.

Correction to the repo note above, after re-testing: the guidance as I wrote it ("never reference a plain class_name class by its bare global name from a -s script") is overgeneralized and contradicted by existing code - `tests/test_enemy_composition.gd` calls `EnemyDefinitions.get_definition(...)` by bare class name from `_initialize()` and passes reliably, and I confirmed a fresh isolated probe doing the same (bare `EnemyDefinitions` + loading `PlanetSurface.tscn`) is fine too. The real, narrower trigger, isolated by a further probe: it's specifically when the SAME script gets loaded two different ways in close succession within one process - a bare global-class-name reference AND a `preload("res://Script.gd")` by path elsewhere (here, `PlanetSurfaceController.gd`, which is part of `PlanetSurface.tscn`'s own tree, already does `const SURFACE_PROFILES = preload("res://PlanetSurfaceProfiles.gd")`). Referencing `PlanetSurfaceProfiles` bare right before instantiating `PlanetSurface.tscn` collides with that; referencing `EnemyDefinitions` bare doesn't, because nothing in `PlanetSurface.tscn`'s subtree preloads `EnemyDefinitions.gd` by path. So the safe rule is narrower than I originally wrote: bare global class_name references are fine in general (matches existing test conventions like `WorldGen.has_planet_at_chunk`, `EnemyDefinitions.get_definition`). Only worth avoiding - preload by path into a local instead - when the SAME script is also preloaded by path somewhere in a scene your test is about to instantiate in that same process. If a future test crashes in code you didn't touch, that's the specific thing to check, not bare class-name usage in general.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
metaheavens/spaceship#51
No description provided.