improve new event code

This commit...
* debounces the has-custom-priorities check;
* fixes collection-modified-during-enumeration errors if an event handler is added or removed while the event is being raised;
* fixes Remove(handler) removing all instances of the handler instead of the last one.
This commit is contained in:
Jesse Plamondon-Willard 2020-06-16 20:14:27 -04:00
parent f63f14c703
commit c41b92f721
No known key found for this signature in database
GPG Key ID: CF8B1456B3E29F49
2 changed files with 33 additions and 29 deletions

View File

@ -14,23 +14,23 @@ namespace StardewModdingAPI.Framework.Events
/********* /*********
** Fields ** Fields
*********/ *********/
/// <summary>The underlying event handlers.</summary>
private readonly List<ManagedEventHandler<TEventArgs>> EventHandlers = new List<ManagedEventHandler<TEventArgs>>();
/// <summary>The mod registry with which to identify mods.</summary> /// <summary>The mod registry with which to identify mods.</summary>
protected readonly ModRegistry ModRegistry; protected readonly ModRegistry ModRegistry;
/// <summary>Tracks performance metrics.</summary> /// <summary>Tracks performance metrics.</summary>
private readonly PerformanceMonitor PerformanceMonitor; private readonly PerformanceMonitor PerformanceMonitor;
/// <summary>The underlying event handlers.</summary>
private readonly List<ManagedEventHandler<TEventArgs>> Handlers = new List<ManagedEventHandler<TEventArgs>>();
/// <summary>A cached snapshot of <see cref="Handlers"/>, or <c>null</c> to rebuild it next raise.</summary>
private ManagedEventHandler<TEventArgs>[] CachedHandlers = new ManagedEventHandler<TEventArgs>[0];
/// <summary>The total number of event handlers registered for this events, regardless of whether they're still registered.</summary> /// <summary>The total number of event handlers registered for this events, regardless of whether they're still registered.</summary>
private int RegistrationIndex; private int RegistrationIndex;
/// <summary>Whether any registered event handlers have a custom priority value.</summary> /// <summary>Whether new handlers were added since the last raise.</summary>
private bool HasCustomPriorities; private bool HasNewHandlers;
/// <summary>Whether event handlers should be sorted before the next invocation.</summary>
private bool NeedsSort;
/********* /*********
@ -62,7 +62,7 @@ namespace StardewModdingAPI.Framework.Events
/// <summary>Get whether anything is listening to the event.</summary> /// <summary>Get whether anything is listening to the event.</summary>
public bool HasListeners() public bool HasListeners()
{ {
return this.EventHandlers.Count > 0; return this.Handlers.Count > 0;
} }
/// <summary>Add an event handler.</summary> /// <summary>Add an event handler.</summary>
@ -73,19 +73,25 @@ namespace StardewModdingAPI.Framework.Events
EventPriority priority = handler.Method.GetCustomAttribute<EventPriorityAttribute>()?.Priority ?? EventPriority.Normal; EventPriority priority = handler.Method.GetCustomAttribute<EventPriorityAttribute>()?.Priority ?? EventPriority.Normal;
var managedHandler = new ManagedEventHandler<TEventArgs>(handler, this.RegistrationIndex++, priority, mod); var managedHandler = new ManagedEventHandler<TEventArgs>(handler, this.RegistrationIndex++, priority, mod);
this.EventHandlers.Add(managedHandler); this.Handlers.Add(managedHandler);
this.HasCustomPriorities = this.HasCustomPriorities || managedHandler.HasCustomPriority(); this.CachedHandlers = null;
this.HasNewHandlers = true;
if (this.HasCustomPriorities)
this.NeedsSort = true;
} }
/// <summary>Remove an event handler.</summary> /// <summary>Remove an event handler.</summary>
/// <param name="handler">The event handler.</param> /// <param name="handler">The event handler.</param>
public void Remove(EventHandler<TEventArgs> handler) public void Remove(EventHandler<TEventArgs> handler)
{ {
this.EventHandlers.RemoveAll(p => p.Handler == handler); // match C# events: if a handler is listed multiple times, remove the last one added
this.HasCustomPriorities = this.HasCustomPriorities && this.EventHandlers.Any(p => p.HasCustomPriority()); for (int i = this.Handlers.Count - 1; i >= 0; i--)
{
if (this.Handlers[i].Handler != handler)
continue;
this.Handlers.RemoveAt(i);
this.CachedHandlers = null;
break;
}
} }
/// <summary>Raise the event and notify all handlers.</summary> /// <summary>Raise the event and notify all handlers.</summary>
@ -94,21 +100,25 @@ namespace StardewModdingAPI.Framework.Events
public void Raise(TEventArgs args, Func<IModMetadata, bool> match = null) public void Raise(TEventArgs args, Func<IModMetadata, bool> match = null)
{ {
// skip if no handlers // skip if no handlers
if (this.EventHandlers.Count == 0) if (this.Handlers.Count == 0)
return; return;
// sort event handlers by priority // update cached data
// (This is done here to avoid repeatedly sorting when handlers are added/removed.) // (This is debounced here to avoid repeatedly sorting when handlers are added/removed,
if (this.NeedsSort) // and keeping a separate cached list allows changes during enumeration.)
if (this.CachedHandlers == null)
{ {
this.NeedsSort = false; if (this.HasNewHandlers && this.Handlers.Any(p => p.Priority != EventPriority.Normal))
this.EventHandlers.Sort(); this.Handlers.Sort();
this.CachedHandlers = this.Handlers.ToArray();
this.HasNewHandlers = false;
} }
// raise event // raise event
this.PerformanceMonitor.Track(this.EventName, () => this.PerformanceMonitor.Track(this.EventName, () =>
{ {
foreach (ManagedEventHandler<TEventArgs> handler in this.EventHandlers) foreach (ManagedEventHandler<TEventArgs> handler in this.CachedHandlers)
{ {
if (match != null && !match(handler.SourceMod)) if (match != null && !match(handler.SourceMod))
continue; continue;

View File

@ -39,12 +39,6 @@ namespace StardewModdingAPI.Framework.Events
this.SourceMod = sourceMod; this.SourceMod = sourceMod;
} }
/// <summary>Get whether the event handler has a custom priority value.</summary>
public bool HasCustomPriority()
{
return this.Priority != EventPriority.Normal;
}
/// <summary>Compares the current instance with another object of the same type and returns an integer that indicates whether the current instance precedes, follows, or occurs in the same position in the sort order as the other object.</summary> /// <summary>Compares the current instance with another object of the same type and returns an integer that indicates whether the current instance precedes, follows, or occurs in the same position in the sort order as the other object.</summary>
/// <param name="obj">An object to compare with this instance.</param> /// <param name="obj">An object to compare with this instance.</param>
/// <exception cref="T:System.ArgumentException"><paramref name="obj" /> is not the same type as this instance.</exception> /// <exception cref="T:System.ArgumentException"><paramref name="obj" /> is not the same type as this instance.</exception>