From d86959b32ea65fa09817d4710be25de21937524c Mon Sep 17 00:00:00 2001 From: IIIaKa Date: Mon, 3 Feb 2025 15:53:09 +0500 Subject: [PATCH] Hook conflicts and the call order of hooks in plugins 123 --- src/Plugins/Plugin.cs | 24 ++- src/Plugins/PluginManager.cs | 307 ++++++++++++++++++++++++++--------- 2 files changed, 251 insertions(+), 80 deletions(-) diff --git a/src/Plugins/Plugin.cs b/src/Plugins/Plugin.cs index 73a84e1c..1ebba4e1 100644 --- a/src/Plugins/Plugin.cs +++ b/src/Plugins/Plugin.cs @@ -185,16 +185,28 @@ protected Plugin() } /// - /// Subscribes this plugin to the specified hook + /// Subscribes this plugin to the specified hook. + /// If the plugin is already subscribed to the given hook and the index is >= 0, + /// the plugin will be moved to the specified index in the subscription list. + /// If the index is -1, the plugin will be appended to the end of the list. /// - /// - protected void Subscribe(string hook) => Manager.SubscribeToHook(hook, this); + /// The hook to subscribe to + /// The index in the subscription list where the plugin should be moved to, if >= 0. Default is -1(append to the end) + protected void Subscribe(string hookName, int subIndex = -1) => Manager.SubscribeToHook(hookName, this, subIndex); + /// - /// Unsubscribes this plugin to the specified hook + /// Unsubscribes this plugin from the specified hook /// - /// - protected void Unsubscribe(string hook) => Manager.UnsubscribeToHook(hook, this); + /// The hook to unsubscribe from + protected void Unsubscribe(string hookName) => Manager.UnsubscribeFromHook(hookName, this); + + /// + /// Adding the expected type for the hook. Works only for standard hooks(On and Can) + /// + /// The name of the hook to which it needs to be added + /// The type that needs to be added + public void AddTypeToHook(string hookName, Type type) => Manager.AddTypeToHook(hookName, type); /// /// Called when this plugin has been added to the specified manager diff --git a/src/Plugins/PluginManager.cs b/src/Plugins/PluginManager.cs index fa72eca7..47593d83 100644 --- a/src/Plugins/PluginManager.cs +++ b/src/Plugins/PluginManager.cs @@ -24,10 +24,13 @@ private struct SubscriptionChange public SubscriptionChangeType Change { get; } - public SubscriptionChange(Plugin plugin, SubscriptionChangeType type) + public int Index { get; } + + public SubscriptionChange(Plugin plugin, SubscriptionChangeType type, int subIndex = -1) { Plugin = plugin; Change = type; + Index = subIndex; } } @@ -37,13 +40,22 @@ private class HookSubscriptions public int CallDepth { get; set; } + public bool IsStandartHook { get; } + + public HashSet ExpectedTypes { get; } + public Queue PendingChanges { get; } - public HookSubscriptions() + public HookSubscriptions(HashSet types = null) { Plugins = new List(); - PendingChanges = new Queue(); CallDepth = 0; + if (types != null) + { + IsStandartHook = true; + ExpectedTypes = types; + } + PendingChanges = new Queue(); } } @@ -74,13 +86,36 @@ public HookSubscriptions() private readonly IDictionary hookSubscriptions; // Stores the last time a deprecation warning was printed for a specific hook - private readonly Dictionary lastDeprecatedWarningAt = new Dictionary(); + private readonly Dictionary nextDeprecatedWarningAt = new Dictionary(); // Re-usable conflict list used for hook calls private readonly List hookConflicts = new List(); + // A list of hooks(where conflicts exist between plugins) with the time for the next conflict check + private readonly Dictionary nextHookConflictsCheckAt = new Dictionary(StringComparer.OrdinalIgnoreCase); + private IArrayPoolProvider ObjectPool { get; } + // Dictionary of saved types with their associated hooks + private readonly Dictionary> expectedHookTypes = new Dictionary>() + { + { typeof(bool), new HashSet(StringComparer.OrdinalIgnoreCase) + { + "OnSaveLoad", "OnEntityActiveCheck", "OnEntityDistanceCheck", "OnEntityFromOwnerCheck", "OnEntityVisibilityCheck", + "OnAIBrainStateSwitch", "OnEyePosValidate", "OnFuelCheck", "OnHorseHitch", "OnItemCraft", + "OnMagazineReload", "OnPurchaseItem", "OnVendingTransaction", "OnWireClear", "OnRackedWeaponMount" + } }, + { typeof(int), new HashSet(StringComparer.OrdinalIgnoreCase) + { + "OnFuelGetAmount", "OnFuelUse", "OnInventoryItemsCount", "OnInventoryItemsTake", "OnItemResearched", + "OnItemUse", "OnMaxStackable", "OnResearchCostDetermine" + } }, + { typeof(float), new HashSet(StringComparer.OrdinalIgnoreCase) + { + "OnOvenTemperature", "OnExplosiveFuseSet" + } } + }; + /// /// Initializes a new instance of the PluginManager class /// @@ -91,6 +126,28 @@ public PluginManager(Logger logger) loadedPlugins = new Dictionary(); hookSubscriptions = new Dictionary(); Logger = logger; + + // Safe addition of default types unknown to PluginManager + AddDefaultHookTypes("Item, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", + "OnItemSplit", "OnFuelItemCheck", "OnFindBurnable", "OnInventoryItemFind", "OnInventoryAmmoItemFind", "OnDispenserBonus", "OnQuarryConsumeFuel", "OnFishCatch"); + AddDefaultHookTypes("BuildingPrivlidge, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", "OnBuildingPrivilege"); + AddDefaultHookTypes("BaseCorpse, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", "OnCorpsePopulate"); + AddDefaultHookTypes("SleepingBag, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", "OnPlayerRespawn"); + AddDefaultHookTypes("BasePlayer+SpawnPoint, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", "OnFindSpawnPoint", "OnPlayerRespawn"); + AddDefaultHookTypes("ItemContainer+CanAcceptResult, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", "CanAcceptItem"); + + string listType = System.Text.RegularExpressions.Regex.Replace(typeof(List).AssemblyQualifiedName, @"\[\[.*?\]\]", "[[{0}]]"); + AddDefaultHookTypes(string.Format(listType, "Item, Assembly-CSharp, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null"), "OnInventoryItemsFind"); + AddDefaultHookTypes(string.Format(listType, "UnityEngine.Vector3, UnityEngine.CoreModule, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null"), "OnBoatPathGenerate"); + + void AddDefaultHookTypes(string typeName, params string[] hooks) + { + var type = Type.GetType(typeName); + if (type != null) + { + expectedHookTypes[type] = new HashSet(hooks, StringComparer.OrdinalIgnoreCase); + } + } } /// @@ -157,50 +214,52 @@ public Plugin GetPlugin(string name) public IEnumerable GetPlugins() => loadedPlugins.Values; /// - /// Subscribes the specified plugin to the specified hook + /// Subscribes the specified plugin to the specified hook. + /// If the plugin is already subscribed to the given hook and the index is >= 0, + /// the plugin will be moved to the specified index in the subscription list. + /// If the index is -1, the plugin will be appended to the end of the list. /// - /// - /// - internal void SubscribeToHook(string hook, Plugin plugin) + /// The hook to subscribe to + /// The plugin to subscribe + /// The index in the subscription list where the plugin should be moved to, if >= 0. Default is -1(append to the end) + internal void SubscribeToHook(string hookName, Plugin plugin, int subIndex = -1) { - if (!loadedPlugins.ContainsKey(plugin.Name) || !plugin.IsCorePlugin && (hook.StartsWith("IOn") || hook.StartsWith("ICan"))) + if (!loadedPlugins.ContainsKey(plugin.Name) || !plugin.IsCorePlugin && (hookName.StartsWith("IOn") || hookName.StartsWith("ICan"))) { return; } - HookSubscriptions sublist; + HookSubscriptions subscriptions; lock (hookSubscriptions) { - if (!hookSubscriptions.TryGetValue(hook, out sublist)) + if (!hookSubscriptions.TryGetValue(hookName, out subscriptions)) { - sublist = new HookSubscriptions(); - hookSubscriptions[hook] = sublist; + hookSubscriptions[hookName] = subscriptions = new HookSubscriptions(GetHookTypes(hookName)); + /*if (subscriptions.IsStandartHook) + Logger.Write(LogType.Debug, $"Hook '{hookName}' expect next types({subscriptions.ExpectedTypes.Count()}): {string.Join(", ", subscriptions.ExpectedTypes.Select(t => t.ToString()).ToArray())}");*/ } } // Avoids modifying the plugin list while iterating over it during a hook call - if (sublist.CallDepth > 0) + if (subscriptions.CallDepth > 0) { - sublist.PendingChanges.Enqueue(new SubscriptionChange(plugin, SubscriptionChangeType.Subscribe)); - return; + subscriptions.PendingChanges.Enqueue(new SubscriptionChange(plugin, SubscriptionChangeType.Subscribe, subIndex)); } - - if (!sublist.Plugins.Contains(plugin)) + else { - sublist.Plugins.Add(plugin); + TryAddPluginToHook(subscriptions, plugin, subIndex); } - //Logger.Write(LogType.Debug, $"Plugin {plugin.Name} is subscribing to hook '{hook}'!"); } /// - /// Unsubscribes the specified plugin to the specified hook + /// Unsubscribes the specified plugin from the specified hook /// - /// - /// - internal void UnsubscribeToHook(string hook, Plugin plugin) + /// The hook to unsubscribe from + /// The plugin to unsubscribe + internal void UnsubscribeFromHook(string hookName, Plugin plugin) { - if (!loadedPlugins.ContainsKey(plugin.Name) || !plugin.IsCorePlugin && (hook.StartsWith("IOn") || hook.StartsWith("ICan"))) + if (!loadedPlugins.ContainsKey(plugin.Name) || !plugin.IsCorePlugin && (hookName.StartsWith("IOn") || hookName.StartsWith("ICan"))) { return; } @@ -209,7 +268,7 @@ internal void UnsubscribeToHook(string hook, Plugin plugin) lock (hookSubscriptions) { - if (!hookSubscriptions.TryGetValue(hook, out sublist)) + if (!hookSubscriptions.TryGetValue(hookName, out sublist)) { return; } @@ -223,29 +282,70 @@ internal void UnsubscribeToHook(string hook, Plugin plugin) } sublist.Plugins.Remove(plugin); - //Logger.Write(LogType.Debug, $"Plugin {plugin.Name} is unsubscribing to hook '{hook}'!"); + //Logger.Write(LogType.Debug, $"Plugin '{plugin.Name}' is unsubscribing from hook '{hook}'!"); + } + + /// + /// Adding the expected type for the hook. Works only for standard hooks(On and Can) + /// + /// The name of the hook to which it needs to be added + /// The type that needs to be added + public void AddTypeToHook(string hookName, Type type) + { + // The type to add is null or the hook is non-standard(!On and !Can) + if (type == null || (!hookName.StartsWith("On", StringComparison.OrdinalIgnoreCase) && !hookName.StartsWith("Can", StringComparison.OrdinalIgnoreCase))) + { + return; + } + + // Adding type to the dictionary of Types + if (!expectedHookTypes.TryGetValue(type, out var hooks)) + { + expectedHookTypes[type] = hooks = new HashSet(StringComparer.OrdinalIgnoreCase); + } + + // Failed to add because it is already present in this list and no actions are needed, exiting + if (!hooks.Add(hookName)) + { + return; + } + + HookSubscriptions subscriptions; + + lock (hookSubscriptions) + { + if (!hookSubscriptions.TryGetValue(hookName, out subscriptions)) + { + return; + } + } + + // Adding a Type to an existing subscribed hook + if (subscriptions.IsStandartHook) + { + subscriptions.ExpectedTypes.Add(type); + } } /// /// Calls a hook on all plugins of this manager /// - /// + /// /// /// - public object CallHook(string hook, params object[] args) + public object CallHook(string hookName, params object[] args) { HookSubscriptions subscriptions; // Locate the sublist lock (hookSubscriptions) { - if (!hookSubscriptions.TryGetValue(hook, out subscriptions)) + if (!hookSubscriptions.TryGetValue(hookName, out subscriptions)) { return null; } } - if (subscriptions.Plugins.Count == 0) { return null; @@ -253,38 +353,46 @@ public object CallHook(string hook, params object[] args) // Loop each item object[] values = ObjectPool.Take(subscriptions.Plugins.Count); - int returnCount = 0; - object finalValue = null; - Plugin finalPlugin = null; - subscriptions.CallDepth++; + object returnValue = null;// The value that will be returned, it will remain null if all plugins return null values + object lastValue = null;// The last non-null value for non-standard(!On and !Can) hooks or valid types + Plugin lastPlugin = null;// The last plugin with a non-null value for non-standard(!On and !Can) hooks or valid types + int validCount = 0;// The count of non-null values for non-standard(!On and !Can) hooks or valid types + subscriptions.CallDepth++; try { for (int i = 0; i < subscriptions.Plugins.Count; i++) { Plugin plugin = subscriptions.Plugins[i]; // Call the hook - object value = plugin.CallHook(hook, args); + object value = plugin.CallHook(hookName, args); if (value != null) { - values[i] = value; - finalValue = value; - finalPlugin = plugin; - returnCount++; + returnValue = value;// Assigning the last non-null value of ANY type + + // Assigning non-null values for non-standard(!On and !Can) hooks or VALID types + if (!subscriptions.IsStandartHook || subscriptions.ExpectedTypes.Contains(value.GetType())) + { + values[i] = value; + lastValue = value; + lastPlugin = plugin; + validCount++; + } } } - // Is there a return value? - if (returnCount == 0) + // Assigning the last valid value to the return value in case the last iterated plugin returns an invalid value + if (validCount > 0) { - ObjectPool.Return(values); - return null; + returnValue = lastValue; } - if (returnCount > 1 && finalValue != null) + // The number of non-null values of non-standard(!On and !Can) or VALID types is greater than 0, the last hook conflict time was not found or has already passed + // If a hook conflict is present, it won't go away quickly, and performing constant checks for such hooks is unnecessary and causes spam in the console, especially for hooks like CanBeTargeted + if (validCount > 0 && (!nextHookConflictsCheckAt.TryGetValue(hookName, out float nextConflictAt) || nextConflictAt <= Interface.Oxide.Now)) { - // Notify log of hook conflict + // Perform a hook conflict check hookConflicts.Clear(); for (int i = 0; i < subscriptions.Plugins.Count; i++) { @@ -297,25 +405,32 @@ public object CallHook(string hook, params object[] args) if (value.GetType().IsValueType) { - if (!values[i].Equals(finalValue)) + if (!values[i].Equals(lastValue)) { hookConflicts.Add($"{plugin.Name} - {value} ({value.GetType().Name})"); } } else { - if (values[i] != finalValue) + if (values[i] != lastValue) { hookConflicts.Add($"{plugin.Name} - {value} ({value.GetType().Name})"); } } } + + // The number of conflicting values is greater than 0 if (hookConflicts.Count > 0) { - hookConflicts.Add($"{finalPlugin.Name} ({finalValue} ({finalValue.GetType().Name}))"); - Logger.Write(LogType.Warning, "Calling hook {0} resulted in a conflict between the following plugins: {1}", hook, string.Join(", ", hookConflicts.ToArray())); + // Notify the log about it + hookConflicts.Add($"{lastPlugin.Name} ({lastValue} ({lastValue.GetType().Name}))"); + Logger.Write(LogType.Warning, "Calling hook '{0}' resulted in a conflict between the following plugins: {1}", hookName, string.Join(", ", hookConflicts.ToArray())); + + // Setting the time for which the hook conflict check will be ignored to avoid unnecessary checks and console spam + nextHookConflictsCheckAt[hookName] = Interface.Oxide.Now + 60f; } } + ObjectPool.Return(values); } finally @@ -323,31 +438,24 @@ public object CallHook(string hook, params object[] args) subscriptions.CallDepth--; if (subscriptions.CallDepth == 0) { - ProcessHookChanges(subscriptions); - } - } - - return finalValue; - } - - private void ProcessHookChanges(HookSubscriptions subscriptions) - { - while (subscriptions.PendingChanges.Count != 0) - { - SubscriptionChange change = subscriptions.PendingChanges.Dequeue(); - - if (change.Change == SubscriptionChangeType.Subscribe) - { - if (!subscriptions.Plugins.Contains(change.Plugin)) + // ProcessHookChanges + while (subscriptions.PendingChanges.Count != 0) { - subscriptions.Plugins.Add(change.Plugin); + SubscriptionChange change = subscriptions.PendingChanges.Dequeue(); + + if (change.Change == SubscriptionChangeType.Subscribe) + { + TryAddPluginToHook(subscriptions, change.Plugin, change.Index); + } + else + { + subscriptions.Plugins.Remove(change.Plugin); + } } } - else - { - subscriptions.Plugins.Remove(change.Plugin); - } } + + return returnValue; } /// @@ -377,14 +485,65 @@ public object CallDeprecatedHook(string oldHook, string newHook, DateTime expire } float now = Interface.Oxide.Now; - if (!lastDeprecatedWarningAt.TryGetValue(oldHook, out float lastWarningAt) || now - lastWarningAt > 3600f) + if (!nextDeprecatedWarningAt.TryGetValue(oldHook, out float nextWarningAt) || nextWarningAt <= now) { // TODO: Add better handling - lastDeprecatedWarningAt[oldHook] = now; - Interface.Oxide.LogWarning($"'{subscriptions.Plugins[0].Name} v{subscriptions.Plugins[0].Version}' is using deprecated hook '{oldHook}', which will stop working on {expireDate.ToString("D")}. Please ask the author to update to '{newHook}'"); + var plugin = subscriptions.Plugins[0]; + Interface.Oxide.LogWarning($"'{plugin.Name} v{plugin.Version}' is using deprecated hook '{oldHook}', which will stop working on {expireDate.ToString("D")}. Please contact the author '{plugin.Author}' to update to '{newHook}'"); + + // Setting the time for which notifications about deprecated hooks will be ignored + nextDeprecatedWarningAt[oldHook] = now + 3600f; } return CallHook(oldHook, args); } + + // Retrieving saved Types from the dictionary by hook name + private HashSet GetHookTypes(string hookName) + { + // The hook doesn't start with On or Can, meaning it's not a standard hook, returning null + if (!hookName.StartsWith("On", StringComparison.OrdinalIgnoreCase) && !hookName.StartsWith("Can", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + var result = new HashSet(); + + // The hook starts with Can, which means it's a boolean hook + if (hookName.StartsWith("Can", StringComparison.OrdinalIgnoreCase)) + { + result.Add(typeof(bool)); + } + + foreach (var kvp in expectedHookTypes) + { + if (kvp.Value.Contains(hookName)) + { + result.Add(kvp.Key); + } + } + return result; + } + + // Plugin subscription to a hook, with the option to specify the queue in the list + // The ability to specify the call order is mainly needed for API plugins, where hooks should be called before others, such as when a player connects + private void TryAddPluginToHook(HookSubscriptions subscriptions, Plugin plugin, int subIndex) + { + //Logger.Write(LogType.Debug, $"Plugin '{plugin.Name}' is subscribing to hook '{hook}'!"); + int currentIndex = subscriptions.Plugins.IndexOf(plugin); + if (currentIndex == -1 || (subIndex >= 0 && subIndex != currentIndex && subscriptions.Plugins.Remove(plugin))) + { + // If the current index is -1, it means the plugin is not in the list, so we simply add it depending on the specified index + // Otherwise, if the subscriber has indicated a desired index(index >= 0), we move the plugin to the specified index + if (subIndex < 0 || subIndex >= subscriptions.Plugins.Count) + { + subscriptions.Plugins.Add(plugin); + } + else + { + subscriptions.Plugins.Insert(subIndex, plugin); + } + } + } } }