stride icon indicating copy to clipboard operation
stride copied to clipboard

[Assets] Prevent crash of the assets compiler when an assembly cannot be fully loaded.

Open Kryptos-FR opened this issue 1 year ago • 3 comments

PR Details

Description

Skip looking for asset compilers in an assembly if its types can't be loaded.

Related Issue

Fixes #2140.

Motivation and Context

Types of changes

  • [ ] Docs change / refactoring / dependency upgrade
  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • [ ] My change requires a change to the documentation.
  • [ ] I have added tests to cover my changes.
  • [ ] All new and existing tests passed.
  • [x] I have built and run the editor to try this change out.

Kryptos-FR avatar Feb 07 '24 10:02 Kryptos-FR

As an alternative we could also introduce an assembly attribute telling the asset compiler whether or not the assembly even contains such implementations.

We would need to add that attribute to the existing code base (or even better let it get added by the assembly processor).

Not sure about the proper name yet, but for example

[assembly: ContainsAssetCompilers]

Type loading could now be skipped entirely, maybe also speeding up the overall compile process.

azeno avatar Feb 07 '24 11:02 azeno

As an alternative we could also introduce an assembly attribute telling the asset compiler whether or not the assembly even contains such implementations.

It's already based on the registry which only takes assembly from the assets category. We don't scan every single assembly and their dependencies, that would be overkill.

But the Game project is such an assembly since it does contain assets. It's just not supposed to have platform-specific dependencies such as Windows Forms. Your case is uncommon as a cleaner architecture would have all Windows dependencies on the Windows-specific project. The Game project is supposed to be cross-platform.

Kryptos-FR avatar Feb 07 '24 12:02 Kryptos-FR

Ok thanks. Makes me wonder why many of our projects would even be put in the assets category then. We have a dependency on Stride.Core.Mathematics very deep down. I guess that's the reason why the assembly processor even comes into play. Maybe there're some tweaks to ensure it doesn't run for those projects (setting StrideAssemblyProcessor to false for example or setting ExcludeAssets = "buildTransitive"), will need to check. But in any case, at some point we will run into a project which does contain Stride assets and has an (unfortunate) dependency on System.Windows.Forms. Therefor I'd be happy with the proposed workaround.

azeno avatar Feb 07 '24 12:02 azeno

Any chance this PR can be merged? It's currently blocking things at our end regarding a project which needs https://github.com/stride3d/stride/pull/2141

azeno avatar Feb 23 '24 12:02 azeno