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.

Sign up for GitHub

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

Jump to bottom

Make shared compstates copy cloneable types #5459

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Make shared compstates copy cloneable types #5459

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
97340df
Make shared compstates copy cloneable types
metalgearsloth Sep 23, 2024
3c4379e
FULL_RELEASE
metalgearsloth Sep 27, 2024
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
16 changes: 12 additions & 4 deletions Robust.Shared.CompNetworkGenerator/ComponentNetworkGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,18 @@ public class ComponentNetworkGenerator : ISourceGenerator

if (IsCloneType(type))
{
// get first ctor arg of the field attribute, which determines whether the field should be cloned
// (like if its a dict or list)
getStateInit.Append($@"
{name} = component.{name},");
if (type.NullableAnnotation == NullableAnnotation.NotAnnotated)
{
// get first ctor arg of the field attribute, which determines whether the field should be cloned
// (like if its a dict or list)
getStateInit.Append($@"
{name} = new(component.{name}),");
}
else
{
getStateInit.Append($@"
{name} = component.{name} == null ? null : new(component.{name}),");
}
Copy link
Member

@ElectroJr ElectroJr Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Only the client should have to clone for get-state (and it would only ever have to do so once per comp).
So this should somehow check is-server/is-client to avoid allocs.

And for fixture sys, its easy to manually add the checks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't debug sourcegen code coz my IDE blows up but not sure if I can just get the assembly or what, I'll see if I can figure something out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I ended up using FULL_RELEASE but somehow it blows up tests. I think on an actual server it probably doesn't matter for getstate? Alternatively I use an msbuild property like pjb said.


handleStateSetters.Append($@"
if (state.{name} == null)
Expand Down
28 changes: 21 additions & 7 deletions Robust.Shared/Physics/Systems/FixtureSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace Robust.Shared.Physics.Systems
public sealed partial class FixtureSystem : EntitySystem
{
[Dependency] private readonly EntityLookupSystem _lookup = default!;
[Dependency] private readonly SharedBroadphaseSystem _broadphase = default!;
[Dependency] private readonly SharedPhysicsSystem _physics = default!;
private EntityQuery<PhysicsMapComponent> _mapQuery;
private EntityQuery<PhysicsComponent> _physicsQuery;
Expand Down Expand Up @@ -224,9 +225,18 @@ internal void OnPhysicsInit(EntityUid uid, FixturesComponent component, PhysicsC

private void OnGetState(EntityUid uid, FixturesComponent component, ref ComponentGetState args)
{
var copied = new Dictionary<string, Fixture>(component.Fixtures.Count);

foreach (var (id, fixture) in component.Fixtures)
{
var copy = new Fixture();
fixture.CopyTo(copy);
copied[id] = copy;
}

args.State = new FixtureManagerComponentState
{
Fixtures = component.Fixtures,
Fixtures = copied,
};
}

Expand All @@ -239,10 +249,6 @@ private void OnHandleState(EntityUid uid, FixturesComponent component, ref Compo
Log.Error($"Tried to apply fixture state for an entity without physics: {ToPrettyString(uid)}");
return;
}
foreach (var fixture in component.Fixtures.Values)
{
fixture.Owner = uid;
}

var toAddFixtures = new ValueList<(string Id, Fixture Fixture)>();
var toRemoveFixtures = new ValueList<(string Id, Fixture Fixture)>();
Expand All @@ -260,6 +266,7 @@ private void OnHandleState(EntityUid uid, FixturesComponent component, ref Compo
}

TransformComponent? xform = null;
var regenerate = false;

// Add / update new fixtures
// FUTURE SLOTH
Expand All @@ -272,10 +279,12 @@ private void OnHandleState(EntityUid uid, FixturesComponent component, ref Compo
{
toAddFixtures.Add((id, fixture));
}
// Retained fixture but new data
else if (!existing.Equivalent(fixture))
{
toRemoveFixtures.Add((id, existing));
toAddFixtures.Add((id, fixture));
fixture.CopyTo(existing);
computeProperties = true;
regenerate = true;
}
}

Expand Down Expand Up @@ -309,6 +318,11 @@ private void OnHandleState(EntityUid uid, FixturesComponent component, ref Compo
{
FixtureUpdate(uid, manager: component, body: physics);
}

if (regenerate)
{
_broadphase.RegenerateContacts(uid, body: physics, fixtures: component, xform: xform);
}
}

#region Restitution
Expand Down
Loading