From 78253ce28475c38f06383d9ca83112737077aed5 Mon Sep 17 00:00:00 2001
From: Vapre <vapre@hotmail.com>
Date: Fri, 30 Oct 2020 23:30:55 +0100
Subject: [PATCH] Activity, fixes.

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.
---
 OpenRA.Game/Activities/Activity.cs | 35 ++++++++++++++++++------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/OpenRA.Game/Activities/Activity.cs b/OpenRA.Game/Activities/Activity.cs
index eb4719ac7e..8f2493005e 100644
--- a/OpenRA.Game/Activities/Activity.cs
+++ b/OpenRA.Game/Activities/Activity.cs
@@ -74,7 +74,7 @@ namespace OpenRA.Activities
 			// 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;
 		}
@@ -120,7 +120,8 @@ namespace OpenRA.Activities
 				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;
@@ -206,18 +207,18 @@ namespace OpenRA.Activities
 
 		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>
@@ -269,15 +270,21 @@ namespace OpenRA.Activities
 
 		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;
 		}
 	}