IEntitySystem Event Subscription Code Generation

[Centronias]

Adds an IIncrementalGenerator which takes annotated event handler methods and assembles them into subscription calls for you in EntitySystems so you don't need to put them in override void Initialize() .

Why?

Most of my experience recently has been in languages with robust type inference, so when I encountered the need to declare and register event handlers with a bunch of type arguments, I got unhappy. For example, consider the client AudioSystem which contains the event handler

private void OnAudioStartup(EntityUid uid, AudioComponent component, ComponentStartup args) { ... }

. To use this handler, we must register it with

SubscribeLocalEvent<AudioComponent, ComponentStartup>(OnAudioStartup);

. Conceptually, we are just attaching the handle to the method, OnAudioStartup, to a "Local subscription" concept -- this is the minimum information needed. However, thanks to exactly how subscriptions are registered, and exactly how C# does type parameters, it is necessary to re-specify <AudioComponent, ComponentStartup> on the registration, which doesn't provide much value for coders actually reading and maintaining the code.

Additionally, applications built on RobustToolbox are encouraged to declare EntitySystems as partial, which can cause subscriptions to need to be split between files, leading to a pattern of the main file having the override void Initialize() { ... } implementation which delegates to some number of void InitializeSubsystem() { ... } methods.

This generator solves both of these issues by allowing coders to specify "this is an even subscription handler" on the handler method itself, and the generator does the rest, assembling all subscriptions in the class, regardless of partial class parts in disparate files, in one override AutoSubscriptions() { ... } method.

How exactly do I use this?

Check out the example below for something more concrete, but the concept is to simply annotate your event handler where it's declared:

[LocalEventSubscription]
private void OnAudioStartup(EntityUid uid, AudioComponent component, ComponentStartup args) {
    // Implementation omitted
}

The generator looks at the subscription type (LocalEventSubscription, NetworkEventSubscription, EventSubscription ("all")) and the parameters to the handler method (AudioComponent and ComponentStartup here), to decide what to do. It constructs a method binding for each annotation (

SubscribeLocalEvent<Robust.Shared.Audio.Components.AudioComponent, Robust.Shared.GameObjects.ComponentStartup>(OnAudioStartup);

here), and then assembles all of those bindings into one override void AutoSubscriptions() implementation which is put into a generated partial class file.

No more jumping between subscription registrations and handlers, do your coding all in one location in the syntax. No more juggling initialization when dealing with partial classes.

What this DOESN'T do

This doesn't support

• before and after parameters to event subscription methods. I wanted to, but encoding that information into the annotation proved too annoying.
  • Maybe adding more annotations which don't do anything on their own, but are specifically for encoding this information could work. But I'll wait until the Wizards give their opinions on the base capability
• More exotic handlers, like those with type parameters
• More exotic subscriptions, like using a handler with some amount of variance, and then setting the exact types on the subscription
• Probably some other things I'm forgetting

Technical Details

This is implemented with:

• One new analyzer project
• Four new annotations
  • LocalEventSubscriptionAttribute
  • NetworkEventSubscriptionAttribute
  • EventSubscriptionAttribute
• One new IIncrementalGenerator, EntitySystemSubscriptionGenerator
• One new DiagnosticAnalyzer, EntitySystemSubscriptionGeneratorErrorAnalyzer

Modifications to Existing Code

• IEntitySystem gets a new method, void AutoSubscriptions(), to hold onto the generated code. It's called just after Initialize is. Ideally this would be declared on EntitySystem since the actual subscription methods are only available to that type, but that'd require much more change to Initialize and how it's called.
• I've added an IEqualityComparer<PartialTypeInfo> which doesn't consider the PartialTypeInfo's syntax Location. This was needed because of exactly how I'm finding IEntitySystems which contain annotated methods, which requires the generator to dedupe types. However, because PartialTypeInfos constructed from different partial parts of the same class are not equal (with the record's implicit implementation), the deduping wouldn't work without explicitly excluding the Location.
  • This is all new and shouldn't break and usages.
• Demonstration uptake in AudioSystem
  • I don't think this should break anything as the public interface of the class hasn't changed.

Example

Check out e96121b , where I've modified the existing client AudioSystem to use subscription generation. It generates the following syntax:

// <auto-generated />

using Robust.Shared.GameObjects;

namespace Robust.Client.Audio;

public partial class AudioSystem
{
    /// <inheritdoc />
    [MustCallBase]
    public override void AutoSubscriptions()
    {
        base.AutoSubscriptions();

        SubscribeLocalEvent<Robust.Shared.Audio.Components.AudioComponent, Robust.Shared.Analyzers.AfterAutoHandleStateEvent>(OnAudioState);
        SubscribeLocalEvent<Robust.Shared.Audio.Components.AudioComponent, Robust.Shared.GameObjects.EntityPausedEvent>(OnAudioPaused);
        SubscribeLocalEvent<Robust.Shared.Audio.Components.AudioComponent, Robust.Shared.GameObjects.ComponentStartup>(OnAudioStartup);
        SubscribeLocalEvent<Robust.Shared.Audio.Components.AudioComponent, Robust.Shared.GameObjects.ComponentShutdown>(OnAudioShutdown);
        SubscribeNetworkEvent<Robust.Shared.Audio.Systems.SharedAudioSystem.PlayAudioPositionalMessage>(OnEntityCoordinates);
        SubscribeNetworkEvent<Robust.Shared.Audio.Systems.SharedAudioSystem.PlayAudioEntityMessage>(OnEntityAudio);
        SubscribeNetworkEvent<Robust.Shared.Audio.Systems.SharedAudioSystem.PlayAudioGlobalMessage>(OnGlobalAudio);

    }
}

[mirrorcult]

very sick!!

[mirrorcult]

was hoping someone would tackle some variant of this eventually

[Centronias]

The changes to the solution file were done with Rider. The rest of the project files I manually edited based on existing generators (as I was initially struggling immensely to get the generator to even run). So lemme know if I borked anything there.

[Centronias]

Oh I also added this because I use it to look up Symbols from the partial type info.

[Centronias]

This is admittedly kinda hairy, but the goal was to make it so that we find systems with annotated methods in them without a need for annotating the system itself.

So this discovers methods, walks up to their systems, dedupes those systems, and then processes the systems.

Intermediate values are equatable PartialTypeInfos, so it should be decent for performance. We do look up compilation-dependent information using info from the PartialTypeInfos, but, again, that's to make the usage more streamlined.

[Centronias]

This method is probably too long, but trying to break out parts of it into functions means passing around IncrementalValueProvider<T>s and tons of contexts which all make the function signatures less than legible.

[Centronias]

Basically anywhere along the way that something goes bad, we return null and just ignore the value, expecting the other analyzer to complain about it for us.

[Centronias]

These are the intermediate values between analyzer steps. I think they should be good for incremental generation, but I am very open to being corrected.

[mirrorcult]

main issue i can see is this implementation seems to make it so that any system which uses the annotations for event handler generation must then move their other initialization logic into a separate function, Init or something, annotated with [CallAfterSubscriptions]

this seems unnecessarily clunky and unintuitive imo, my preferred solution at least would be that Initialize is not touched in the generated code at all, and a new virtual method is introduced to EntitySystem that the generator can override, which is just called in the same places as Initialize.

this does present the issue of people potentially overriding this function unnecessarily or misunderstanding its purpose, and i -thought- rider had easy support for excluding methods from intellisense using [EditorBrowsable(EditorBrowsableState.Never)], but this functionality is apparently off by default (editor>general>code completion>filter members by editorbrowsable attribute) and cant be enabled by the project itself ... so maybe there should be another solution

[Centronias]

Yeah, I agree that needing to retrofit existing Initialize implementations is not good. At the same time, I wasn't sure what sort of changes I'm "allowed" to make to IEntitySystem, so if adding something to that is permissible, I definitely think that's a better solution.

[PJB3005]

Didn't review the main body of code yet.