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

Store ICrushable traits as actor field. #21261

Merged
merged 1 commit into from Jan 31, 2024

Conversation

anvilvapre
Copy link
Contributor

Store ICrushable traits as actor field.

To avoid using calls to trait dictionary TraitsImplementing that will cause the actor to be looked up in a list of all actors - with the ICrushable trait - using a binary search.

Relevant as path planning uses it to determine if a cell is blocked.

The down side is that actor needs to be ICrushable aware. This requires the move of ICrushable from Common to OpenRA.Traits.

Alternative could be to only introduce an abstract IActorIsBlockingCheck trait that could be cast to an ICrushable. But that seems less clean. Or to let trait dictionary store certain traits that are looked up mostly by actor as a hash table.

@PunkPun
Copy link
Member

PunkPun commented Dec 18, 2023

related #14920

Copy link
Member

@RoosterDragon RoosterDragon left a comment

Choose a reason for hiding this comment

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

We already have so many traits against actor for pathfinding perf, I think moving Crushable out of Common is acceptable to enable this last one.

@PunkPun
Copy link
Member

PunkPun commented Jan 6, 2024

We should really create a dumbed down version of actor that actor inherits

@PunkPun
Copy link
Member

PunkPun commented Jan 6, 2024

something that tree's for example could be. They for example don't need activities nor order handling

We already have so many traits against actor for pathfinding perf

could you list them?

@RoosterDragon
Copy link
Member

OpenRA/OpenRA.Game/Actor.cs

Lines 164 to 183 in 9a1823d

// PERF: Cache all these traits as soon as the actor is created. This is a fairly cheap one-off cost per
// actor that allows us to provide some fast implementations of commonly used methods that are relied on by
// performance-sensitive parts of the core game engine, such as pathfinding, visibility and rendering.
// Note: The blocks are required to limit the scope of the t's, so we make an exception to our normal style
// rules for spacing in order to keep these assignments compact and readable.
{ if (trait is IOccupySpace t) OccupiesSpace = t; }
{ if (trait is IEffectiveOwner t) EffectiveOwner = t; }
{ if (trait is IFacing t) facing = t; }
{ if (trait is IHealth t) health = t; }
{ if (trait is IResolveOrder t) resolveOrdersList.Add(t); }
{ if (trait is IRenderModifier t) renderModifiersList.Add(t); }
{ if (trait is IRender t) rendersList.Add(t); }
{ if (trait is IMouseBounds t) mouseBoundsList.Add(t); }
{ if (trait is IVisibilityModifier t) visibilityModifiersList.Add(t); }
{ if (trait is IDefaultVisibility t) defaultVisibility = t; }
{ if (trait is INotifyBecomingIdle t) becomingIdlesList.Add(t); }
{ if (trait is INotifyIdle t) tickIdlesList.Add(t); }
{ if (trait is ITargetable t) targetablesList.Add(t); }
{ if (trait is ITargetablePositions t) targetablePositionsList.Add(t); }
{ if (trait is ISync t) syncHashesList.Add(new SyncHash(t)); }

@PunkPun PunkPun merged commit 64cdfcb into OpenRA:bleed Jan 31, 2024
3 checks passed
@PunkPun
Copy link
Member

PunkPun commented Jan 31, 2024

Changelog

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

Successfully merging this pull request may close these issues.

None yet

3 participants