Animate thruster flame

[fluffyfreak]

This is a resurrection of #6056 by possibly "jeffmd", it seemed a shame to lose the work with a nice visual effect

The thrust flame is now animated by randomly expanding/contracting the flame and by making small changes in the flames diffuse color and alpha each frame. Its simplistic but for now adds some life to the ship when viewed externally.

Gifs by @bszlrd from that original PR

[impaktor]

Good eye-candy to have in the release notes next year.

[ghost]

I didn't resubmit this because I had used AI to generate most of the code back in Jan/Feb 2025 and in March noticed the AI clause in the README.md : https://github.com/pioneerspacesim/pioneer/blob/master/README.md#ai-policy.

[sturnclaw]

@jeffmd For the record, our policy on no AI content is mostly aimed at people who have neither the knowledge nor the desire to learn what their code actually does, and behave as though a subscription to a large language model makes them a source of indisputable oracular wisdom.

We specifically have a carveout in that policy for programmers who wish to use an LLM as an assistive tool, and aren't just copy-pasting AI-generated code verbatim into the Github interface. AI output that has been modified to a sufficient degree that it can be considered a transformative work is acceptable, though as the following review comments may show, the use of AI can cause more work to fix up its output to a mergeable state than it's actually worth.

@fluffyfreak There's some weirdness regarding the m_renderData.timeStep = 0.f statements inside the Model::Render() functions. I'm not a great fan of whatever is going on there regarding persisted state, and I'd recommend a closer audit of the behavior inside the frame.

In general, storing a time value per-thruster is an anti-pattern and I'd prefer it be nuked with fire. Additionally, there's some major UB happening in passing data to the shader, and that section needs to be rewritten to properly clamp the alpha value to the 0...255 range before assignment.

I've left more detailed comments around specific areas of the code. If a per-instance hash is needed, I'd recommend using a hash function in the form mix32(pos.x + pos.y + pos.z) for the hash value to remain independent of the view direction ( 😅 ). That function is derived from research into an optimal 32-bit hash function and would take the form (operating on the bits of the floating-point value):

uint32_t hash32(uint32_t x)
{
	x ^= x >> 16;
	x *= 0x21f0aaad;
	x ^= x >> 15;
	x *= 0xd35a2d97;
	x ^= x >> 15;
	return x;
}

My quick-and-dirty testing version of the following comments (some improvement could obviously be made) looks like so:

float hash = pos.x + pos.y + pos.z;
hash = (uint16_t(hash32(*reinterpret_cast<uint32_t *>(&hash)) & 0xFFFF)) / 65535.f;

float time = rd->timeStep * 55.f * (0.75f + hash * 0.5f);
const float flicker = abs(sin(time));

m_tMat->emissive.a = m_glowMat->emissive.a = flicker * 255.f;

2025-09-08.22-47-52.mp4

[fluffyfreak]

Ok I've rebased, fixed up the commented changes, adopted @sturnclaw hash system, passed through the game time, including guarding when there is no game running, and added a linear fade in/out effect like in the original gifs when using the keyboard.

This reduces the changed files to 5: shader + thruster files only.

[fluffyfreak]

ah dammit, this doesn't like me dragging in those headers it seems

[fluffyfreak]

This is very hacky and I'm not keen on it, but I also don't like polluting the Node.h and every other class just to give it access to a timeStep that is either (1.0 / 60.0) or calculated only when the Game class is instantiated and in use.

[sturnclaw]

Long-term, this kind of state should be stored in a RenderWorld object that is passed as a dependency to various render functions. My vision is for thrusters, StaticGeometry nodes, etc. to simply have a lightweight handle (32-bit ID+version) to a RenderMesh object associated with the RenderWorld, and the work of Thruster::Render to be setting the transform and material parameters of that RenderMesh. This proposed design would separate graphics resources from gameplay resources (e.g. tags, collision, etc).

Eventually, this kind of information should just be uploaded to the GPU in a global render-pass data buffer and all this math moved to the shader instead.

In the short term, this is an acceptable hack, though very very rough. I'd prefer passing this through the RenderData mechanism instead, and sprucing up that class as necessary to make it more than just an internal implementation detail.

[fluffyfreak]

I've had to revert this, it didn't work, and I don't quite understand why but suspect that it was defining multiple statics across multiple TUs. SO you'd set it from one place and the value wouldn't change when read elsewhere.

I can now totally understand why JeffMD did it via the RenderData :(

[fluffyfreak]

I'm going to leave this for a couple of days now. Quite annoyed that I couldn't avoid modifying Node etc