stride3d / stride

Stride (formerly Xenko), a free and open-source cross-platform C# game engine.
https://stride3d.net
MIT License
6.65k stars 957 forks source link

feat: Assembly Processor Module Initializer to Roslyn #2402

Closed IXLLEGACYIXL closed 2 months ago

IXLLEGACYIXL commented 4 months ago

PR Details

Entry point to convert assembly processor to roslyn.

  1. ModuleInitializers are source generated through roslyn not mono cecil
  2. ModuleInitializerProcessor is gone
  3. converted all mono.cecil processors to directly write into the < Module > instead of writing to ModuleInitalizer which then gets scanned just for the sake to be scanned again

Related Issue

didnt find yet the tracking

Types of changes

Checklist

Remarks

Order of ModuleInitializers is a horrible concept, the reason why it was done from my analysis AssemblyScan and SerializationProcessor had to be registered before anything else, IF these arent the first ones to be registered stride just dies at multiple points, which means if you give a higher priority of -999 and you rely on serialization then it just breaks

There is no other usage of it, so if the rest gets translated to roslyn, we can generate first the assemblyscan and serialization register AND then add all others in whatever order

IXLLEGACYIXL commented 4 months ago

ready for review, it works no idea about hte assemblyprocessor change in the json? should that be ? didnt do that intentionally

until the attribute question isnt solved icant continue with roslyn, so this is as far as it gets

Fydar commented 4 months ago

When referencing a source generator from a project there are some additional parameters you need:

<ProjectReference Include="SourceGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />

That's probably why the .deps.json is changing.

IXLLEGACYIXL commented 4 months ago

When referencing a source generator from a project there are some additional parameters you need:

<ProjectReference Include="SourceGenerator.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />

That's probably why the .deps.json is changing.

the assembly processor doesnt project reference stride.core which is currently the way its implemented that if you reference stride.core you get the compilerservices see Stride.Core.CompilerServices.props

IXLLEGACYIXL commented 4 months ago

@tebjan could you try if it affects vvvv or if you see shader problems? Tests are passing but not sure if tests cover that much

Kryptos-FR commented 3 months ago

This PR is important for the Avalonia port (in the area of loading the editor "plugins" and discovering the types for views and viewmodels).

Since Avalonia 11.1.x, the old module initializer doesn't work anymore because Avalonia own IL post-processing seems to break our generated <Module> class. It didn't occur with Avalonia 11.0.x and older versions.

I tried this PR on top of Avalonia 11.1.x and it seems to work again.

IXLLEGACYIXL commented 2 months ago

can this be reviewed so avalonia and roslyn transition can continue?

IXLLEGACYIXL commented 2 months ago

Overall it looks good to me. However, I tried to run it on Teamcity, and it seems incremental build is broken.

image As you can see on this image, if I rebuild master twice in a row, the second build takes only 1 min. Looking at the log, I can see lot of stuff skipped away like this:

Skipping target "CoreCompile" because all output files are up-to-date with respect to the input files.

However, this doesn't happen anymore with this branch.

Let me know if you need help looking into it.

is this now an issue with this branch or is it better ?

i dont know yet what i can do about it, the asemblyprocessor has to be rebuilt and wasnt there an issue with rebuilding it? Like i dont know what to do as teh tests of the engine passed

xen2 commented 2 months ago

Sorry if I wasn't clear. It is a regression introduced in this branch.

It seems using this incremental generator breaks the "rebuild" optimizations. If I was to not change any file (or only in GameStudio.dll), it would skip most of the rebuild as C# input files are same. But with this branch, it seems it rebuilds everything, even if there are no changes.

I assume it's because it needs to know the "dependencies" (file read) when generating code. I will take a quick look at if/how it can be controlled with source generators.

xen2 commented 2 months ago

FYI, it seems to work on my computer. It might be just the build agent. Updating to latest VS2022, and if not enough I will investigate/debug on the build agent itself.

xen2 commented 2 months ago

Please disregard previous comments, it was simply due to the agent doing a git clean when not on master (on TeamCity, I used "Clean On Branch Change", it shouldn't have happened with two builds of this branch in a row, somehow master is treated differently....)

IXLLEGACYIXL commented 2 months ago

merging this also means nugets using the stride libs need to update their stride core and need to be recompiled so the genreator can be applied

IXLLEGACYIXL commented 2 months ago

someone could try to measure build time if it got faster or slower if someone is bored but its not too important

Eideren commented 2 months ago

Looks like this PR is ready to be merge ?

IXLLEGACYIXL commented 2 months ago

seems so yes

Eideren commented 2 months ago

Thanks !