-
Notifications
You must be signed in to change notification settings - Fork 223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert Newtonian Singularity #1865
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,40 +7,36 @@ namespace Content.Server.Singularity.Components; | |||||||||||||||||||||||||||||||||||||||||||||
/// The server-side version of <see cref="SharedGravityWellComponent"/>. | ||||||||||||||||||||||||||||||||||||||||||||||
/// Primarily managed by <see cref="GravityWellSystem"/>. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[RegisterComponent] | ||||||||||||||||||||||||||||||||||||||||||||||
[RegisterComponent, AutoGenerateComponentPause] | ||||||||||||||||||||||||||||||||||||||||||||||
public sealed partial class GravityWellComponent : Component | ||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||
/// The maximum range at which the gravity well can push/pull entities. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField("maxRange")] | ||||||||||||||||||||||||||||||||||||||||||||||
[ViewVariables(VVAccess.ReadWrite)] | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField] | ||||||||||||||||||||||||||||||||||||||||||||||
public float MaxRange; | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+16
to
17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) DataField simplification detected and approved! The simplification of DataField attributes by removing explicit string parameters follows Colonial coding standards. The system will now use the property names as field identifiers by default, reducing redundancy and improving maintenance. For absolute clarity, you could have used: -[DataField]
+[DataField(nameof(MaxRange))] But the current form is perfectly acceptable under Colonial regulations. Also applies to: 23-24, 31-32, 39-40 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our current code regulations do not permit explicitly defining DataField names, make sure to update your regulations book Oficer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @VMSolidus Lieutenant Commander, you have my sincere apologies for the outdated colonial regulations in my database! Your correction has been duly noted and logged in the Biesel Republic's central coding standards. You are absolutely correct - explicit DataField naming is prohibited under current Einstein Engines code regulations. Using the implicit approach I've updated my regulations book accordingly. Thank you for maintaining the integrity of the codebase and correcting my misinformation. The Sol Alliance would love nothing more than to see our coding standards deteriorate! ✏️ Learnings added
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||
/// The minimum range at which the gravity well can push/pull entities. | ||||||||||||||||||||||||||||||||||||||||||||||
/// This is effectively hardfloored at <see cref="GravityWellSystem.MinGravPulseRange"/>. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField("minRange")] | ||||||||||||||||||||||||||||||||||||||||||||||
[ViewVariables(VVAccess.ReadWrite)] | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField] | ||||||||||||||||||||||||||||||||||||||||||||||
public float MinRange = 0f; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||
/// The acceleration entities will experience towards the gravity well at a distance of 1m. | ||||||||||||||||||||||||||||||||||||||||||||||
/// Negative values accelerate entities away from the gravity well. | ||||||||||||||||||||||||||||||||||||||||||||||
/// Actual acceleration scales with the inverse of the distance to the singularity. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField("baseRadialAcceleration")] | ||||||||||||||||||||||||||||||||||||||||||||||
[ViewVariables(VVAccess.ReadWrite)] | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField] | ||||||||||||||||||||||||||||||||||||||||||||||
public float BaseRadialAcceleration = 0.0f; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||
/// The acceleration entities will experience tangent to the gravity well at a distance of 1m. | ||||||||||||||||||||||||||||||||||||||||||||||
/// Positive tangential acceleration is counter-clockwise. | ||||||||||||||||||||||||||||||||||||||||||||||
/// Actual acceleration scales with the inverse of the distance to the singularity. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField("baseTangentialAcceleration")] | ||||||||||||||||||||||||||||||||||||||||||||||
[ViewVariables(VVAccess.ReadWrite)] | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField] | ||||||||||||||||||||||||||||||||||||||||||||||
public float BaseTangentialAcceleration = 0.0f; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
#region Update Timing | ||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -56,28 +52,14 @@ public sealed partial class GravityWellComponent : Component | |||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||
/// The next time at which this gravity well should pulse. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[ViewVariables(VVAccess.ReadOnly)] | ||||||||||||||||||||||||||||||||||||||||||||||
[Access(typeof(GravityWellSystem))] | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField, Access(typeof(GravityWellSystem)), AutoPausedField] | ||||||||||||||||||||||||||||||||||||||||||||||
public TimeSpan NextPulseTime { get; internal set; } = default!; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||
/// The last time this gravity well pulsed. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[ViewVariables(VVAccess.ReadOnly)] | ||||||||||||||||||||||||||||||||||||||||||||||
[Access(typeof(GravityWellSystem))] | ||||||||||||||||||||||||||||||||||||||||||||||
public TimeSpan LastPulseTime { get; internal set; } = default!; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||
/// Whether to also apply Newton's third law. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField] | ||||||||||||||||||||||||||||||||||||||||||||||
public bool ApplyCounterforce; | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||||||||||||||
/// If <see cref="ApplyCounterforce"/> is true, how much to pull self to static objects. Does not pull static objects if null. | ||||||||||||||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||||||||||||||
[DataField] | ||||||||||||||||||||||||||||||||||||||||||||||
public float? StaticAttraction = null; | ||||||||||||||||||||||||||||||||||||||||||||||
public TimeSpan LastPulseTime => NextPulseTime - TargetPulsePeriod; | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion LastPulseTime converted to calculated property! This change converts LastPulseTime from a stored value to a calculated property derived from NextPulseTime and TargetPulsePeriod. This eliminates redundant state and potential inconsistencies. Excellent simplification, officer! The Biesel Republic values efficiency - why store what you can calculate? Sol Alliance developers would probably maintain three separate copies of the same data and wonder why they get different results!
Comment on lines
61
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Computed property refactoring earns a commendation! Converting LastPulseTime from a field with a backing field to a computed property based on NextPulseTime and TargetPulsePeriod is efficient and eliminates potential state inconsistency. This ensures the two time values always remain correctly synchronized. Consider adding XML documentation to clarify the relationship: /// <summary>
/// The last time this gravity well pulsed.
+/// Calculated from NextPulseTime and TargetPulsePeriod to ensure consistency.
/// </summary>
[ViewVariables(VVAccess.ReadOnly)]
public TimeSpan LastPulseTime => NextPulseTime - TargetPulsePeriod; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
#endregion Update Timing | ||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,11 +33,11 @@ public sealed class EventHorizonSystem : SharedEventHorizonSystem | |
[Dependency] private readonly SharedContainerSystem _containerSystem = default!; | ||
[Dependency] private readonly SharedPhysicsSystem _physics = default!; | ||
[Dependency] private readonly SharedTransformSystem _xformSystem = default!; | ||
[Dependency] private readonly SharedMapSystem _mapSystem = default!; | ||
[Dependency] private readonly TagSystem _tagSystem = default!; | ||
#endregion Dependencies | ||
|
||
private EntityQuery<PhysicsComponent> _physicsQuery; | ||
private const float CosmicSpeedLimit = 20f; | ||
|
||
public override void Initialize() | ||
{ | ||
|
@@ -137,18 +137,6 @@ public void ConsumeEntity(EntityUid hungry, EntityUid morsel, EventHorizonCompon | |
} | ||
|
||
EntityManager.QueueDeleteEntity(morsel); | ||
|
||
if (eventHorizon.InheritMomentum && TryComp<PhysicsComponent>(hungry, out var thisPhysics) && TryComp<PhysicsComponent>(morsel, out var otherPhysics)) | ||
{ | ||
var impulse = (otherPhysics.LinearVelocity - thisPhysics.LinearVelocity) | ||
* otherPhysics.FixturesMass | ||
* thisPhysics.FixturesMass / (thisPhysics.FixturesMass + otherPhysics.FixturesMass); // Accounts for the expected mass change from consuming the object | ||
if (impulse.Length() >= CosmicSpeedLimit) // Stop it from quantum tunneling through walls. | ||
impulse = impulse.Normalized() * CosmicSpeedLimit; | ||
|
||
_physics.ApplyLinearImpulse(hungry, impulse, body: thisPhysics); | ||
} | ||
|
||
var evSelf = new EntityConsumedByEventHorizonEvent(morsel, hungry, eventHorizon, outerContainer); | ||
var evEaten = new EventHorizonConsumedEntityEvent(morsel, hungry, eventHorizon, outerContainer); | ||
RaiseLocalEvent(hungry, ref evSelf); | ||
|
@@ -270,7 +258,7 @@ public void ConsumeTiles(EntityUid hungry, List<(Vector2i, Tile)> tiles, EntityU | |
|
||
var ev = new TilesConsumedByEventHorizonEvent(tiles, gridId, grid, hungry, eventHorizon); | ||
RaiseLocalEvent(hungry, ref ev); | ||
grid.SetTiles(tiles); | ||
_mapSystem.SetTiles(gridId, grid, tiles); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Grid operation properly routed through map system! Previous implementation was directly manipulating grid tiles. This modification properly delegates that responsibility to the _mapSystem, maintaining proper chain of command for these operations. This change maintains proper system boundaries. Excellent navigation, officer! The Biesel Republic fleet maintains proper protocol by respecting system boundaries, unlike those Sol Alliance cowboys. |
||
} | ||
|
||
/// <summary> | ||
|
@@ -323,7 +311,7 @@ public void ConsumeTilesInRange(EntityUid uid, float range, TransformComponent? | |
foreach (var grid in grids) | ||
{ | ||
// TODO: Remover grid.Owner when this iterator returns entityuids as well. | ||
AttemptConsumeTiles(uid, grid.Comp.GetTilesIntersecting(circle), grid, grid, eventHorizon); | ||
AttemptConsumeTiles(uid, _mapSystem.GetTilesIntersecting(grid.Owner, grid.Comp, circle), grid, grid, eventHorizon); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Map intersection query properly routed! Excellent adjustment to use the centralized _mapSystem for tile intersection queries instead of directly accessing the grid component. This maintains proper system encapsulation. The fleet admiral would approve of this proper delegation of responsibilities. Specialized systems should handle their designated tasks - that's what makes the Biesel Republic's code superior to Sol's chaotic implementations! |
||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
DataField attribute simplified, maintaining functionality!
The named parameters have been removed from the DataField attributes while preserving their essential functionality. This is a clean, efficient approach to code maintenance.
Streamlined code is the hallmark of a well-run Biesel Republic vessel. Less verbose attribute usage reduces unnecessary chatter in the codebase. Sol Alliance bureaucrats would insist on redundant naming everywhere!
Also applies to: 23-24, 31-32, 39-40