Skip to content

Commit

Permalink
Activity, fixes.
Browse files Browse the repository at this point in the history
Do not call SkipDoneActivities method recursively via the
NextActivity property. Rather use the nextActivity member.
Avoiding additional function calls and a recursively
growing stack.

Do not call ChildActivity and NextActivity properties
twice in a row. Once to test for null and after to access
it's value. It will cause the complete list of activities
to be traversed twice looking for non done activities.

Replace Queue method with a version that does not the
NextActivity property causing an extra call to
SkipDoneActivities. Avoid calling Queue recursively.

Similar replace QueueChild with a version that does
not call additional methods.

Note that ActivitiesImplementing returns only non
done activities. The method name does not suggest this.

Please consider making NextActivity a method to cleary indicate it
involves the logic of skipping Done activities. To let
the called know it is 'expensive'.

Please consider renaming the protected property ChildActivity to
FirstChildActivityNotDone to avoid it being used as childActivity.

Please consider maintaining a pointer to the first
non done activity. This avoids the need the each time find it.
  • Loading branch information
anvilvapre committed Oct 31, 2020
1 parent 7b75a78 commit 5fe6606
Showing 1 changed file with 21 additions and 14 deletions.
35 changes: 21 additions & 14 deletions OpenRA.Game/Activities/Activity.cs
Expand Up @@ -74,7 +74,7 @@ internal static Activity SkipDoneActivities(Activity first)
// drop valid activities queued after it. Walk the queue until we find a valid activity or
// (more likely) run out of activities.
while (first != null && first.State == ActivityState.Done)
first = first.NextActivity;
first = first.nextActivity;

return first;
}
Expand Down Expand Up @@ -120,7 +120,8 @@ public Activity TickOuter(Actor self)
lastRun = Tick(self);

// Avoid a single tick delay if the childactivity was just queued.
if (ChildActivity != null && ChildActivity.State == ActivityState.Queued)
var ca = ChildActivity;
if (ca != null && ca.State == ActivityState.Queued)
{
if (ChildHasPriority)
lastRun = TickChild(self) && finishing;
Expand Down Expand Up @@ -206,18 +207,18 @@ public virtual void Cancel(Actor self, bool keepQueue = false)

public void Queue(Activity activity)
{
if (NextActivity != null)
NextActivity.Queue(activity);
else
NextActivity = activity;
var it = this;
while (it.nextActivity != null)
it = it.nextActivity;
it.nextActivity = activity;
}

public void QueueChild(Activity activity)
{
if (ChildActivity != null)
ChildActivity.Queue(activity);
if (childActivity != null)
childActivity.Queue(activity);
else
ChildActivity = activity;
childActivity = activity;
}

/// <summary>
Expand Down Expand Up @@ -269,15 +270,21 @@ public IEnumerable<string> DebugLabelComponents()

public IEnumerable<T> ActivitiesImplementing<T>(bool includeChildren = true) where T : IActivityInterface
{
if (includeChildren && ChildActivity != null)
foreach (var a in ChildActivity.ActivitiesImplementing<T>())
yield return a;
// Skips Done child and next activities
if (includeChildren)
{
var ca = ChildActivity;
if (ca != null)
foreach (var a in ca.ActivitiesImplementing<T>())
yield return a;
}

if (this is T)
yield return (T)(object)this;

if (NextActivity != null)
foreach (var a in NextActivity.ActivitiesImplementing<T>())
var na = NextActivity;
if (na != null)
foreach (var a in na.ActivitiesImplementing<T>())
yield return a;
}
}
Expand Down

0 comments on commit 5fe6606

Please sign in to comment.