Merge pull request #9288 from RoosterDragon/action-queue-order

Fix ActionQueue ordering

OpenRA.Game/Game.cs

@@ -384,9 +384,9 @@ namespace OpenRA
 
 		// Note: These delayed actions should only be used by widgets or disposing objects
 		// - things that depend on a particular world should be queuing them on the worldactor.
-		static ActionQueue delayedActions = new ActionQueue();
+		static volatile ActionQueue delayedActions = new ActionQueue();
-		public static void RunAfterTick(Action a) { delayedActions.Add(a); }
+		public static void RunAfterTick(Action a) { delayedActions.Add(a, Game.RunTime); }
-		public static void RunAfterDelay(int delay, Action a) { delayedActions.Add(a, delay); }
+		public static void RunAfterDelay(int delayMilliseconds, Action a) { delayedActions.Add(a, Game.RunTime + delayMilliseconds); }
 
 		static void TakeScreenshotInner()
 		{
@@ -487,7 +487,7 @@ namespace OpenRA
 
 		static void LogicTick()
 		{
-			delayedActions.PerformActions();
+			delayedActions.PerformActions(Game.RunTime);
 
 			if (OrderManager.Connection.ConnectionState != lastConnectionState)
 			{

OpenRA.Game/Primitives/ActionQueue.cs

@@ -9,6 +9,7 @@
 #endregion
 
 using System;
+using System.Collections.Generic;
 
 namespace OpenRA.Primitives
 {
@@ -17,37 +18,56 @@ namespace OpenRA.Primitives
 	/// </summary>
 	public class ActionQueue
 	{
-		object syncRoot = new object();
+		readonly List<DelayedAction> actions = new List<DelayedAction>();
-		PriorityQueue<DelayedAction> actions = new PriorityQueue<DelayedAction>();
 
-		public void Add(Action a) { Add(a, 0); }
+		public void Add(Action a, int desiredTime)
-		public void Add(Action a, int delay)
 		{
-			lock (syncRoot)
+			if (a == null)
-				actions.Add(new DelayedAction(a, Game.RunTime + delay));
+				throw new ArgumentNullException("a");
+
+			lock (actions)
+			{
+				var action = new DelayedAction(a, desiredTime);
+				var index = Index(action);
+				actions.Insert(index, action);
+			}
 		}
 
-		public void PerformActions()
+		public void PerformActions(int currentTime)
 		{
-			Action a = () => { };
+			DelayedAction[] pendingActions;
-			lock (syncRoot)
+			lock (actions)
 			{
-				var t = Game.RunTime;
+				var dummyAction = new DelayedAction(null, currentTime);
-				while (!actions.Empty && actions.Peek().Time <= t)
+				var index = Index(dummyAction);
-				{
+				if (index <= 0)
-					var da = actions.Pop();
+					return;
-					a = da.Action + a;
+
-				}
+				pendingActions = new DelayedAction[index];
+				actions.CopyTo(0, pendingActions, 0, index);
+				actions.RemoveRange(0, index);
 			}
 
-			a();
+			foreach (var delayedAction in pendingActions)
+				delayedAction.Action();
+		}
+
+		int Index(DelayedAction action)
+		{
+			// Returns the index of the next action with a strictly greater time.
+			var index = actions.BinarySearch(action);
+			if (index < 0)
+				return ~index;
+			while (index < actions.Count && action.CompareTo(actions[index]) >= 0)
+				index++;
+			return index;
 		}
 	}
 
 	struct DelayedAction : IComparable<DelayedAction>
 	{
-		public int Time;
+		public readonly int Time;
-		public Action Action;
+		public readonly Action Action;
 
 		public DelayedAction(Action action, int time)
 		{
@@ -59,5 +79,10 @@ namespace OpenRA.Primitives
 		{
 			return Time.CompareTo(other.Time);
 		}
+
+		public override string ToString()
+		{
+			return "Time: " + Time + " Action: " + Action;
+		}
 	}
 }

OpenRA.Test/OpenRA.Game/ActionQueueTest.cs

@@ -0,0 +1,42 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2015 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation. For more information,
+ * see COPYING.
+ */
+#endregion
+
+using System.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using OpenRA.Primitives;
+
+namespace OpenRA.Test.OpenRA.Game
+{
+	[TestFixture]
+	class ActionQueueTest
+	{
+		[TestCase(TestName = "ActionQueue performs actions in order of time, then insertion order.")]
+		public void ActionsArePerformedOrderedByTimeThenByInsertionOrder()
+		{
+			var list = new List<int>();
+			var queue = new ActionQueue();
+			queue.Add(() => list.Add(1), 0);
+			queue.Add(() => list.Add(7), 2);
+			queue.Add(() => list.Add(8), 2);
+			queue.Add(() => list.Add(4), 1);
+			queue.Add(() => list.Add(2), 0);
+			queue.Add(() => list.Add(3), 0);
+			queue.Add(() => list.Add(9), 2);
+			queue.Add(() => list.Add(5), 1);
+			queue.Add(() => list.Add(6), 1);
+			queue.PerformActions(1);
+			queue.PerformActions(2);
+			queue.PerformActions(3);
+			if (!list.SequenceEqual(new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9 }))
+				Assert.Fail("Actions were not performed in the correct order. Actual order was: " + string.Join(", ", list));
+		}
+	}
+}

OpenRA.Test/OpenRA.Test.csproj

@@ -45,6 +45,7 @@
     <Reference Include="System" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="OpenRA.Game\ActionQueueTest.cs" />
     <Compile Include="OpenRA.Game\MiniYamlTest.cs" />
     <Compile Include="OpenRA.Game\ActorInfoTest.cs" />
     <Compile Include="OpenRA.Game\CoordinateTest.cs" />