Skip to content
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

Avoid unnecessary DirtyField() calls #5620

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ END TEMPLATE-->

### New features

*None yet*
* Added `EntityManager.DirtyFields()`, which allows components with delta states to simultaneously mark several fields as dirty at the same time.

### Bugfixes

Expand Down
24 changes: 23 additions & 1 deletion Robust.Client/GameObjects/ClientEntityManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,33 @@ public override void Dirty(EntityUid uid, IComponent component, MetaDataComponen
/// <inheritdoc />
public override void Dirty<T>(Entity<T> ent, MetaDataComponent? meta = null)
{
// Client only dirties during prediction
// Client only dirties during prediction
if (_gameTiming.InPrediction)
base.Dirty(ent, meta);
}

public override void DirtyField<T>(EntityUid uid, T comp, string fieldName, MetaDataComponent? metadata = null)
{
// TODO Prediction
// does the client actually need to dirty the field?
// I.e., can't it just dirty the whole component to trigger a reset?

// Client only dirties during prediction
if (_gameTiming.InPrediction)
base.DirtyField(uid, comp, fieldName, metadata);
}

public override void DirtyFields<T>(EntityUid uid, T comp, MetaDataComponent? meta, params ReadOnlySpan<string> fields)
{
// TODO Prediction
// does the client actually need to dirty the field?
// I.e., can't it just dirty the whole component to trigger a reset?

// Client only dirties during prediction
if (_gameTiming.InPrediction)
base.DirtyFields(uid, comp, meta, fields);
}

/// <inheritdoc />
public override void Dirty<T1, T2>(Entity<T1, T2> ent, MetaDataComponent? meta = null)
{
Expand Down
4 changes: 4 additions & 0 deletions Robust.Shared/Containers/SharedContainerSystem.Insert.cs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ private void RecursivelyUpdatePhysics(Entity<TransformComponent, PhysicsComponen
{
if (entity.Comp2 is { } physics)
{
// TODO CONTAINER
// Is this actually needed?
// I.e., shouldn't this just do a if (_timing.ApplyingState) return

// Here we intentionally don't dirty the physics comp. Client-side state handling will apply these same
// changes. This also ensures that the server doesn't have to send the physics comp state to every
// player for any entity inside of a container during init.
Expand Down
24 changes: 23 additions & 1 deletion Robust.Shared/GameObjects/EntityManager.ComponentDeltas.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using Robust.Shared.Timing;

namespace Robust.Shared.GameObjects;
Expand Down Expand Up @@ -38,11 +39,14 @@ public void DirtyField(EntityUid uid, IComponentDelta comp, string fieldName, Me
Dirty(uid, comp, metadata);
}

public void DirtyField<T>(EntityUid uid, T comp, string fieldName, MetaDataComponent? metadata = null)
public virtual void DirtyField<T>(EntityUid uid, T comp, string fieldName, MetaDataComponent? metadata = null)
where T : IComponentDelta
{
var compReg = ComponentFactory.GetRegistration(CompIdx.Index<T>());

// TODO
// consider storing this on MetaDataComponent?
// We alsready store other dirtying information there anyways, and avoids having to fetch the registration.
if (!compReg.NetworkedFieldLookup.TryGetValue(fieldName, out var idx))
{
_sawmill.Error($"Tried to dirty delta field {fieldName} on {ToPrettyString(uid)} that isn't implemented.");
Expand All @@ -54,6 +58,24 @@ public void DirtyField<T>(EntityUid uid, T comp, string fieldName, MetaDataCompo
comp.LastModifiedFields[idx] = curTick;
Dirty(uid, comp, metadata);
}

public virtual void DirtyFields<T>(EntityUid uid, T comp, MetaDataComponent? meta, params ReadOnlySpan<string> fields)
where T : IComponentDelta
{
var compReg = ComponentFactory.GetRegistration(CompIdx.Index<T>());

var curTick = _gameTiming.CurTick;
foreach (var field in fields)
{
if (!compReg.NetworkedFieldLookup.TryGetValue(field, out var idx))
_sawmill.Error($"Tried to dirty delta field {field} on {ToPrettyString(uid)} that isn't implemented.");
else
comp.LastModifiedFields[idx] = curTick;
}

comp.LastFieldUpdate = curTick;
Dirty(uid, comp, meta);
}
}

/// <summary>
Expand Down
3 changes: 1 addition & 2 deletions Robust.Shared/GameObjects/EntityManager.Components.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1694,7 +1694,6 @@ public bool HasComponent([NotNullWhen(true)] EntityUid? uid)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
[Pure]
public bool Resolve(EntityUid uid, [NotNullWhen(true)] ref TComp1? component, bool logMissing = true)
{
if (component != null)
Expand All @@ -1717,7 +1716,7 @@ public bool Resolve(EntityUid uid, [NotNullWhen(true)] ref TComp1? component, bo
return false;
}

[MethodImpl(MethodImplOptions.AggressiveInlining), Pure]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool Resolve(ref Entity<TComp1?> entity, bool logMissing = true)
{
return Resolve(entity.Owner, ref entity.Comp, logMissing);
Expand Down
7 changes: 7 additions & 0 deletions Robust.Shared/GameObjects/EntitySystem.Proxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,13 @@ protected void DirtyField<T>(EntityUid uid, T component, string fieldName, MetaD
EntityManager.DirtyField(uid, component, fieldName, meta);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
protected void DirtyFields<T>(EntityUid uid, T comp, MetaDataComponent? meta, params ReadOnlySpan<string> fields)
where T : IComponentDelta
{
EntityManager.DirtyFields(uid, comp, meta);
}

/// <summary>
/// Marks a component as dirty. This also implicitly dirties the entity this component belongs to.
/// </summary>
Expand Down
114 changes: 50 additions & 64 deletions Robust.Shared/Physics/Systems/SharedPhysicsSystem.Components.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,26 +139,26 @@ private void OnPhysicsHandleState(EntityUid uid, PhysicsComponent component, ref

if (args.Current is PhysicsLinearVelocityDeltaState linearState)
{
SetLinearVelocity(uid, linearState.LinearVelocity, body: component, manager: manager);
SetLinearVelocity(uid, linearState.LinearVelocity, dirty: false, body: component, manager: manager);
}
else if (args.Current is PhysicsVelocityDeltaState velocityState)
{
SetLinearVelocity(uid, velocityState.LinearVelocity, body: component, manager: manager);
SetAngularVelocity(uid, velocityState.AngularVelocity, body: component, manager: manager);
SetLinearVelocity(uid, velocityState.LinearVelocity, dirty: false, body: component, manager: manager);
SetAngularVelocity(uid, velocityState.AngularVelocity, dirty: false, body: component, manager: manager);
}
else if (args.Current is PhysicsComponentState newState)
{
SetSleepingAllowed(uid, component, newState.SleepingAllowed);
SetFixedRotation(uid, newState.FixedRotation, body: component);
SetCanCollide(uid, newState.CanCollide, body: component);
SetSleepingAllowed(uid, component, newState.SleepingAllowed, dirty: false);
SetFixedRotation(uid, newState.FixedRotation, body: component, dirty: false);
SetCanCollide(uid, newState.CanCollide, body: component, dirty: false);
component.BodyStatus = newState.Status;

SetLinearVelocity(uid, newState.LinearVelocity, body: component, manager: manager);
SetAngularVelocity(uid, newState.AngularVelocity, body: component, manager: manager);
SetLinearVelocity(uid, newState.LinearVelocity, dirty: false, body: component, manager: manager);
SetAngularVelocity(uid, newState.AngularVelocity, dirty: false, body: component, manager: manager);
SetBodyType(uid, newState.BodyType, manager, component);
SetFriction(uid, component, newState.Friction);
SetLinearDamping(uid, component, newState.LinearDamping);
SetAngularDamping(uid, component, newState.AngularDamping);
SetFriction(uid, component, newState.Friction, dirty: false);
SetLinearDamping(uid, component, newState.LinearDamping, dirty: false);
SetAngularDamping(uid, component, newState.AngularDamping, dirty: false);
component.Force = newState.Force;
component.Torque = newState.Torque;
}
Expand Down Expand Up @@ -270,29 +270,12 @@ public void DestroyContacts(PhysicsComponent body)
/// </summary>
public void ResetDynamics(EntityUid uid, PhysicsComponent body, bool dirty = true)
{
if (body.Torque != 0f)
{
body.Torque = 0f;
DirtyField(uid, body, nameof(PhysicsComponent.Torque));
}

if (body.AngularVelocity != 0f)
{
body.AngularVelocity = 0f;
DirtyField(uid, body, nameof(PhysicsComponent.AngularVelocity));
}

if (body.Force != Vector2.Zero)
{
body.Force = Vector2.Zero;
DirtyField(uid, body, nameof(PhysicsComponent.Force));
}

if (body.LinearVelocity != Vector2.Zero)
{
body.LinearVelocity = Vector2.Zero;
DirtyField(uid, body, nameof(PhysicsComponent.LinearVelocity));
}
body.Torque = 0f;
body.AngularVelocity = 0f;
body.Force = Vector2.Zero;
body.LinearVelocity = Vector2.Zero;
if (dirty)
DirtyFields(uid, body, null, nameof(PhysicsComponent.Torque), nameof(PhysicsComponent.AngularVelocity), nameof(PhysicsComponent.Force), nameof(PhysicsComponent.LinearVelocity));
}

public void ResetMassData(EntityUid uid, FixturesComponent? manager = null, PhysicsComponent? body = null)
Expand Down Expand Up @@ -397,7 +380,8 @@ public bool SetAngularVelocity(EntityUid uid, float value, bool dirty = true, Fi
return false;

body.AngularVelocity = value;
DirtyField(uid, body, nameof(PhysicsComponent.AngularVelocity));
if (dirty)
DirtyField(uid, body, nameof(PhysicsComponent.AngularVelocity));

return true;
}
Expand All @@ -423,7 +407,9 @@ public bool SetLinearVelocity(EntityUid uid, Vector2 velocity, bool dirty = true
return false;

body.LinearVelocity = velocity;
DirtyField(uid, body, nameof(PhysicsComponent.LinearVelocity));
if (dirty)
DirtyField(uid, body, nameof(PhysicsComponent.LinearVelocity));

return true;
}

Expand All @@ -433,7 +419,8 @@ public void SetAngularDamping(EntityUid uid, PhysicsComponent body, float value,
return;

body.AngularDamping = value;
DirtyField(uid, body, nameof(PhysicsComponent.AngularDamping));
if (dirty)
DirtyField(uid, body, nameof(PhysicsComponent.AngularDamping));
}

public void SetLinearDamping(EntityUid uid, PhysicsComponent body, float value, bool dirty = true)
Expand All @@ -442,7 +429,8 @@ public void SetLinearDamping(EntityUid uid, PhysicsComponent body, float value,
return;

body.LinearDamping = value;
DirtyField(uid, body, nameof(PhysicsComponent.LinearDamping));
if (dirty)
DirtyField(uid, body, nameof(PhysicsComponent.LinearDamping));
}

[Obsolete("Use SetAwake with EntityUid<PhysicsComponent>")]
Expand Down Expand Up @@ -518,32 +506,27 @@ public void SetBodyType(EntityUid uid, BodyType value, FixturesComponent? manage
body.BodyType = value;
ResetMassData(uid, manager, body);

body.Force = Vector2.Zero;
body.Torque = 0f;

if (body.BodyType == BodyType.Static)
{
SetAwake((uid, body), false);

if (body.LinearVelocity != Vector2.Zero)
{
body.LinearVelocity = Vector2.Zero;
DirtyField(uid, body, nameof(PhysicsComponent.LinearVelocity));
}
body.LinearVelocity = Vector2.Zero;
body.AngularVelocity = 0f;

if (body.AngularVelocity != 0f)
{
body.AngularVelocity = 0f;
DirtyField(uid, body, nameof(PhysicsComponent.AngularVelocity));
}
DirtyFields(uid, body, null,
nameof(PhysicsComponent.LinearVelocity),
nameof(PhysicsComponent.AngularVelocity),
nameof(PhysicsComponent.Force),
nameof(PhysicsComponent.Torque));
}
// Even if it's dynamic if it can't collide then don't force it awake.
else if (body.CanCollide)
{
SetAwake((uid, body), true);
}

if (body.Torque != 0f)
{
body.Torque = 0f;
DirtyField(uid, body, nameof(PhysicsComponent.Torque));
DirtyFields(uid, body, null, nameof(PhysicsComponent.Force), nameof(PhysicsComponent.Torque));
}

_broadphase.RegenerateContacts(uid, body, manager, xform);
Expand All @@ -561,7 +544,8 @@ public void SetBodyStatus(EntityUid uid, PhysicsComponent body, BodyStatus statu
return;

body.BodyStatus = status;
DirtyField(uid, body, nameof(PhysicsComponent.BodyStatus));
if (dirty)
DirtyField(uid, body, nameof(PhysicsComponent.BodyStatus));
}

/// <summary>
Expand Down Expand Up @@ -612,7 +596,10 @@ public bool SetCanCollide(
var ev = new CollisionChangeEvent(uid, body, value);
RaiseLocalEvent(ref ev);
}
DirtyField(uid, body, nameof(PhysicsComponent.CanCollide));

if (dirty)
DirtyField(uid, body, nameof(PhysicsComponent.CanCollide));

return value;
}

Expand All @@ -622,13 +609,10 @@ public void SetFixedRotation(EntityUid uid, bool value, bool dirty = true, Fixtu
return;

body.FixedRotation = value;
DirtyField(uid, body, nameof(PhysicsComponent.FixedRotation));
body.AngularVelocity = 0.0f;

if (body.AngularVelocity != 0f)
{
body.AngularVelocity = 0.0f;
DirtyField(uid, body, nameof(PhysicsComponent.AngularVelocity));
}
if (dirty)
DirtyFields(uid, body, null, nameof(PhysicsComponent.FixedRotation), nameof(PhysicsComponent.AngularVelocity));

ResetMassData(uid, manager: manager, body: body);
}
Expand All @@ -639,7 +623,8 @@ public void SetFriction(EntityUid uid, PhysicsComponent body, float value, bool
return;

body._friction = value;
DirtyField(uid, body, nameof(PhysicsComponent.Friction));
if (dirty)
DirtyField(uid, body, nameof(PhysicsComponent.Friction));
}

public void SetInertia(EntityUid uid, PhysicsComponent body, float value, bool dirty = true)
Expand Down Expand Up @@ -678,7 +663,8 @@ public void SetSleepingAllowed(EntityUid uid, PhysicsComponent body, bool value,
SetAwake((uid, body), true);

body.SleepingAllowed = value;
DirtyField(uid, body, nameof(PhysicsComponent.SleepingAllowed));
if (dirty)
DirtyField(uid, body, nameof(PhysicsComponent.SleepingAllowed));
}

public void SetSleepTime(PhysicsComponent body, float value)
Expand Down
7 changes: 7 additions & 0 deletions Robust.Shared/Physics/Systems/SharedPhysicsSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ public override void Initialize()

_physicsReg = EntityManager.ComponentFactory.GetRegistration(CompIdx.Index<PhysicsComponent>());

// TODO PHYSICS STATE
// Consider condensing the possible fields into just Linear velocity, angular velocity, and "Other"
// Or maybe even just "velocity" & "other"
// Then get-state doesn't have to iterate over a 10-element array.
// And it simplifies the DirtyField calls.
// Though I guess combining fixtures & physics will complicate it a bit more again.

// If you update this then update the delta state + GetState + HandleState!
EntityManager.ComponentFactory.RegisterNetworkedFields(_physicsReg,
nameof(PhysicsComponent.CanCollide),
Expand Down
Loading