Skip to content

Fueled Thrusters#22

Closed
ArtisticRoomba wants to merge 16 commits intoEphemeralSpace:masterfrom
ArtisticRoomba:fueled-thrusters
Closed

Fueled Thrusters#22
ArtisticRoomba wants to merge 16 commits intoEphemeralSpace:masterfrom
ArtisticRoomba:fueled-thrusters

Conversation

@ArtisticRoomba
Copy link
Contributor

@ArtisticRoomba ArtisticRoomba commented Jun 16, 2025

About the PR

Introduces Fueled Thrusters, a thruster variant that requires some sort of gas to operate.

Thrusters receive benefits like reduced fuel efficiency and increased thrust depending on the gas mixture currently present in the thruster. Gas mixes and their benefits can be determined as an array and have their benefits applied in under different circumstances.

Why / Balance

Needed.

Technical details

A new object is defined that "pairs" up a GasMixture object with data about that GasMixture.
A new atmospheric update loop in ThrusterSystem processes this information and determines the final thrust multiplier/efficiency available to the thruster. It also determines if the thruster is allowed to fire or not.

Overall the code is very scrungly and to be honest I'm thinking of rewriting ThrusterSystem to more modern standards, as it wasn't super extensible to add onto. Ideally we'd like to define certain thruster effects that can happen. Right now it was complicated enough to get certain state/prevention logic working and even then the implementation still suffers from visual bugs.

Media

Content Client_V1HSSK2ipt
Content Client_NLoz9KHz02

Requirements

Breaking changes

not yet

Changelog

Copy link
Contributor

@moonheart08 moonheart08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursory review.

/// <param name="b">The second <see cref="GasMixture"/> to compare.</param>
/// <returns>A float between 0 and 1 based on how similar the gas mixtures are,
/// based on percent compositions.</returns>
public float GetGasMixtureSimilarity(GasMixture? a, GasMixture? b)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit/suggestion: mathematical similarity feels really arbitrary for thrusters, maybe just handle the gasses uniquely instead (esp as this can enable fun interactions like the thrusters oxidizing themselves to death if you put O2 in there and allowing the use of N2O for boosting which makes no sense they're ION thrusters and do not combust the plasma but it'd be funny)

}

// Fall back to 1 if no valid denominator is present
return denominator == 0f ? 1f : numerator / 1f; // numerator is already normalized here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use equals here because platform dependant fpu behavior :)
(use a small epsilon, arbitrary, like 0.001f, so Math.Abs(denominator) < 0.001f to see if the number is sufficiently near zero.)

/// than effectively duplicating the code.</remarks>
[Serializable]
[DataDefinition]
public sealed partial class ThrusterGasMixturePair
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling it a "Pair" is a misnomer, it is not two numbered fields (i.e. public struct Foo(lower, upper)). Just call it a ThrusterGasMixture.

Also make sure to assert the mixture is immutable or you can and will have weird effects at runtime if anyone has the funny idea of balancing toward this mixture!

Comment on lines +56 to +66
/// <summary>
/// Cached query for shuttle components,
/// since they rarely change, and we need to access them frequently.
/// </summary>
private EntityQuery<ShuttleComponent> _shuttleQuery;

/// <summary>
/// Cached query for thruster transforms, since
/// we need to access them frequently when we preform thruster buff/nerf updates.
/// </summary>
private EntityQuery<TransformComponent> _thrusterTransformQuery;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: imo overly verbose docs for a common optimization.


break;

// If the field is not present(???), fallback to Pure.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-presence would be None (your first enum value, 0). You should throw new NotImplementedException() over the fallback, as that means the enum is some unknown invalid value.

// gyroscope by feeding it tritium.
if (ent.Comp.Type == ThrusterType.Angular)
{
shuttlecomp.AngularThrust += deltaThrust;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two thrust values scale very differently, I would slap a sqrt or smth on deltaThrust exclusively for angular thrust just so it doesn't result in extreme spinny with high efficiency. Mess with it a bit ig?

Because right now it seems like this would add over 40,000 angular thrust, way outside the 'intended' range of values, which would be really funny but not desired.

{
var benefit = 0f;

// You'll never catch be writing this shit in upstream in a million years.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm :^)

@github-actions github-actions bot added size/L and removed size/M labels Jun 30, 2025
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@EmoGarbage404
Copy link
Contributor

ended up on #43 because of jank

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants