search

sschmid / Entitas

Entitas is a super fast Entity Component System (ECS) Framework specifically made for C# and Unity
MIT License
7.11k stars 1.11k forks source link

Entitas with Roslyn Code Generation via dotnet IIncrementalGenerator #1005

Closed studentutu closed 1 year ago

studentutu commented 2 years ago

[ EDIT by @sschmid ]

I edited the issue description because this issue is now the main issue for the new code generator

dotnet's source generators, specially IIncrementalGenerator may be a valid alternative to the current approach with Jenny

Learn about dotnet Incremental Generators:

While migrating to dotnet source generators, I will use the opportunity to update and improve the generated code:

I already made good progress but it's still in research state. Any help from the community is greatly appreciated! Please feel free to engage and help in the conversation if you can! 🙏

Tasks

Generators

Attributes

original message from issue author:

Hi,

I have a suggestion and would want to contribute. As in official code generation guide by Unity - https://docs.unity3d.com/Manual/roslyn-analyzers.html - we can use separate project and create dll which unity will use right in it's code compilation steps.

We already have a separate project for it - Jehny, all we need is to make sure that code gen is done by Microsoft Roslyn Source Generators, and put a label “RoslynAnalyzer” for the DLL inside the release branch (create a Unity package)

This way we don't need to use Jehny Server for constant monitoring of changes, and it will help clean up the workflow.

Here's some code that we need to use for Roslyn Source Generators

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Text;
using System.Text

[Generator]
public class ExampleSourceGenerator : ISourceGenerator
{
}

Hope it will be helpful.

sschmid commented 2 years ago

@studentutu thanks! Roslyn Source Generators is definitely sth I would like to research at some point! That might be useful for Entitas. Jenny however follows a different idea where the input can be anything, not just source code or assemblies. It more general purpose for any kind of code generation

sschmid commented 2 years ago

One downside is

Note: Roslyn analyzers are only compatible with the IDEs that Unity publically supports, which are Visual Studio and JetBrains Rider.

But I'll have a look

sschmid commented 2 years ago

Looks like we would need to switch to Unity when we make changes in order to recompile, which would take too long.

sschmid commented 2 years ago

Ok, building in the IDE works too 👍

rubenwe commented 1 year ago

I've written a few Source Generators. My note would be to look into Incremental Source Generators if you decide to go this route. The perf of a normal ISourceGenerator isn't that great.

Unity 2022.2 supports the required newer Microsoft packages for that. They just haven't fixed their documentation yet.

studentutu commented 1 year ago

yep, definitely better with IIncrementalGenerator

rubenwe commented 1 year ago

One thing of note though: I'm having severe problems with AdditionalFiles in Unity 2021.3 - that's files you want to pass along to source generators outside of source code. So for example if you had some csv files you would want to use as a source to generate from.

The Unity docs on this are kind of sparse - and the one page that does mention this describes an implementation that: a) does not work - at all - as described b) is of questionable design, given that one needs to rename files to somename.[Gernerator].additionalfile

So if that is something that one wants to support I'd wait for Unity to actually switch to permanent, modern csproj files and using MsBuild, as outlined in their roadmap talk. Although it is a bit bold to assume that this will just work then. They usually manage to do stuff that is not in line with what .NET devs are used to ;).

sschmid commented 1 year ago

Hi, I started testing IIncrementalGenerator and started a new branch. I added Entitas.Generators and Entitas.Generators.Tests projects to get started.

https://github.com/sschmid/Entitas/tree/roslyn-source-generators/src/Entitas.Generators

https://github.com/sschmid/Entitas/tree/roslyn-source-generators/tests/Entitas.Generators.Tests

So far I'am happy with IIncrementalGenerator performance and will look more into it.

sschmid commented 1 year ago

If all goes well I can imagine that it can replace Jenny and setting up Entitas will be much easier.

I started with snapshot testing to verify the output of the source generators, it's pretty cool. A test looks like this 

[Fact]
public Task Test() => TestHelper.Verify(
    GetFixture("NamespacedComponentWithOneField"),
    new ComponentGenerator());

More about snapshot testing: https://andrewlock.net/creating-a-source-generator-part-2-testing-an-incremental-generator-with-snapshot-testing/

sschmid commented 1 year ago

I made some progress using IIncrementalGenerator. It's great. But now I was testing it with the latest Unity 2022.3 LTS and it looks like IIncrementalGenerator is not yet supported. Can this be true? I hope I'm wrong!

sschmid commented 1 year ago

Ok, got it to work!

Unity docs say you should use this specific version: Microsoft.CodeAnalysis 3.8 https://docs.unity3d.com/Manual/roslyn-analyzers.html

This version does not contain IIncrementalGenerator.

The current version is 4.6.0, but that one does not work in Unity. I manually checked each version to find the latest one that works, which is

<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.1.0" />

And you can use netstandard2.1 <TargetFramework>netstandard2.1</TargetFramework>

sschmid commented 1 year ago

Currently stuck, because it seems like you cannot resolve generated attributes, e.g. similar to the current approach I wanted to generate a convenience context attribute for each context, so you can add this attribute to components as usual

[MyApp.Main.Context, Other.Context]
partial MovableComponent : IComponent { }

But since those attributes are generated, the don't seem to be part of the compilation for looking up symbols 😭

This was easily possible with Jenny...

Any ideas how to solve this?

sschmid commented 1 year ago

To be more specific: Testing with non-generated attributes, I can easily get the attribues like this

var attribute = symbol.GetAttributes().First();

With the generated ones the same code returns ErrorTypeSymbol instead of the attributes

studentutu commented 1 year ago

@sschmid you can use following 1) Create a list of ICustomGenerators with Execute(GeneratorExecutionContext cx, Compilation compilation, other params if needed)

2) For each generator create a separate class: 

/// <summary>
///   Generates systems for step ..... for all components.
/// </summary>
[Generator(LanguageNames.CSharp)]
public class SystemsDescriptorGenerator : IIncrementalGenerator, ICustomGenerators 
{

void ICustomGenerators.Execute(GeneratorExecutionContext cx, Compilation compilation, other params)
  {
    // Do work.
        cx.AddSource($"{cx}.{Attribute}CustomStep{component.Name}.generated", GeneratedCode(cx, component));
  }
}

3) Don't forget to add to the YourComponents.Generators.csproj

    <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.0.1" PrivateAssets="all" />
    <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.3" PrivateAssets="all" />
  </ItemGroup>

And for the actual game /Runtime .csproj:

    <ItemGroup>
    <ProjectReference Include="..\YourComponents.Generators.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
  </ItemGroup>
Ants-Aare commented 1 year ago

afaik step 3. and 4. are already part of the current setup.

Can you provide documentation for ICustomGenerators interface? Is it part of roslyn or a concept you introduced yourself? what's calling ICustomGenerators.Execute?

I don't see how this makes generated code of other incremental generators available to the input compilation of this generator. Can you explain how your approach solves/bypasses this?

studentutu commented 1 year ago

@Ants Aare ICustomGenerators is just a custom interface, for use with a single generator (ISourceGenerator)

Basically, you begin with a normal Generator (ISourceGenerator), that includes a list of other generators (list of ICustomGenerators).

Then in the main Generator, you simply execute each custom one by providing GeneratorExecutionContext and Compilation into them.

So by using a predefined ordering, you will get a correct compilation of source generators.

studentutu commented 1 year ago

By the way, if you will use IIncrementalGenerator- there is no need for custom ordering, as all IIncrementalGenerator's are executed before final compilation.

Ants-Aare commented 1 year ago

Maybe I'm understanding it wrong, but I don't think this solves the problem(?) What you're describing is just a way to call multiple methods one after each other through an interface inside a regular sourcegenerator. First of all this doesn't include recompilation steps inbetween the calls to ICustomGenerators, which would be neccessary to get the symbols as INamedTypeSymbols using GetAttributes(). Secondly this would downgrade the current solution to a normal ISourceGenerator instead of an IIncrementalGenerator. Btw: The order in which we call cx.AddSource is irrelevant as all files will be added to the compilation in the same pass once the generator finishes executing. No matter if we're using SourceProductionContext or GeneratorExecutionContext

studentutu commented 1 year ago

right, missed the part of the "generated source code with new attributes".

rubenwe commented 1 year ago

But since those attributes are generated, they don't seem to be part of the compilation for looking up symbols 😭

Yes, this is part of the design of source generators. This avoids having to have multiple runs and allows the caching mechanisms that make them so efficient.

For my ECS approach I'm not using generated attributes because of this.

The user defines partial classes for the contexts and components implement IComponent<TContext1, ...>.

sschmid commented 1 year ago

Hi, a quick update and some thoughts:

I have a working proof of concept using IIncrementalGenerator. Main goal of this new generator is easy of use (no Jenny setup needed anymore) and support for namespaces in components and contexts + support for Unity asmdefs. As previously explained in other GitHub issues, it's necessary to change the generated code to not used partial on the Entity classes (and others), but instead use static C# extension methods. This way it's possible to get a similar and familiar API as we have now, but at the same time support asmdefs. We also cannot pre-generate all component indexes anymore, as new components may be introduced from other dlls. So there are still a few challenges ahead, but here's a quick sample of how it could like based on my current generators:

Contexts

Now with namespace support! Works with or without a namespace.

You can define contexts in your code like this:

// MainContext.cs
namespace MyApp
{
    partial class MainContext : Entitas.IContext { }
}

// OtherContext.cs
partial class OtherContext : Entitas.IContext { }

Components

Now with namespace support! Works with or without a namespace.

You can define components in your code like this:

// MovableComponent.cs
namespace MyFeature
{
    [MyApp.Main.Context, Other.Context]
    partial class MovableComponent : Entitas.IComponent { }
}

// PositionComponent.cs
namespace MyFeature
{
    [MyApp.Main.Context, Other.Context]
    partial class PositionComponent : Entitas.IComponent
    {
        public int X;
        public int Y;
    }
}

The generated component extensions work for all specified contexts and can be chained:

MainContext mainContext = new MyApp.MainContext();
MyApp.Main.Entity mainEntity = mainContext.CreateEntity()
    .AddMovable()
    .ReplaceMovable()
    .RemoveMovable()
    .AddPosition(1, 2)
    .ReplacePosition(3, 4)
    .RemovePosition();

OtherContext otherContext = new OtherContext();
Other.Entity otherEntity = otherContext.CreateEntity()
    .AddMovable()
    .ReplaceMovable()
    .RemoveMovable()
    .AddPosition(1, 2)
    .ReplacePosition(3, 4)
    .RemovePosition();

Matchers

I currently generate component indexes for each context, e.g. MyFeaturePositionComponentIndex. They also include the namespace. I might use them later to assign component indexes ones the app starts.

Matcher.AllOf(stackalloc[]
{
    MyFeaturePositionComponentIndex.Value,
    MyFeatureMovableComponentIndex.Value
});
sschmid commented 1 year ago

More updates: Yay, added component deconstructors

var (x, y) = entity.GetPosition();
x.Should().Be(1);
y.Should().Be(2);

Also, since the only purpose of ContextAttributes is for code generators, I added a [Conditional] attribute, so all those attributes are stripped from components once compiled.

sschmid commented 1 year ago

Fyi, for those who are interested about the changes, see branch: roslyn-source-generators https://github.com/sschmid/Entitas/tree/roslyn-source-generators

sschmid commented 1 year ago

More updates: I'm currently testing alternatives to the currently generated ComponentLookup. The current approach doesn't allow Unity's asmdefs or multiple projects.

The new approach should work with multiple assemblies per solution. At some point however, you would need to assign an index to each component. With the following idea you can build up your game with multiple separate assemblies and the main project that consumes them can implement a partial method per context and add the ContextInitializationAttribute to it:

public static partial class ContextInitialization
{
    [MyApp.Main.ContextInitialization]
    public static partial void Initialize();
}

This will be picked up by the generator and it will generate everything that used to be in ComponentLookup, e.g.:

namespace Entitas.Generators.IntegrationTests
{
public static partial class ContextInitialization
{
    public static partial void Initialize()
    {
        MyFeatureMovableComponentIndex.Index = new ComponentIndex(0);
        MyFeaturePositionComponentIndex.Index = new ComponentIndex(1);

        MyApp.MainContext.ComponentNames = new string[]
        {
            "MyFeature.Movable",
            "MyFeature.Position"
        };

        MyApp.MainContext.ComponentTypes = new System.Type[]
        {
            typeof(MyFeature.MovableComponent),
            typeof(MyFeature.PositionComponent)
        };
    }
}
}
sschmid commented 1 year ago

More updates: I improved the overall caching performance by using multiple pipelines. This means, only affected files should be regenerated leaving most of the files untouched.

I can recommend this cookbook for incremental generators: https://github.com/dotnet/roslyn/blob/main/docs/features/incremental-generators.md

Next problem: if something in the generator fails, nothing will be generated. This can easily be reproduced by declaring the same component twice which will break everything. I tried to wrap all spc.AddSource() calls in a try catch block, because they fail when filenames are not unique, but this didn't help either.

Does anyone know how to make handle exceptions in a source generator?

studentutu commented 1 year ago

@sschmid you can use tests, and check snapshots? https://andrewlock.net/creating-a-source-generator-part-2-testing-an-incremental-generator-with-snapshot-testing/

Otherwise - you can even use simpler diagnostic as a log inside the compilation See https://github.com/dotnet/roslyn-sdk/blob/main/src/Microsoft.CodeAnalysis.Testing/README.md 

await new CSharpAnalyzerTest<SomeAnalyzerType, XUnitVerifier>
{
    TestState =
    {
        Sources = { @"original source code" },
        ExpectedDiagnostics = null,
        AdditionalFiles =
        {
            ("File1.ext", "content1"),
            ("File2.ext", "content2"),
        },
    },
}.RunAsync();

// Inside try-catch, catch report diagnostic
 context.ReportDiagnostic(Diagnostic.Create(ExceptionRule,
            constructorParameter.Locations[0],
            error));
rubenwe commented 1 year ago

As @studentutu hinted it's probably a good idea to emit diagnostics for these problems if you run into generation failures.

On top of that, you can also be proactive and write additional implementations of DiagnosticAnalyzer for common problem scenarios. Those can also offer code fixes if the class of error has an easy way to fix it. That's the lightbulb fixes that are popping up in Visual Studio / Rider.

In my ECS prototype I used those to validate Queries (so impossible queries produce errors).

sschmid commented 1 year ago

More updates:

Screenshot 2023-06-29 at 15 36 55

I have a working Entitas protoype that supports namespaced contexts and components with multiple Unity asmdefs

sschmid commented 1 year ago

I'm very happy with that now and I will proceed to implement all missing generators to get the full feature set of the current Jenny generators.

Ants-Aare commented 1 year ago

have you thought about using enums perhaps? you can just collect all component names together into one file and make an enum out of it like this

public enum AppContextComponents
{
    Test1Component,
    Test2Component,
    Test3Component,
}

I feel like doing both the MyFeatureMovableComponentIndex.Index = new ComponentIndex(0); and MyApp.MainContext.ComponentNames = new string[]

feels like it's basically the same thing as an enum. (ints in the lower level code that also have a string version if you need it). Plus you can just declare the enum using the component names without worrying about the actual numbers etc. the only thing that matters is that they're unique, right? enums should guarantee that. Maybe I'm understanding something wrong though.

Ants-Aare commented 1 year ago

Is there a use case for interfaces that derive from Entitas.IComponent? I.e.:

ICustomInterface : Entitas.IComponent{}

PositionComponent : ICustomInterface{}

If not I would recommend to make Entitas.IComponent a sealed interface. This way we can filter out more syntax trees in the predicate, thereby improving performance. The incremental generators re-run the transform every time, even if the predicate didn’t result in any new or changed syntax trees.(documentation). Which means that on every recompile the transform will create & cache empty immutable arrays for every single class declaration in the solution. If we filter out only relevant syntaxes using this:

&& candidate.BaseList.Types.Any(x => x.Type switch
{
    IdentifierNameSyntax identifierNameSyntax => identifierNameSyntax.Identifier is { Text: "IComponent" },
    QualifiedNameSyntax qualifiedNameSyntax =>
        qualifiedNameSyntax.Left is IdentifierNameSyntax { Identifier.Text: "Entitas" } &&
        qualifiedNameSyntax.Right is IdentifierNameSyntax { Identifier.Text: "IComponent" },
    _ => false
})

we would not create as many duplicate immutablearrays (this code snippet would be added onto the predicate for the syntaxvalueprovider)

Ants-Aare commented 1 year ago

I also noticed an inefficiency in the newly made improvements you posted. I don't think it makes sense for the first transform to return an array of arrays. If we do this we will have multiple duplicate copies of ComponentDeclaration with the same data in all fields except one (the context). If you have 20 contexts attributes on one component you will get 20 strings for Namespace, 20 strings for FullName, etc etc etc. soooo much duplicate data. Why not make one struct and have it hold an array of contexts. this way you only have 1 string for each + 20 entries in the array inside of the ComponentDeclaration. You can do the rest with custom equality comparers etc. and the pipeline shouldn't be affected too much

Ants-Aare commented 1 year ago

@sschmid did you check out the PR I did? I added Diagnostics to some methods using something similar to a Result pattern. capturing diagnostics properly can sometimes be a bit weird and you kind of need to split off the pipeline to it's own source output. I checked out the way the MVVM source generator is doing it after I saw this comment, their setup is nice and they're doing lots of things right imo. here's a link to the comment

sschmid commented 1 year ago

Hi @Ants-Aare,

about the enums: I plan to remove ComponentNames like MyApp.MainContext.ComponentNames and update the ContextInfo to only contain the component types.

Also I'm affraid this won't work when you have multiple assemblies: Generated code like AddPosition() depends on an index, which will potentially be assigned in another assembly. Therefore you cannot use an enum which contains all component indexes. Instead I generate a readonly struct ComponentIndex for each component, which can be assigned later in the main assembly.

Thanks for the perf improvments tips, that's always great to talk about! Your suggestion with the sealed component interface makes sense! I will try it soon! Thanks

I switched to one ComponentDeclaration per context with the hope to improve caching, as only the added or removed ones are affected. One component might result in mutliple generated files, like extension methods for entity, context, matcher etc, so I tried to optimize for that and reduce the need to regen unaffected ones. We can discuss this if you think I'm doing it wrong, because I might actually :D

Yes, thanks again for the PR. For now I was foccussing on getting a working protoype that works in Unity, Rider and VSCode + multiple asmdefs. During this research the ComponentGenerator changed heavily basically every day, and I wasn't yet sure if I can make it work, so no need for diags :D But now that that works, I can now also work diags and will have a look at your PR again

Ants-Aare commented 1 year ago

ah, okay! I understand where you're going with this 🤔 I agree that enums would be a bit too simplistic.

I'll try and update the PR so there's not that many conflicts with the current state of the branch. Can you add me to the Entitas Repo perchance? would be a bit easier to work on stuff

sschmid commented 1 year ago

@Ants-Aare fyi, your suggestion with IComponent is is:

https://github.com/sschmid/Entitas/commit/951893400108352a1499d2e41c21187b72a05d2c

btw, turns out there is no such thing as a sealed interface :D

sschmid commented 1 year ago

I have an issue, does someone know how to handle this?

Description:

I added a new project Entitas.Generators.Attributes which contains 1 attribute:

I wanted to try to use that one on one of the test components:

using Entitas;
using Entitas.Generators.Attributes;
using MyApp;

namespace MyFeature
{
    [MyApp.Main.Context]
    [Context(typeof(MainContext))]
    public sealed class SomeNamespacedComponent : IComponent { }
}

Problem:

The ComponentGenerator does not correctly identify this attribute. Inseatd of Entitas.Generators.Attributes.ContextAttribute it resolves it to UnderlyingSymbol: {ErrorType Entitas.Context}.

I tried to add the Entitas.Generators.Attributes project to the tests run by adding it to the references:

IEnumerable<PortableExecutableReference> references = new[]
{
    MetadataReference.CreateFromFile(typeof(object).Assembly.Location),
    MetadataReference.CreateFromFile(typeof(IComponent).Assembly.Location),
    MetadataReference.CreateFromFile(typeof(Attributes.ContextAttribute).Assembly.Location), // <------- added
    MetadataReference.CreateFromFile(typeof(MyApp.Library.ContextAttribute).Assembly.Location)
};

Reproduce

When changing the usage of the attribute in SomeNamespacedComponent from [Context(typeof(MainContext))] to [ContextAttribute(typeof(MainContext))], it's still an ErrorType, but at least the fullName is now correct: Entitas.Generators.Attributes.ContextAttribute

No idea how to solves this atm. I think the dependency to Entitas.Generators.Attributes should be correct by using MetadataReference.CreateFromFile() (maybe this is wrong?). At least that's how I do it for the other dependencies

Ants-Aare commented 1 year ago

To me it sounds like there's both a issue with the compilation and an issue with correctly resolving the typesymbols.

It seems to be assuming that if you write [Context] you want to resolve it into the Entitas namespace instead of in the Entitas.Generators.Attributes namespace. If you write [ContextAttribute] then it resolves that correctly into the Entitas.Generators.Attributes namespace. Maybe there's a class inside Entitas that is also called Context or something like that, or it's just using the first namespace it finds. Maybe I'm understanding it wrong though.

I'm not 100% sure what the cause for the faulty compilation could be, in my mind it should be able to find the Entitas.Generators.Attributes.ContextAttribute correctly. What happens when you try getting syntaxContext.SemanticModel.Compilation.GetTypeByMetadataName("Entitas.Generators.Attributes.ContextAttribute")? is that one an ErrorType too?

In any case, do we really want it to only work when it can resolve the type? We're not using the attribute type for anything as far as I understand. Maybe checking if the Identity is equal to the context attribute is enough. We should follow the programmer's intent, and if we can verify that intent without depending on other parts of the system compiling correctly then I say we should go for it.

PS: does this issue occur only in the test case or when built too? I haven't checked that yet, but if it works then it's probably just an issue with creating a correct compilation in tests.

sschmid commented 1 year ago

Yes, it finds it correctly with syntaxContext.SemanticModel.Compilation.GetTypeByMetadataName("Entitas.Generators.Attributes.ContextAttribute")

I'll investigate more...

rubenwe commented 1 year ago

Looking at my tests I think I ran into similar problems... I had this in there:

internal struct ForceAssemblyLoadComponent : IComponent
{
}

public static class GeneratorTestHelper
{
    public static string GetGeneratedOutput<TGenerator>(string source, ITestOutputHelper helperOutput) 
        where TGenerator : IIncrementalGenerator, new()
    {
        var trick17 = (IComponent) new ForceAssemblyLoadComponent();
        var syntaxTree = CSharpSyntaxTree.ParseText(source);

        var references = new List<MetadataReference>();
        Assembly[] assemblies = AppDomain.CurrentDomain.GetAssemblies();
        foreach (var assembly in assemblies)
        {
            if (!assembly.IsDynamic && !string.IsNullOrWhiteSpace(assembly.Location))
            {
                references.Add(MetadataReference.CreateFromFile(assembly.Location));
            }
        }

        var compilation = CSharpCompilation.Create(
            "foo", 
            new SyntaxTree[] { syntaxTree }, 
            references, 
            new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));

        // TODO: Uncomment this line if you want to fail tests when the injected program isn't valid _before_ running generators
        // var compileDiagnostics = compilation.GetDiagnostics();
        // Assert.False(compileDiagnostics.Any(d => d.Severity == DiagnosticSeverity.Error), "Failed: " + compileDiagnostics.FirstOrDefault()?.GetMessage());

        IIncrementalGenerator generator = new TGenerator();

        var driver = CSharpGeneratorDriver.Create(generator);
        driver.RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var generateDiagnostics);
        Assert.False(generateDiagnostics.Any(d => d.Severity == DiagnosticSeverity.Error), "Failed: " + generateDiagnostics.FirstOrDefault()?.GetMessage());

        string output = outputCompilation.SyntaxTrees.Last().ToString();

        helperOutput.WriteLine(output);

        return output;
    }
}
sschmid commented 1 year ago

@rubenwe 😍😍😍

I just added this and concatenated it with my existing references, and now it works 😍😍😍

https://github.com/sschmid/Entitas/commit/abe5dc257b6a09ed32606140a7e54efcb37d6482

Thanks for the fix!

sschmid commented 1 year ago

More updates and fun things to share:

I just played around with Unsafe.As() :D For the new Entitas code gen I wanted to have sth like a type-safe int, e.g. when I create a Matcher with indexes, I want the compiler to be able to make sure I use the correct indexes from the same context and don't mix them with other contexts.

The generator generates one readonly struct ComponentIndex per context with only 1 int field. Matchers themselves work with int, so the one way of doing it could be:

public static Entitas.IAllOfMatcher<Entity> AllOf(Main.ComponentIndex[] indices)
{
    var ints = new int[indices.Length];
    for (var i = 0; i < indices.Length; i++)
        ints[i] = indices[i].Value;

    return Entitas.Matcher<Entity>.AllOf(ints);
}

This approach creates 2 arrays:

This is not ideal, because we now create a garbage extra array each time we create a matcher.

Another way to solve this could be to stackalloc an array of ComponenIndex on the stack and pass the span:

public static Entitas.IAllOfMatcher<Entity> AllOf(System.Span<ComponentIndex> indices)
{
    var ints = new int[indices.Length];
    for (var i = 0; i < indices.Length; i++)
        ints[i] = indices[i].Value;

    return Entitas.Matcher<Entity>.AllOf(ints);
}

This is better as it only create one array on the heap. You still need to copy the values.

I currently use another approach using Unsafe.As()

public static Entitas.IAllOfMatcher<Entity> AllOf(Main.ComponentIndex[] indices)
{
    var indexes = System.Runtime.CompilerServices.Unsafe.As<ComponentIndex[], int[]>(ref indices);
    return Entitas.Matcher<Entity>.AllOf(indexes);
}

I cannot direclty cast ComponentIndex[] to int[]. But since the ComponentIndex struct has the same size as an int (4 bytes) I can just ignore the compiler safety checks and treat ComponentIndex as int. Only one array is created now and no iteration or copying needs to happen. 😍

https://github.com/sschmid/Entitas/blob/roslyn-source-generators/gen/Entitas.Generators/Context/ContextGenerator.cs#L110-L135

Here are some examples from the integration tests https://github.com/sschmid/Entitas/blob/roslyn-source-generators/tests/Entitas.Generators.IntegrationTests/MatcherTests.cs

sschmid commented 1 year ago

...of course, cannot use Unsafe in Unity 😭 

error CS0122: 'Unsafe' is inaccessible due to its protection level

I already tries to allow unsafe code in the player settings, but no success yet... let's see if there's a solution.

Does anyone happen to know?

sschmid commented 1 year ago

There's also an option to allow unsafe code per asmdef, unfortunately this also doesn't help. However, Unity seems to offer their own impl of Unsafe.As()

var indexes = global::Unity.Collections.LowLevel.Unsafe.UnsafeUtility.As<ComponentIndex[], int[]>(ref indices);

I'll probably add sth like this:

    public static global::Entitas.IAllOfMatcher<Entity> AllOf(params ComponentIndex[] indices)
    {
#if NON_UNITY
        var indexes = global::System.Runtime.CompilerServices.Unsafe.As<ComponentIndex[], int[]>(ref indices);
#else
        var indexes = global::Unity.Collections.LowLevel.Unsafe.UnsafeUtility.As<ComponentIndex[], int[]>(ref indices);
#endif
        return global::Entitas.Matcher<Entity>.AllOf(indexes);
    }

Optimizing usablilty for Unity, as I expect most people will use Entitas with Unity. Non Unity C# project must add NON_UNITY compiler flag

Ants-Aare commented 1 year ago

I think it would make sense to test this on a couple of devices before pushing these changes out. How's the performance on this? is it actually casting it or is it a trick so the compiler won't complain? On another note... What do you think about using a bit array? If we know the number of components we can make a bit array. if there's 20 components in a context it will always pass around a bit array of length 20, etc. and then do a bitwise comparison for each component. I'm not 100% sure about the implementation of this since I haven't thought it through but my thinking was that when requesting a matcher it creates the full bit array and on each additional matcher call it will just modify the bit array in place. would look kinda like this then: boolean value that would match on FooComponent=> myContextBitArrayMatcher[0] boolean value that would match on BarComponent=> myContextBitArrayMatcher[1] boolean value that would match on FooBarComponent=> myContextBitArrayMatcher[2]

since it would be super fast to iterate over, take up super little memory and if we can figure out a way to modify it in place while still keeping usability good it'd be even faster and better.

On usability it would look something like this (idk if it's gonna work)

MyContext.CreateMatcher() //Allocates the bit array once
    .WithFooComponent() // a bitarray extension method type of deal that's specific to the namespace of the MyContext (?)
    .WithBarComponent() // Would set the bit at index 1 to true

idk maybe i'm missing something here and it wouldn't work as well as it's in my head.

sschmid commented 1 year ago

@Ants-Aare I'm totally down to discuss these kinds of improvements. I was also recently fantasizing about migrating Entitas to struct arrays at some point etc. I also met again with Maxim and talked about more efficient ways to store components.

But to stay focussed I will need to stick to my current roadmap which looks like this:

PereViader commented 1 year ago

I'd suggest to use the UNITY_ENGINE compiler flag instead of needing a new one NON_UNITY

sschmid commented 1 year ago

@PereViader I was hoping that sth like this exists, but unfortunately I don't think it does. I checked the docs https://docs.unity3d.com/Manual/PlatformDependentCompilation.html and I also had a look at the generated Unity csproj, they don't contain UNITY_ENGINE

But Unity includes UNITY_X_Y_OR_NEWER, e.g. UNITY_2020_1_OR_NEWER, so I will try using that instead of `NON_UNITY . I hope they will never remove those in the future :D

sschmid commented 1 year ago

@Ants-Aare About your Unsafe.As() question:

How's the performance on this?

Here are the benchmark results from https://github.com/sschmid/Entitas/blob/roslyn-source-generators/bench/Entitas.Benchmarks/ComponentIndexBenchmarks.cs

|                          Method |     Mean |     Error |    StdDev | Rank |   Gen0 | Allocated |
|-------------------------------- |---------:|----------:|----------:|-----:|-------:|----------:|
|  ComponentIndexUnsafeToIntArray | 3.305 ns | 0.0161 ns | 0.0151 ns |    1 | 0.0057 |      48 B |
|    SpanComponentIndexToIntArray | 6.592 ns | 0.0436 ns | 0.0408 ns |    2 | 0.0057 |      48 B |
| RefSpanComponentIndexToIntArray | 7.753 ns | 0.0161 ns | 0.0134 ns |    3 | 0.0057 |      48 B |
|        ComponentIndexToIntArray | 8.640 ns | 0.0518 ns | 0.0484 ns |    4 | 0.0115 |      96 B |

Also interesting that ref Span was always slower than normal Span in my runs...

sschmid commented 1 year ago

Hi my roslyn experts :D

I have another issue:

At one point I need to generate a class that collects all components from all assemblies per contexts, so I can assign an unique component index to them.

I attempted to solve this using the Entitas.Generators.Attributes.ContextInitializationAttribute. The developer can mark a custom partial method with this attribute and the code generator will generate the rest, e.g. ContextInitialization.cs which will generate sth like this (it found 4 components: Loading, Movable, Position, User)

using global::MyApp.Main;

namespace MyApp
{
    public static partial class ContextInitialization
    {
        public static partial void InitializeMain()
        {
            global::MyFeature.MyAppMainLoadingComponentIndex.Index = new ComponentIndex(0);
            global::MyFeature.MyAppMainMovableComponentIndex.Index = new ComponentIndex(1);
            global::MyFeature.MyAppMainPositionComponentIndex.Index = new ComponentIndex(2);
            global::MyFeature.MyAppMainUserComponentIndex.Index = new ComponentIndex(3);

            MyApp.MainContext.ComponentTypes = new global::System.Type[]
            {
                typeof(global::MyFeature.LoadingComponent),
                typeof(global::MyFeature.MovableComponent),
                typeof(global::MyFeature.PositionComponent),
                typeof(global::MyFeature.UserComponent)
            };
        }
    }
}

I created a new separat project with a component for a specific context to unit test if the generator also successfully finds all components from all assemblies. This must be supported in order to work well with Unity asmdef, where multiple projects may contain components.

The test fails and does not detect the HealthComponent.

Reproduce and debug

It successfully resolves HealthComponent when checking manually with

syntaxContext.SemanticModel.Compilation.GetTypeByMetadataName("MyOtherFeature.HealthComponent");

This means the test setup seems to be correct and all necessary dlls are loaded.

Question

How can I get all types that implement IComponent from the compilation from all assemblies?

I was also trying a CompilationProvider in the Initialize method with initContext.CompilationProvider, but I haven't found a way yet to get all components.

Does anyone know how I can get all components from the compilation?