From c1cfaf520d04dc939376700dab5f7bbe1d743c95 Mon Sep 17 00:00:00 2001 From: ORelio Date: Sat, 15 May 2021 16:31:02 +0200 Subject: [PATCH] Improve InvokeOnMainThread mechanism Add documentation to make the invoke mechanism easier to understand Make it clear in documentation that code is invoked synchronously Use Action and Func for minimizing the amount of code to write Use type parameter T to automatically adjust return value type Throw exceptions on the calling thread, not the main thread --- MinecraftClient/ChatBot.cs | 13 +- MinecraftClient/McClient.cs | 44 ++++--- .../Protocol/Handlers/Protocol18.cs | 10 +- .../Protocol/Handlers/Protocol18Terrain.cs | 20 +-- .../Protocol/IMinecraftComHandler.cs | 21 +++- MinecraftClient/TaskWithResult.cs | 115 +++++++++++++++--- 6 files changed, 164 insertions(+), 59 deletions(-) diff --git a/MinecraftClient/ChatBot.cs b/MinecraftClient/ChatBot.cs index aafc2942..e15bd771 100644 --- a/MinecraftClient/ChatBot.cs +++ b/MinecraftClient/ChatBot.cs @@ -1402,12 +1402,13 @@ namespace MinecraftClient /// Console.WriteLine("10 seconds has passed"); /// }), 100); /// - protected void ScheduleTaskDelayed(Delegate task, int delayTicks = 0) + // TODO: Adapt to new IMinecraftComHandler API + /*protected void ScheduleTaskDelayed(Delegate task, int delayTicks = 0) { if (delayTicks <= 0) { // Immediately schedule to run on next update - Handler.ScheduleTask(task); + Handler.InvokeOnMainThread(task); } else { @@ -1416,17 +1417,19 @@ namespace MinecraftClient delayTasks.Add(new DelayedTask(task, delayTicks)); } } - } + }*/ /// /// Schedule a task to run on main thread. /// /// Task to run /// Any value returned from the task + // TODO: Adapt to new IMinecraftComHandler API + /* protected object ScheduleTask(Delegate task) { - return Handler.ScheduleTask(task); - } + return Handler.InvokeOnMainThread(task); + }*/ /// /// Command runner definition. diff --git a/MinecraftClient/McClient.cs b/MinecraftClient/McClient.cs index a19763d4..d825c95d 100644 --- a/MinecraftClient/McClient.cs +++ b/MinecraftClient/McClient.cs @@ -32,7 +32,7 @@ namespace MinecraftClient private Queue chatQueue = new Queue(); private static DateTime nextMessageSendTime = DateTime.MinValue; - private Queue threadTasks = new Queue(); + private Queue threadTasks = new Queue(); private object threadTasksLock = new object(); private readonly List bots = new List(); @@ -302,7 +302,6 @@ namespace MinecraftClient /// /// Allows the user to send chat messages, commands, and leave the server. - /// Enqueue text typed in the command prompt for processing on the main thread. /// private void CommandPrompt() { @@ -312,7 +311,7 @@ namespace MinecraftClient while (client.Client.Connected) { string text = ConsoleIO.ReadLine(); - ScheduleTask(new Action(() => { HandleCommandPromptText(text); })); + InvokeOnMainThread(() => HandleCommandPromptText(text)); } } catch (IOException) { } @@ -641,14 +640,12 @@ namespace MinecraftClient SendRespawnPacket(); } - lock (threadTasksLock) { while (threadTasks.Count > 0) { - var taskToRun = threadTasks.Dequeue(); - taskToRun.Execute(); - taskToRun.Release(); + Action taskToRun = threadTasks.Dequeue(); + taskToRun(); } } } @@ -696,28 +693,43 @@ namespace MinecraftClient } /// - /// Schedule a task to run on the main thread + /// Invoke a task on the main thread, wait for completion and retrieve return value. /// - /// Task to run - /// Any result returned from delegate - public object ScheduleTask(Delegate task) + /// Task to run with any type or return value + /// Any result returned from task, result type is inferred from the task + /// bool result = InvokeOnMainThread(methodThatReturnsAbool); + /// bool result = InvokeOnMainThread(() => methodThatReturnsAbool(argument)); + /// int result = InvokeOnMainThread(() => { yourCode(); return 42; }); + /// Type of the return value + public T InvokeOnMainThread(Func task) { if (!InvokeRequired()) { - return task.DynamicInvoke(); + return task(); } else { - var taskAndResult = new TaskWithResult(task); + TaskWithResult taskWithResult = new TaskWithResult(task); lock (threadTasksLock) { - threadTasks.Enqueue(taskAndResult); + threadTasks.Enqueue(taskWithResult.ExecuteSynchronously); } - taskAndResult.Block(); - return taskAndResult.Result; + return taskWithResult.WaitGetResult(); } } + /// + /// Invoke a task on the main thread and wait for completion + /// + /// Task to run without return value + /// InvokeOnMainThread(methodThatReturnsNothing); + /// InvokeOnMainThread(() => methodThatReturnsNothing(argument)); + /// InvokeOnMainThread(() => { yourCode(); }); + public void InvokeOnMainThread(Action task) + { + InvokeOnMainThread(() => { task(); return true; }); + } + /// /// Check if calling thread is main thread or other thread /// diff --git a/MinecraftClient/Protocol/Handlers/Protocol18.cs b/MinecraftClient/Protocol/Handlers/Protocol18.cs index 7a1e087b..5018080b 100644 --- a/MinecraftClient/Protocol/Handlers/Protocol18.cs +++ b/MinecraftClient/Protocol/Handlers/Protocol18.cs @@ -423,10 +423,9 @@ namespace MinecraftClient.Protocol.Handlers int compressedDataSize = dataTypes.ReadNextInt(packetData); byte[] compressed = dataTypes.ReadData(compressedDataSize, packetData); byte[] decompressed = ZlibUtils.Decompress(compressed); - new Task(new Action(() => - { + new Thread(() => { pTerrain.ProcessChunkColumnData(chunkX, chunkZ, chunkMask, addBitmap, currentDimension == 0, chunksContinuous, currentDimension, new Queue(decompressed)); - })).Start(); + }).Start(); } else { @@ -450,10 +449,9 @@ namespace MinecraftClient.Protocol.Handlers else dataTypes.ReadData(1024 * 4, packetData); // Biomes - 1.15 and above } int dataSize = dataTypes.ReadNextVarInt(packetData); - new Task(new Action(() => - { + new Thread(() => { pTerrain.ProcessChunkColumnData(chunkX, chunkZ, chunkMask, 0, false, chunksContinuous, currentDimension, packetData); - })).Start(); + }).Start(); } } break; diff --git a/MinecraftClient/Protocol/Handlers/Protocol18Terrain.cs b/MinecraftClient/Protocol/Handlers/Protocol18Terrain.cs index 8a0f92a9..14202599 100644 --- a/MinecraftClient/Protocol/Handlers/Protocol18Terrain.cs +++ b/MinecraftClient/Protocol/Handlers/Protocol18Terrain.cs @@ -162,12 +162,12 @@ namespace MinecraftClient.Protocol.Handlers } //We have our chunk, save the chunk into the world - handler.ScheduleTask(new Action(() => + handler.InvokeOnMainThread(() => { if (handler.GetWorld()[chunkX, chunkZ] == null) handler.GetWorld()[chunkX, chunkZ] = new ChunkColumn(); handler.GetWorld()[chunkX, chunkZ][chunkY] = chunk; - })); + }); //Pre-1.14 Lighting data if (protocolversion < Protocol18Handler.MC114Version) @@ -192,10 +192,10 @@ namespace MinecraftClient.Protocol.Handlers if (chunksContinuous && chunkMask == 0) { //Unload the entire chunk column - handler.ScheduleTask(new Action(() => + handler.InvokeOnMainThread(() => { handler.GetWorld()[chunkX, chunkZ] = null; - })); + }); } else { @@ -214,12 +214,12 @@ namespace MinecraftClient.Protocol.Handlers chunk[blockX, blockY, blockZ] = new Block(queue.Dequeue()); //We have our chunk, save the chunk into the world - handler.ScheduleTask(new Action(() => + handler.InvokeOnMainThread(() => { if (handler.GetWorld()[chunkX, chunkZ] == null) handler.GetWorld()[chunkX, chunkZ] = new ChunkColumn(); handler.GetWorld()[chunkX, chunkZ][chunkY] = chunk; - })); + }); } } @@ -248,10 +248,10 @@ namespace MinecraftClient.Protocol.Handlers if (chunksContinuous && chunkMask == 0) { //Unload the entire chunk column - handler.ScheduleTask(new Action(() => + handler.InvokeOnMainThread(() => { handler.GetWorld()[chunkX, chunkZ] = null; - })); + }); } else { @@ -297,12 +297,12 @@ namespace MinecraftClient.Protocol.Handlers for (int blockX = 0; blockX < Chunk.SizeX; blockX++) chunk[blockX, blockY, blockZ] = new Block(blockTypes.Dequeue(), blockMeta.Dequeue()); - handler.ScheduleTask(new Action(() => + handler.InvokeOnMainThread(() => { if (handler.GetWorld()[chunkX, chunkZ] == null) handler.GetWorld()[chunkX, chunkZ] = new ChunkColumn(); handler.GetWorld()[chunkX, chunkZ][chunkY] = chunk; - })); + }); } } } diff --git a/MinecraftClient/Protocol/IMinecraftComHandler.cs b/MinecraftClient/Protocol/IMinecraftComHandler.cs index e34dc490..6b4072a2 100644 --- a/MinecraftClient/Protocol/IMinecraftComHandler.cs +++ b/MinecraftClient/Protocol/IMinecraftComHandler.cs @@ -41,11 +41,24 @@ namespace MinecraftClient.Protocol ILogger GetLogger(); /// - /// Schedule a task to run on the main thread + /// Invoke a task on the main thread, wait for completion and retrieve return value. /// - /// Task to run - /// Any result returned from delegate - object ScheduleTask(Delegate task); + /// Task to run with any type or return value + /// Any result returned from task, result type is inferred from the task + /// bool result = InvokeOnMainThread(methodThatReturnsAbool); + /// bool result = InvokeOnMainThread(() => methodThatReturnsAbool(argument)); + /// int result = InvokeOnMainThread(() => { yourCode(); return 42; }); + /// Type of the return value + T InvokeOnMainThread(Func task); + + /// + /// Invoke a task on the main thread and wait for completion + /// + /// Task to run without return value + /// InvokeOnMainThread(methodThatReturnsNothing); + /// InvokeOnMainThread(() => methodThatReturnsNothing(argument)); + /// InvokeOnMainThread(() => { yourCode(); }); + void InvokeOnMainThread(Action task); /// /// Called when a network packet received or sent diff --git a/MinecraftClient/TaskWithResult.cs b/MinecraftClient/TaskWithResult.cs index c68a5014..2762fb28 100644 --- a/MinecraftClient/TaskWithResult.cs +++ b/MinecraftClient/TaskWithResult.cs @@ -6,42 +6,121 @@ using System.Threading; namespace MinecraftClient { - public class TaskWithResult + /// + /// Holds an asynchronous task with return value + /// + /// Type of the return value + public class TaskWithResult { - private Delegate Task; - private AutoResetEvent ResultEvent = new AutoResetEvent(false); + private AutoResetEvent resultEvent = new AutoResetEvent(false); + private Func task; + private T result = default(T); + private Exception exception = null; + private bool taskRun = false; + private object taskRunLock = new object(); - public object Result; - - public TaskWithResult(Delegate task) + /// + /// Create a new asynchronous task with return value + /// + /// Delegate with return value + public TaskWithResult(Func task) { - Task = task; + this.task = task; } /// - /// Execute the delegate and set the property to the returned value + /// Check whether the task has finished running /// - /// Value returned from delegate - public object Execute() + public bool HasRun { - Result = Task.DynamicInvoke(); - return Result; + get + { + return taskRun; + } } /// - /// Block the program execution + /// Get the task result (return value of the inner delegate) /// - public void Block() + /// Thrown if the task is not finished yet + public T Result { - ResultEvent.WaitOne(); + get + { + if (taskRun) + { + return result; + } + else throw new InvalidOperationException("Attempting to retrieve the result of an unfinished task"); + } } /// - /// Resume the program execution + /// Get the exception thrown by the inner delegate, if any /// - public void Release() + public Exception Exception { - ResultEvent.Set(); + get + { + return exception; + } + } + + /// + /// Execute the task in the current thread and set the property or to the returned value + /// + public void ExecuteSynchronously() + { + // Make sur the task will not run twice + lock (taskRunLock) + { + if (taskRun) + { + throw new InvalidOperationException("Attempting to run a task twice"); + } + } + + // Run the task + try + { + result = task(); + } + catch (Exception e) + { + exception = e; + } + + // Mark task as complete and release wait event + lock (taskRunLock) + { + taskRun = true; + } + resultEvent.Set(); + } + + /// + /// Wait until the task has run from another thread and get the returned value or exception thrown by the task + /// + /// Task result once available + /// Any exception thrown by the task + public T WaitGetResult() + { + // Wait only if the result is not available yet + bool mustWait = false; + lock (taskRunLock) + { + mustWait = !taskRun; + } + if (mustWait) + { + resultEvent.WaitOne(); + } + + // Receive exception from task + if (exception != null) + throw exception; + + return result; } } }