Give the gas planet surface its own background shader #51
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
A landed gas giant currently looks like a purple rock planet, because
GasSurfaceProfile.tresreuseslunar_surface.gdshaderand only recolours its uniforms. The crater field is still there - it reads as regolith, not atmosphere.Scope
gas_surface.gdshader, keeping the same uniform interface aslunar_surface.gdshadersoLunarBackground.gdcan feed it unchanged:camera_cell,camera_frac,camera_zoom,viewport_size,cell_size,base_color,crater_color,highlight_color.base_colorbecomes the deep atmosphere,crater_colorthe shadowed band,highlight_colorthe lit band crest - so the profile resource keeps meaning something.lunar_surface.gdshaderand inStarfield.gdfirst: absolute world coordinates reach the tens of thousands, where mobile float precision collapses and nohighphelps. Never reconstruct an absolute position in the shader.hash21/hash22formulation already inlunar_surface.gdshader(Dave Hoskins, all multipliers < 1). Do not introducesin()-based hashing - it is precisely what those functions exist to avoid.TIMEdirectly for the drift rather than adding a uniform, soLunarBackground.gdneeds no change at all.PlanetSurfaceProfilegains@export var background_shader: Shader(defaultnull, meaning "keep the scene's existing lunar shader").PlanetSurfaceController._compose_surface()assignsbackground.material.shaderwhen the profile names one, before setting the colour parameters.GasSurfaceProfile.tresat 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
bash tests/run_tests.sh atmosphereandrun_tests.sh gaspass.Tests
New
tests/test_surface_profiles.gd: the gas profile resolves a differentbackground_shaderthan the rocky profile,_compose_surface()actually applies it to the background material, and a nullbackground_shaderleaves the scene's own shader in place. Plus 720x1280 screenshots of both surfaces.Repo notes (read before starting)
/storage/emulated/0/GodotProjects/SpaceshipTest.touchon 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.bash tests/run_tests.sh <substring>; full suite with no argument. Tests areSceneTreescripts printingTEST_NAME: PASS/TEST_NAME: FAIL (reason)and callingquit(0|1)- copy the shape oftests/test_capture_points.gd.bash tests/screenshot.sh <res://Scene.tscn> <out.png> <frames> 720x1280. Use few frames (~15); the software rasterizer is slow.--headlessrenders nothing at all, so it cannot validate visuals.NeonDraw/NeonPiece; do not introduce a different visual idiom.*Definitionresources plus capability component scenes. Do not addif type == "..."branches.FileAccess, nouser://).GameStateis an in-memory autoload. Do not invent a save system.Traps that have already cost hours on this codebase
const X = preload(...)in a test file that is itself the-sentry point. It evaluates during that file's own early compilation, which can run before autoloads are registered; any script it pulls in that referencesGameStateinside a function body then fails to compile withIdentifier not found: GameState. Usevar X: Scriptplusload()inside_initialize()instead. See the header comment intests/test_capture_points.gd.[]to a typedArray[Resource]export from another script fails at runtime - the compiler only sees the target asResourceand cannot confirm the element type. Leave the declared default alone, or build the typed array inside the owning script.Fleet.spawn_ships(0, ...)does not empty the fleet - it falls back to one default interceptor by design. Clearfleet.shipsexplicitly if a test or preview needs genuinely zero ships.ship.positionwithout also settingship.move_targetletsShip._physics_processsteer it straight back toward the stale target within a few frames.SceneTreetest'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 onelapsed >= duration.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._screenshot_tool.gd's_initialize(), so their_ready()races the game scene's own_ready(). Do setup on the first_process()tick, andget_tree().paused = trueonce everything is placed if the scene must hold still.Done in commit
4606c34, pushed to main.New
gas_surface.gdshaderreplaces the recolouredlunar_surface.gdshaderfor 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), soLunarBackground.gdneeded no changes.PlanetSurfaceProfilegainsbackground_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.trespoints at the new shader and retunes its three colours to a deep-atmosphere/shadowed-band/lit-crest read.RockySurfaceProfile.tresis untouched, so the rocky surface renders exactly as before - proven both by the new test (sameShaderresource 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.gduse 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 nobackground_shaderand 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:
class_nameclass by its bare global name from a-sentry-point test script -load()it by path into a local var instead, same asPlanetSurfaceController.gd/PlanetSetups.gdalready do. I hit this directly: referencingPlanetSurfaceProfiles.get_profile(...)by bare name didn't fail to compile - it corrupted resource-loading state badly enough that an unrelated scene'sRock.tscnlost its script attachment andRockField'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.tests/screenshot.shrender (genuinely minutes for a crater-shader scene on softpipe) at the same time as arun_tests.shfull-suite run starves both for CPU on this device - kill one before waiting on the other.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.gdcallsEnemyDefinitions.get_definition(...)by bare class name from_initialize()and passes reliably, and I confirmed a fresh isolated probe doing the same (bareEnemyDefinitions+ loadingPlanetSurface.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 ofPlanetSurface.tscn's own tree, already doesconst SURFACE_PROFILES = preload("res://PlanetSurfaceProfiles.gd")). ReferencingPlanetSurfaceProfilesbare right before instantiatingPlanetSurface.tscncollides with that; referencingEnemyDefinitionsbare doesn't, because nothing inPlanetSurface.tscn's subtree preloadsEnemyDefinitions.gdby 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.