Skip to content

Commit a50df67

Browse files
Banane9Banane9
committed
Fix collection modified exception when unregistering a mod
Also adds a bunch of logging and docs
1 parent 119751b commit a50df67

5 files changed

Lines changed: 184 additions & 24 deletions

File tree

‎MonkeyLoader/Events/AsyncEventDispatchers.cs‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,23 @@
88

99
namespace MonkeyLoader.Events
1010
{
11+
/// <summary>
12+
/// Concrete async dispatcher for <see cref="AsyncEvent"/>s.
13+
/// </summary>
14+
/// <inheritdoc cref="EventDispatcher{TEvent}"/>
1115
internal sealed class AsyncEventDispatcher<TEvent>
1216
: EventDispatcherBase<IAsyncEventSource<TEvent>, IAsyncEventHandler<TEvent>>
1317
where TEvent : AsyncEvent
1418
{
19+
/// <summary>
20+
/// Creates a new async dispatcher for the given <paramref name="manager"/>.
21+
/// </summary>
22+
/// <param name="manager">The manager that this dispatcher belongs to.</param>
1523
public AsyncEventDispatcher(EventManager manager)
1624
: base(manager, AccessTools.DeclaredMethod(typeof(AsyncEventDispatcher<TEvent>), nameof(RemoveSource)))
1725
{ }
1826

27+
/// <inheritdoc cref="EventDispatcher{TEvent}.AddSource"/>
1928
public bool AddSource<TDerivedEvent>(Mod mod, IAsyncEventSource<TDerivedEvent> eventSource)
2029
where TDerivedEvent : TEvent
2130
{
@@ -30,6 +39,7 @@ public bool AddSource<TDerivedEvent>(Mod mod, IAsyncEventSource<TDerivedEvent> e
3039
return true;
3140
}
3241

42+
/// <inheritdoc cref="EventDispatcher{TEvent}.RemoveSource"/>
3343
public bool RemoveSource<TDerivedEvent>(Mod mod, IAsyncEventSource<TDerivedEvent> eventSource)
3444
where TDerivedEvent : TEvent
3545
{
@@ -61,28 +71,38 @@ private AsyncEventDispatching<TDerivedEvent> MakeEventDispatcher<TDerivedEvent>(
6171
where TDerivedEvent : TEvent => new(DispatchEventsAsync);
6272
}
6373

74+
/// <summary>
75+
/// Concrete async dispatcher for <see cref="CancelableAsyncEvent"/>s.
76+
/// </summary>
77+
/// <inheritdoc cref="EventDispatcher{TEvent}"/>
6478
internal sealed class CancelableAsyncEventDispatcher<TEvent>
6579
: EventDispatcherBase<ICancelableAsyncEventSource<TEvent>, ICancelableAsyncEventHandler<TEvent>>
6680
where TEvent : CancelableAsyncEvent
6781
{
82+
/// <summary>
83+
/// Creates a new async dispatcher for the given <paramref name="manager"/>.
84+
/// </summary>
85+
/// <param name="manager">The manager that this dispatcher belongs to.</param>
6886
public CancelableAsyncEventDispatcher(EventManager manager)
6987
: base(manager, AccessTools.DeclaredMethod(typeof(CancelableAsyncEventDispatcher<TEvent>), nameof(RemoveSource)))
7088
{ }
7189

72-
public bool AddSource<TDerivedEvent>(Mod mod, ICancelableAsyncEventSource<TDerivedEvent> eventSource)
90+
/// <inheritdoc cref="EventDispatcher{TEvent}.AddSource"/>
91+
public bool AddSource<TDerivedEvent>(Mod mod, ICancelableAsyncEventSource<TDerivedEvent> source)
7392
where TDerivedEvent : TEvent
7493
{
75-
if (!AddSource(mod, typeof(TDerivedEvent), eventSource))
94+
if (!AddSource(mod, typeof(TDerivedEvent), source))
7695
return false;
7796

7897
// Have to wrap the DispatchEvents method in the correct delegate type,
7998
// otherwise the event will throw when adding it, despite being compatible
8099
var eventDispatcher = eventDispatchers.GetOrCreateValue(MakeEventDispatcher<TDerivedEvent>);
81-
eventSource.Dispatching += eventDispatcher;
100+
source.Dispatching += eventDispatcher;
82101

83102
return true;
84103
}
85104

105+
/// <inheritdoc cref="EventDispatcher{TEvent}.RemoveSource"/>
86106
public bool RemoveSource<TDerivedEvent>(Mod mod, ICancelableAsyncEventSource<TDerivedEvent> eventSource)
87107
where TDerivedEvent : TEvent
88108
{

‎MonkeyLoader/Events/DispatchableBaseEventAttribute.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace MonkeyLoader.Events
1010
/// Marks an <see cref="Event"/>-derived class as to be dispatched,
1111
/// even if it's only a base class of the concrete event coming from the source.
1212
/// </summary>
13+
// Todo: This could be a protected nested class in Event to enforce it only being used there
1314
[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
1415
public sealed class DispatchableBaseEventAttribute : MonkeyLoaderAttribute
1516
{ }

‎MonkeyLoader/Events/EventDispatcherBase.cs‎

Lines changed: 105 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,76 +7,174 @@
77

88
namespace MonkeyLoader.Events
99
{
10+
/// <summary>
11+
/// Represents the abstract base for the concrete dispatcher types.
12+
/// </summary>
13+
/// <typeparam name="TSource">The type of the event sources.</typeparam>
14+
/// <typeparam name="THandler">The type of the event handlers.</typeparam>
1015
internal abstract class EventDispatcherBase<TSource, THandler> : IEventDispatcher
1116
where THandler : class, IPrioritizable
1217
{
1318
protected readonly AnyMap eventDispatchers = new();
1419
protected readonly PrioritySortedCollection<THandler> handlers = [];
1520
protected readonly Dictionary<Mod, Dictionary<Type, HashSet<TSource>>> sourcesByMod = [];
21+
1622
private readonly Dictionary<Mod, HashSet<THandler>> _handlersByMod = [];
1723
private readonly EventManager _manager;
1824
private readonly MethodInfo _removeSourceMethod;
25+
1926
protected Logger Logger => _manager.Logger;
2027

28+
/// <summary>
29+
/// Creates a new dispatcher with the given details.
30+
/// </summary>
31+
/// <param name="manager">The manager that this dispatcher belongs to.</param>
32+
/// <param name="removeSourceMethod">The generic <c>RemoveSource</c> implementation of the concrete dispatcher.</param>
2133
protected EventDispatcherBase(EventManager manager, MethodInfo removeSourceMethod)
2234
{
2335
_manager = manager;
2436
_removeSourceMethod = removeSourceMethod;
2537
}
2638

39+
/// <summary>
40+
/// Adds the <paramref name="handler"/> from the given <paramref name="mod"/> to this dispatcher.
41+
/// </summary>
42+
/// <param name="mod">The mod that the <paramref name="handler"/> comes from.</param>
43+
/// <param name="handler">The <typeparamref name="THandler"/> to add.</param>
44+
/// <returns><c>true</c> if the handler was newly added; otherwise, <c>false</c>.</returns>
2745
public bool AddHandler(Mod mod, THandler handler)
2846
{
2947
if (_handlersByMod.GetOrCreateValue(mod).Add(handler))
3048
{
3149
handlers.Add(handler);
50+
Logger.Debug(() => $"Added handler [{handler.GetType().CompactDescription()}] to event source [{typeof(TSource).CompactDescription()}] for mod: {mod}!");
51+
3252
return true;
3353
}
3454

55+
Logger.Warn(() => $"Tried to add duplicate handler [{handler.GetType().CompactDescription()}] to event source [{typeof(TSource).CompactDescription()}] for mod: {mod}!");
56+
3557
return false;
3658
}
3759

60+
/// <summary>
61+
/// Removes the <typeparamref name="THandler"/> from the given <paramref name="mod"/> from this dispatcher.
62+
/// </summary>
63+
/// <param name="mod">The mod that the <paramref name="handler"/> comes from.</param>
64+
/// <param name="handler">The <typeparamref name="THandler"/> to remove.</param>
65+
/// <returns><c>true</c> if the handler was found and removed; otherwise, <c>false</c>.</returns>
3866
public bool RemoveHandler(Mod mod, THandler handler)
3967
{
4068
if (_handlersByMod.TryGetValue(mod, out var modHandlers))
4169
{
42-
modHandlers.Remove(handler);
43-
handlers.Remove(handler);
70+
if (modHandlers.Remove(handler))
71+
{
72+
handlers.Remove(handler);
73+
Logger.Debug(() => $"Removed handler [{handler.GetType().CompactDescription()}] from event source [{typeof(TSource).CompactDescription()}] for mod: {mod}!");
4474

45-
return true;
75+
return true;
76+
}
77+
78+
Logger.Warn(() => $"Tried to remove missing handler [{handler.GetType().CompactDescription()}] from event source [{typeof(TSource).CompactDescription()}] for missing mod: {mod}!");
79+
80+
return false;
4681
}
4782

83+
Logger.Warn(() => $"Tried to remove handler [{handler.GetType().CompactDescription()}] from event source [{typeof(TSource).CompactDescription()}] for missing mod: {mod}!");
84+
4885
return false;
4986
}
5087

88+
/// <summary>
89+
/// Removes all <typeparamref name="TSource"/>s and <typeparamref name="THandler"/>s
90+
/// of the given <paramref name="mod"/> from this dispatcher.
91+
/// </summary>
92+
/// <param name="mod">The mod that the sources and handlers come from.</param>
5193
public void UnregisterMod(Mod mod)
5294
{
95+
Logger.Debug(() => $"Removing all {typeof(TSource).CompactDescription()} sources of mod: {mod}!");
96+
5397
if (sourcesByMod.TryGetValue(mod, out var modSourcesByType))
5498
{
99+
List<Action> sourceRemovals = [];
100+
55101
foreach (var typeModSources in modSourcesByType)
56102
{
103+
Logger.Debug(() => $"Removing sources of event type: {typeModSources.Key.CompactDescription()}!");
104+
57105
var removeSource = _removeSourceMethod.MakeGenericMethod(typeModSources.Key);
58106

59107
foreach (var source in typeModSources.Value)
60-
removeSource.Invoke(this, [mod, source]);
108+
{
109+
Logger.Trace(() => $"Removing concrete source: {source!.GetType().CompactDescription()}!");
110+
111+
sourceRemovals.Add(() => removeSource.Invoke(this, [mod, source]));
112+
}
61113
}
114+
115+
foreach (var removeSource in sourceRemovals)
116+
removeSource();
62117
}
63118

64119
sourcesByMod.Remove(mod);
65120
_handlersByMod.Remove(mod);
66121
}
67122

123+
/// <summary>
124+
/// Adds the <paramref name="source"/> for the <paramref name="eventType"/>
125+
/// from the given <paramref name="mod"/> to this dispatcher.
126+
/// </summary>
127+
/// <param name="mod">The mod that the <paramref name="source"/> comes from.</param>
128+
/// <param name="eventType">The type of the event that the source is for.</param>
129+
/// <param name="source">The <typeparamref name="TSource"/> to add.</param>
130+
/// <returns><c>true</c> if the source was newly added; otherwise, <c>false</c>.</returns>
68131
protected bool AddSource(Mod mod, Type eventType, TSource source)
69-
=> sourcesByMod.GetOrCreateValue(mod).GetOrCreateValue(eventType).Add(source);
132+
{
133+
if (sourcesByMod.GetOrCreateValue(mod).GetOrCreateValue(eventType).Add(source))
134+
{
135+
Logger.Debug(() => $"Added source [{source?.GetType().CompactDescription()}] for event [{eventType.CompactDescription()}] for mod: {mod}!");
70136

137+
return true;
138+
}
139+
140+
Logger.Warn(() => $"Tried to add duplicate source [{source?.GetType().CompactDescription()}] for event [{eventType.CompactDescription()}] for mod: {mod}!");
141+
142+
return false;
143+
}
144+
145+
/// <summary>
146+
/// Removes the <paramref name="source"/> for the <paramref name="eventType"/>
147+
/// from the given <paramref name="mod"/> from this dispatcher.
148+
/// </summary>
149+
/// <param name="mod">The mod that the <paramref name="source"/> comes from.</param>
150+
/// <param name="eventType">The type of the event that the source is for.</param>
151+
/// <param name="source">The <typeparamref name="TSource"/> to remove.</param>
152+
/// <returns><c>true</c> if the source was found and removed; otherwise, <c>false</c>.</returns>
71153
protected bool RemoveSource(Mod mod, Type eventType, TSource source)
72154
{
73155
if (sourcesByMod.TryGetValue(mod, out var modSourcesByType))
74156
{
75157
if (modSourcesByType.TryGetValue(eventType, out var modTypeSources))
76-
modTypeSources.Remove(source);
77-
return true;
158+
{
159+
if (modTypeSources.Remove(source))
160+
{
161+
Logger.Debug(() => $"Removed source [{source!.GetType().CompactDescription()}] of event [{eventType.CompactDescription()}] for mod: {mod}!");
162+
163+
return true;
164+
}
165+
166+
Logger.Warn(() => $"Tried to remove missing source [{source!.GetType().CompactDescription()}] of event [{eventType.CompactDescription()}] for mod: {mod}!");
167+
168+
return false;
169+
}
170+
171+
Logger.Warn(() => $"Tried to remove source [{source!.GetType().CompactDescription()}] of missing event [{eventType.CompactDescription()}] for mod: {mod}!");
172+
173+
return false;
78174
}
79175

176+
Logger.Warn(() => $"Tried to remove source [{source!.GetType().CompactDescription()}] of event [{eventType.CompactDescription()}] for missing mod: {mod}!");
177+
80178
return false;
81179
}
82180
}

‎MonkeyLoader/Events/EventDispatchers.cs‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,36 +9,46 @@
99

1010
namespace MonkeyLoader.Events
1111
{
12+
/// <summary>
13+
/// Concrete dispatcher for <see cref="CancelableSyncEvent"/>s.
14+
/// </summary>
15+
/// <inheritdoc cref="EventDispatcher{TEvent}"/>
1216
internal sealed class CancelableEventDispatcher<TEvent>
1317
: EventDispatcherBase<ICancelableEventSource<TEvent>, ICancelableEventHandler<TEvent>>
1418
where TEvent : CancelableSyncEvent
1519
{
20+
/// <summary>
21+
/// Creates a new dispatcher for the given <paramref name="manager"/>.
22+
/// </summary>
23+
/// <param name="manager">The manager that this dispatcher belongs to.</param>
1624
public CancelableEventDispatcher(EventManager manager)
1725
: base(manager, AccessTools.DeclaredMethod(typeof(CancelableEventDispatcher<TEvent>), nameof(RemoveSource)))
1826
{ }
1927

20-
public bool AddSource<TDerivedEvent>(Mod mod, ICancelableEventSource<TDerivedEvent> eventSource)
28+
/// <inheritdoc cref="EventDispatcher{TEvent}.AddSource"/>
29+
public bool AddSource<TDerivedEvent>(Mod mod, ICancelableEventSource<TDerivedEvent> source)
2130
where TDerivedEvent : TEvent
2231
{
23-
if (!AddSource(mod, typeof(TDerivedEvent), eventSource))
32+
if (!AddSource(mod, typeof(TDerivedEvent), source))
2433
return false;
2534

2635
// Have to wrap the DispatchEvents method in the correct delegate type,
2736
// otherwise the event will throw when adding it, despite being compatible
2837
var eventDispatcher = eventDispatchers.GetOrCreateValue(MakeEventDispatcher<TDerivedEvent>);
29-
eventSource.Dispatching += eventDispatcher;
38+
source.Dispatching += eventDispatcher;
3039

3140
return true;
3241
}
3342

34-
public bool RemoveSource<TDerivedEvent>(Mod mod, ICancelableEventSource<TDerivedEvent> eventSource)
43+
/// <inheritdoc cref="EventDispatcher{TEvent}.RemoveSource"/>
44+
public bool RemoveSource<TDerivedEvent>(Mod mod, ICancelableEventSource<TDerivedEvent> source)
3545
where TDerivedEvent : TEvent
3646
{
37-
if (!RemoveSource(mod, typeof(TDerivedEvent), eventSource))
47+
if (!RemoveSource(mod, typeof(TDerivedEvent), source))
3848
return false;
3949

4050
var eventDispatcher = eventDispatchers.GetOrCreateValue(MakeEventDispatcher<TDerivedEvent>);
41-
eventSource.Dispatching -= eventDispatcher;
51+
source.Dispatching -= eventDispatcher;
4252

4353
return true;
4454
}
@@ -68,36 +78,56 @@ private CancelableEventDispatching<TDerivedEvent> MakeEventDispatcher<TDerivedEv
6878
where TDerivedEvent : TEvent => new(DispatchEvents);
6979
}
7080

81+
/// <summary>
82+
/// Concrete dispatcher for <see cref="SyncEvent"/>s.
83+
/// </summary>
84+
/// <typeparam name="TEvent">The type of the dispatched events.</typeparam>
7185
internal sealed class EventDispatcher<TEvent>
7286
: EventDispatcherBase<IEventSource<TEvent>, IEventHandler<TEvent>>
7387
where TEvent : SyncEvent
7488
{
89+
/// <summary>
90+
/// Creates a new dispatcher for the given <paramref name="manager"/>.
91+
/// </summary>
92+
/// <param name="manager">The manager that this dispatcher belongs to.</param>
7593
public EventDispatcher(EventManager manager)
7694
: base(manager, AccessTools.DeclaredMethod(typeof(EventDispatcher<TEvent>), nameof(RemoveSource)))
7795
{ }
7896

79-
public bool AddSource<TDerivedEvent>(Mod mod, IEventSource<TDerivedEvent> eventSource)
97+
/// <summary>
98+
/// Adds the <typeparamref name="TDerivedEvent"/> <paramref name="source"/>
99+
/// from the given <paramref name="mod"/> to this dispatcher.
100+
/// </summary>
101+
/// <typeparam name="TDerivedEvent">The concrete type of the events created by the source.</typeparam>
102+
/// <inheritdoc cref="EventDispatcherBase{TSource, THandler}.AddSource"/>
103+
public bool AddSource<TDerivedEvent>(Mod mod, IEventSource<TDerivedEvent> source)
80104
where TDerivedEvent : TEvent
81105
{
82-
if (!AddSource(mod, typeof(TDerivedEvent), eventSource))
106+
if (!AddSource(mod, typeof(TDerivedEvent), source))
83107
return false;
84108

85109
// Have to wrap the DispatchEvents method in the correct delegate type,
86110
// otherwise the event will throw when adding it, despite being compatible
87111
var eventDispatcher = eventDispatchers.GetOrCreateValue(MakeEventDispatcher<TDerivedEvent>);
88-
eventSource.Dispatching += eventDispatcher;
112+
source.Dispatching += eventDispatcher;
89113

90114
return true;
91115
}
92116

93-
public bool RemoveSource<TDerivedEvent>(Mod mod, IEventSource<TDerivedEvent> eventSource)
117+
/// <summary>
118+
/// Removes the <typeparamref name="TDerivedEvent"/> <paramref name="source"/>
119+
/// from the given <paramref name="mod"/> from this dispatcher.
120+
/// </summary>
121+
/// <typeparam name="TDerivedEvent">The concrete type of the events created by the source.</typeparam>
122+
/// <inheritdoc cref="EventDispatcherBase{TSource, THandler}.RemoveSource"/>
123+
public bool RemoveSource<TDerivedEvent>(Mod mod, IEventSource<TDerivedEvent> source)
94124
where TDerivedEvent : TEvent
95125
{
96-
if (!RemoveSource(mod, typeof(TDerivedEvent), eventSource))
126+
if (!RemoveSource(mod, typeof(TDerivedEvent), source))
97127
return false;
98128

99129
var eventDispatcher = eventDispatchers.GetOrCreateValue(MakeEventDispatcher<TDerivedEvent>);
100-
eventSource.Dispatching -= eventDispatcher;
130+
source.Dispatching -= eventDispatcher;
101131

102132
return true;
103133
}

0 commit comments

Comments
 (0)