Allow handling by-value events by ref (#4373)

This commit is contained in:
DrSmugleaf
2023-10-18 18:37:43 -07:00
committed by GitHub
parent 54529fdbe3
commit f87012e681
7 changed files with 45 additions and 147 deletions

View File

@@ -23,16 +23,6 @@ public sealed class ByRefEventAnalyzer : DiagnosticAnalyzer
"Make sure that methods subscribing to a ref event have the ref keyword for the event argument."
);
private static readonly DiagnosticDescriptor ByValueEventSubscribedByRefRule = new(
Diagnostics.IdValueEventRaisedByRef,
"Value event subscribed to by-ref",
"Tried to subscribe to a value event '{0}' by-ref.",
"Usage",
DiagnosticSeverity.Error,
true,
"Make sure that methods subscribing to value events do not have the ref keyword for the event argument."
);
private static readonly DiagnosticDescriptor ByRefEventRaisedByValueRule = new(
Diagnostics.IdByRefEventRaisedByValue,
"By-ref event raised by value",
@@ -55,7 +45,6 @@ public sealed class ByRefEventAnalyzer : DiagnosticAnalyzer
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
ByRefEventSubscribedByValueRule,
ByValueEventSubscribedByRefRule,
ByRefEventRaisedByValueRule,
ByValueEventRaisedByRefRule
);
@@ -64,71 +53,9 @@ public sealed class ByRefEventAnalyzer : DiagnosticAnalyzer
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.EnableConcurrentExecution();
context.RegisterOperationAction(CheckEventSubscription, OperationKind.Invocation);
context.RegisterOperationAction(CheckEventRaise, OperationKind.Invocation);
}
private void CheckEventSubscription(OperationAnalysisContext context)
{
if (context.Operation is not IInvocationOperation operation)
return;
var subscribeMethods = context.Compilation
.GetTypeByMetadataName("Robust.Shared.GameObjects.EntitySystem")?
.GetMembers()
.Where(m => m.Name.Contains("SubscribeLocalEvent"))
.Cast<IMethodSymbol>();
if (subscribeMethods == null)
return;
if (!subscribeMethods.Any(m => m.Equals(operation.TargetMethod.OriginalDefinition, Default)))
return;
var typeArguments = operation.TargetMethod.TypeArguments;
if (typeArguments.Length < 1 || typeArguments.Length > 2)
return;
if (operation.Arguments.First().Value is not IDelegateCreationOperation delegateCreation)
return;
if (delegateCreation.Target is not IMethodReferenceOperation methodReference)
return;
var eventParameter = methodReference.Method.Parameters.LastOrDefault();
if (eventParameter == null)
return;
ITypeSymbol eventArgument;
switch (typeArguments.Length)
{
case 1:
eventArgument = typeArguments[0];
break;
case 2:
eventArgument = typeArguments[1];
break;
default:
return;
}
var byRefAttribute = context.Compilation.GetTypeByMetadataName(ByRefAttribute);
if (byRefAttribute == null)
return;
var isByRefEventType = eventArgument
.GetAttributes()
.Any(attribute => attribute.AttributeClass?.Equals(byRefAttribute, Default) ?? false);
var parameterIsRef = eventParameter.RefKind == RefKind.Ref;
if (isByRefEventType != parameterIsRef)
{
var descriptor = isByRefEventType ? ByRefEventSubscribedByValueRule : ByValueEventSubscribedByRefRule;
var diagnostic = Diagnostic.Create(descriptor, operation.Syntax.GetLocation(), eventArgument);
context.ReportDiagnostic(diagnostic);
}
}
private void CheckEventRaise(OperationAnalysisContext context)
{
if (context.Operation is not IInvocationOperation operation)

View File

@@ -18,7 +18,6 @@ public static class Diagnostics
public const string IdInvalidNotNullableFlagType = "RA0011";
public const string IdNotNullableFlagValueType = "RA0012";
public const string IdByRefEventSubscribedByValue = "RA0013";
public const string IdValueEventSubscribedByRef = "RA0014";
public const string IdByRefEventRaisedByValue = "RA0015";
public const string IdValueEventRaisedByRef = "RA0016";
public const string IdDataDefinitionPartial = "RA0017";

View File

@@ -20,9 +20,13 @@ namespace Robust.Shared.GameObjects
/// <param name="source"></param>
/// <param name="subscriber">Subscriber that owns the handler.</param>
/// <param name="eventHandler">Delegate that handles the event.</param>
/// <seealso cref="SubscribeEvent{T}(EventSource, IEntityEventSubscriber, EntityEventRefHandler{T})"/>
// [Obsolete("Subscribe to the event by ref instead (EntityEventRefHandler)")]
void SubscribeEvent<T>(EventSource source, IEntityEventSubscriber subscriber,
EntityEventHandler<T> eventHandler) where T : notnull;
/// <seealso cref="SubscribeEvent{T}(EventSource, IEntityEventSubscriber, EntityEventRefHandler{T})"/>
// [Obsolete("Subscribe to the event by ref instead (EntityEventRefHandler)")]
void SubscribeEvent<T>(
EventSource source,
IEntityEventSubscriber subscriber,
@@ -133,7 +137,7 @@ namespace Robust.Shared.GameObjects
var type = args.GetType();
ref var unitRef = ref ExtractUnitRef(ref args, type);
ProcessSingleEvent(source, ref unitRef, type, false);
ProcessSingleEvent(source, ref unitRef, type);
}
}
@@ -260,7 +264,7 @@ namespace Robust.Shared.GameObjects
var eventType = toRaise.GetType();
ref var unitRef = ref ExtractUnitRef(ref toRaise, eventType);
ProcessSingleEvent(source, ref unitRef, eventType, false);
ProcessSingleEvent(source, ref unitRef, eventType);
}
public void RaiseEvent<T>(EventSource source, T toRaise) where T : notnull
@@ -268,7 +272,7 @@ namespace Robust.Shared.GameObjects
if (source == EventSource.None)
throw new ArgumentOutOfRangeException(nameof(source));
ProcessSingleEvent(source, ref Unsafe.As<T, Unit>(ref toRaise), typeof(T), false);
ProcessSingleEvent(source, ref Unsafe.As<T, Unit>(ref toRaise), typeof(T));
}
public void RaiseEvent<T>(EventSource source, ref T toRaise) where T : notnull
@@ -276,7 +280,7 @@ namespace Robust.Shared.GameObjects
if (source == EventSource.None)
throw new ArgumentOutOfRangeException(nameof(source));
ProcessSingleEvent(source, ref Unsafe.As<T, Unit>(ref toRaise), typeof(T), true);
ProcessSingleEvent(source, ref Unsafe.As<T, Unit>(ref toRaise), typeof(T));
}
/// <inheritdoc />
@@ -301,7 +305,7 @@ namespace Robust.Shared.GameObjects
inverse.Remove(eventType);
}
private void ProcessSingleEvent(EventSource source, ref Unit unitRef, Type eventType, bool byRef)
private void ProcessSingleEvent(EventSource source, ref Unit unitRef, Type eventType)
{
if (!_eventData.TryGetValue(eventType, out var subs))
return;
@@ -314,20 +318,16 @@ namespace Robust.Shared.GameObjects
// This means ordered broadcast events have no overhead over non-ordered ones.
}
ProcessSingleEventCore(source, ref unitRef, subs, byRef);
ProcessSingleEventCore(source, ref unitRef, subs);
}
private static void ProcessSingleEventCore(
EventSource source,
ref Unit unitRef,
EventData subs,
bool byRef)
EventData subs)
{
foreach (var handler in subs.BroadcastRegistrations)
{
if (handler.ReferenceEvent != byRef)
ThrowByRefMisMatch();
if ((handler.Mask & source) != 0)
handler.Handler(ref unitRef);
}

View File

@@ -39,10 +39,6 @@ internal sealed partial class EntityEventBus : IEventBus
public bool IgnoreUnregisteredComponents;
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ThrowByRefMisMatch() =>
throw new InvalidOperationException("Mismatching by-ref ness on event!");
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static ref Unit ExtractUnitRef(ref object obj, Type objType)
{

View File

@@ -137,8 +137,7 @@ namespace Robust.Shared.GameObjects
component.Owner,
component,
CompIdx.Index(component.GetType()),
ref unitRef,
false);
ref unitRef);
}
void IDirectedEventBus.RaiseComponentEvent<TEvent>(IComponent component, CompIdx type, TEvent args)
@@ -149,8 +148,7 @@ namespace Robust.Shared.GameObjects
component.Owner,
component,
type,
ref unitRef,
false);
ref unitRef);
}
/// <inheritdoc />
@@ -162,8 +160,7 @@ namespace Robust.Shared.GameObjects
component.Owner,
component,
CompIdx.Index(component.GetType()),
ref unitRef,
true);
ref unitRef);
}
public void OnlyCallOnRobustUnitTestISwearToGodPleaseSomebodyKillThisNightmare()
@@ -178,7 +175,7 @@ namespace Robust.Shared.GameObjects
var type = typeof(TEvent);
ref var unitRef = ref Unsafe.As<TEvent, Unit>(ref args);
RaiseLocalEventCore(uid, ref unitRef, type, broadcast, false);
RaiseLocalEventCore(uid, ref unitRef, type, broadcast);
}
/// <inheritdoc />
@@ -187,7 +184,7 @@ namespace Robust.Shared.GameObjects
var type = args.GetType();
ref var unitRef = ref Unsafe.As<object, Unit>(ref args);
RaiseLocalEventCore(uid, ref unitRef, type, broadcast, false);
RaiseLocalEventCore(uid, ref unitRef, type, broadcast);
}
public void RaiseLocalEvent<TEvent>(EntityUid uid, ref TEvent args, bool broadcast = false)
@@ -196,7 +193,7 @@ namespace Robust.Shared.GameObjects
var type = typeof(TEvent);
ref var unitRef = ref Unsafe.As<TEvent, Unit>(ref args);
RaiseLocalEventCore(uid, ref unitRef, type, broadcast, true);
RaiseLocalEventCore(uid, ref unitRef, type, broadcast);
}
public void RaiseLocalEvent(EntityUid uid, ref object args, bool broadcast = false)
@@ -204,25 +201,25 @@ namespace Robust.Shared.GameObjects
var type = args.GetType();
ref var unitRef = ref Unsafe.As<object, Unit>(ref args);
RaiseLocalEventCore(uid, ref unitRef, type, broadcast, true);
RaiseLocalEventCore(uid, ref unitRef, type, broadcast);
}
private void RaiseLocalEventCore(EntityUid uid, ref Unit unitRef, Type type, bool broadcast, bool byRef)
private void RaiseLocalEventCore(EntityUid uid, ref Unit unitRef, Type type, bool broadcast)
{
if (!_eventData.TryGetValue(type, out var subs))
return;
if (subs.IsOrdered)
{
RaiseLocalOrdered(uid, type, subs, ref unitRef, broadcast, byRef);
RaiseLocalOrdered(uid, type, subs, ref unitRef, broadcast);
return;
}
EntDispatch(uid, type, ref unitRef, byRef);
EntDispatch(uid, type, ref unitRef);
// we also broadcast it so the call site does not have to.
if (broadcast)
ProcessSingleEventCore(EventSource.Local, ref unitRef, subs, byRef);
ProcessSingleEventCore(EventSource.Local, ref unitRef, subs);
}
/// <inheritdoc />
@@ -238,8 +235,7 @@ namespace Robust.Shared.GameObjects
typeof(TComp),
typeof(TEvent),
EventHandler,
null,
false);
null);
}
public void SubscribeLocalEvent<TComp, TEvent>(
@@ -260,8 +256,7 @@ namespace Robust.Shared.GameObjects
typeof(TComp),
typeof(TEvent),
EventHandler,
orderData,
false);
orderData);
RegisterCommon(typeof(TEvent), orderData, out _);
}
@@ -277,8 +272,7 @@ namespace Robust.Shared.GameObjects
typeof(TComp),
typeof(TEvent),
EventHandler,
null,
true);
null);
}
public void SubscribeLocalEvent<TComp, TEvent>(ComponentEventRefHandler<TComp, TEvent> handler, Type orderType,
@@ -295,8 +289,7 @@ namespace Robust.Shared.GameObjects
typeof(TComp),
typeof(TEvent),
EventHandler,
orderData,
true);
orderData);
RegisterCommon(typeof(TEvent), orderData, out _);
}
@@ -345,12 +338,6 @@ namespace Robust.Shared.GameObjects
if (_subscriptionLock)
throw new InvalidOperationException("Subscription locked.");
var referenceEvent = eventType.HasCustomAttribute<ByRefEventAttribute>();
if (referenceEvent != registration.ReferenceEvent)
throw new InvalidOperationException(
$"Attempted to subscribe by-ref and by-value to the same directed event! comp={compTypeObj.Name}, event={eventType.Name} eventIsByRef={referenceEvent} subscriptionIsByRef={registration.ReferenceEvent}");
if (compType.Value >= _entSubscriptions.Length || _entSubscriptions[compType.Value] is not { } compSubs)
{
if (IgnoreUnregisteredComponents)
@@ -377,7 +364,7 @@ namespace Robust.Shared.GameObjects
Type compTypeObj,
Type eventType,
DirectedEventHandler<TEvent> handler,
OrderingData? order, bool byReference)
OrderingData? order)
where TEvent : notnull
{
EntAddSubscription(compType, compTypeObj, eventType, new DirectedRegistration(handler, order,
@@ -385,7 +372,7 @@ namespace Robust.Shared.GameObjects
{
ref var tev = ref Unsafe.As<Unit, TEvent>(ref ev);
handler(uid, comp, ref tev);
}, byReference));
}));
}
private void EntUnsubscribe(CompIdx compType, Type eventType)
@@ -525,7 +512,7 @@ namespace Robust.Shared.GameObjects
}
}
private void EntDispatch(EntityUid euid, Type eventType, ref Unit args, bool dispatchByReference)
private void EntDispatch(EntityUid euid, Type eventType, ref Unit args)
{
if (!EntTryGetSubscriptions(eventType, euid, out var enumerator))
return;
@@ -535,9 +522,6 @@ namespace Robust.Shared.GameObjects
if (component.Deleted)
continue;
if (reg.ReferenceEvent != dispatchByReference)
ThrowByRefMisMatch();
reg.Handler(euid, component, ref args);
}
}
@@ -545,22 +529,18 @@ namespace Robust.Shared.GameObjects
private void EntCollectOrdered(
EntityUid euid,
Type eventType,
ref ValueList<OrderedEventDispatch> found,
bool byRef)
ref ValueList<OrderedEventDispatch> found)
{
if (!EntTryGetSubscriptions(eventType, euid, out var enumerator))
return;
while (enumerator.MoveNext(out var component, out var reg))
{
if (reg.ReferenceEvent != byRef)
ThrowByRefMisMatch();
found.Add(new OrderedEventDispatch((ref Unit ev) =>
{
if (!component.Deleted)
reg.Handler(euid, component, ref ev);
}, reg.Order));
}, reg.Order));
}
}
@@ -568,8 +548,7 @@ namespace Robust.Shared.GameObjects
EntityUid euid,
IComponent component,
CompIdx baseType,
ref Unit args,
bool dispatchByReference)
ref Unit args)
where TEvent : notnull
{
var compSubs = _entSubscriptions[baseType.Value]!;
@@ -577,9 +556,6 @@ namespace Robust.Shared.GameObjects
if (!compSubs.TryGetValue(typeof(TEvent), out var reg))
return;
if (reg.ReferenceEvent != dispatchByReference)
ThrowByRefMisMatch();
reg.Handler(euid, component, ref args);
}
@@ -690,17 +666,14 @@ namespace Robust.Shared.GameObjects
{
public readonly Delegate Original;
public readonly DirectedEventHandler Handler;
public readonly bool ReferenceEvent;
public DirectedRegistration(
Delegate original,
OrderingData? ordering,
DirectedEventHandler handler,
bool referenceEvent) : base(ordering)
DirectedEventHandler handler) : base(ordering)
{
Original = original;
Handler = handler;
ReferenceEvent = referenceEvent;
}
public void SetOrder(int order)
@@ -736,6 +709,8 @@ namespace Robust.Shared.GameObjects
}
}
/// <seealso cref="ComponentEventRefHandler{TComp, TEvent}"/>
// [Obsolete("Use ComponentEventRefHandler instead")]
public delegate void ComponentEventHandler<in TComp, in TEvent>(EntityUid uid, TComp component, TEvent args)
where TComp : IComponent
where TEvent : notnull;

View File

@@ -11,14 +11,10 @@ namespace Robust.Shared.GameObjects
private static void CollectBroadcastOrdered(
EventSource source,
EventData sub,
ref ValueList<OrderedEventDispatch> found,
bool byRef)
ref ValueList<OrderedEventDispatch> found)
{
foreach (var handler in sub.BroadcastRegistrations)
{
if (handler.ReferenceEvent != byRef)
ThrowByRefMisMatch();
if ((handler.Mask & source) != 0)
found.Add(new OrderedEventDispatch(handler.Handler, handler.Order));
}
@@ -29,8 +25,7 @@ namespace Robust.Shared.GameObjects
Type eventType,
EventData subs,
ref Unit unitRef,
bool broadcast,
bool byRef)
bool broadcast)
{
if (!subs.OrderingUpToDate)
UpdateOrderSeq(eventType, subs);
@@ -38,9 +33,9 @@ namespace Robust.Shared.GameObjects
var found = new ValueList<OrderedEventDispatch>();
if (broadcast)
CollectBroadcastOrdered(EventSource.Local, subs, ref found, byRef);
CollectBroadcastOrdered(EventSource.Local, subs, ref found);
EntCollectOrdered(uid, eventType, ref found, byRef);
EntCollectOrdered(uid, eventType, ref found);
DispatchOrderedEvents(ref unitRef, ref found);
}

View File

@@ -24,6 +24,8 @@ namespace Robust.Shared.GameObjects
SubEvent(EventSource.Network, handler, before, after);
}
/// <seealso cref="SubscribeLocalEvent{T}(EntityEventRefHandler{T}, Type[], Type[])"/>
// [Obsolete("Subscribe to the event by ref instead (EntityEventRefHandler)")]
protected void SubscribeLocalEvent<T>(
EntityEventHandler<T> handler,
Type[]? before = null, Type[]? after = null)
@@ -72,6 +74,8 @@ namespace Robust.Shared.GameObjects
SubSessionEvent(EventSource.All, handler, before, after);
}
/// <seealso cref="SubEvent{T}(EventSource, EntityEventRefHandler{T}, Type[], Type[])"/>
// [Obsolete("Subscribe to the event by ref instead (EntityEventRefHandler)")]
private void SubEvent<T>(
EventSource src,
EntityEventHandler<T> handler,
@@ -108,6 +112,8 @@ namespace Robust.Shared.GameObjects
_subscriptions.Add(new SubBroadcast<EntitySessionMessage<T>>(src));
}
/// <seealso cref="SubscribeLocalEvent{TComp, TEvent}(ComponentEventRefHandler{TComp, TEvent}, Type[], Type[])"/>
// [Obsolete("Subscribe to the event by ref instead (ComponentEventRefHandler)")]
protected void SubscribeLocalEvent<TComp, TEvent>(
ComponentEventHandler<TComp, TEvent> handler,
Type[]? before = null, Type[]? after = null)