stride3d / stride

Stride Game Engine (formerly Xenko)
https://stride3d.net
MIT License
6.34k stars 920 forks source link

Missing glslangValidator.exe for Vulkan builds #2001

Open Basewq opened 8 months ago

Basewq commented 8 months ago

Release Type: Official Release

Version: 4.1.0.1728 and beyond

Platform(s): Windows/Linux/Mac(?)

Describe the bug Running any projects with <StrideGraphicsApi>Vulkan</StrideGraphicsApi> crashes the app with the error System.AggregateException: 'One or more errors occurred. (One or more errors occurred. (An error occurred trying to start process 'win-x64\glslangValidator.exe' with working directory 'C:\[PATH]\FirstPersonShooter2\Bin\Windows\Debug'. The system cannot find the file specified.))'

To Reproduce Steps to reproduce the behavior:

  1. Start any template
  2. Add <StrideGraphicsApi>Vulkan</StrideGraphicsApi> to the .Windows.csproj file
  3. Build and run
  4. Encounter exception

Expected behavior Include glslangValidator.exe in the build output so the app doesn't crash.

Additional context Bug introduced by #1428 Stride.Shaders.Compiler package changed glslangValidator.exe/glslangValidator.bin to be treated as contentFiles in nuget package (as seen in C:\Users\[USERNAME]\.nuget\packages\stride.shaders.compiler\4.1.0.1948\stride.shaders.compiler.nuspec)

The game project indirectly references this library via Stride.Engine however that package declares all contentfiles as private assets so do not get added to the build output, ie. <PackageReference Include="Stride.Engine" Version="4.1.0.1948" PrivateAssets="contentfiles;analyzers" />

Workaround Explicitly reference Stride.Shaders.Compiler in the .Game.csproj to include contentFiles, ie. <PackageReference Include="Stride.Shaders.Compiler" Version="4.1.0.1948" />

azeno commented 8 months ago

Do we know the reason why it marks them as private?

Basewq commented 8 months ago

Do we know the reason why it marks them as private?

Fished very deep into the history https://github.com/stride3d/stride/commit/4c6ca70b399fc77bb0439916bbd0f240350e13f8#diff-32811e1522543d3084b14d6bc611f24e83a34dbe62e4aab8b3c7d326d9ed97ae

It seems back when Stride was a monolithic library (Xenko) it was all private, then xen2 just applied the same rules when he split them out.

However it seems private assets rules was removed for the 'New game' template, but not the other templates https://github.com/stride3d/stride/commit/d552c9780d83c705efa01257672165c527886efc#diff-deaae9294428885eae5c540bb7e6c3c6946236caddee12e4a480fa8181ed366f https://github.com/stride3d/stride/commit/105bd5cd007eb490f90e8952932c9cff1d971574#diff-deaae9294428885eae5c540bb7e6c3c6946236caddee12e4a480fa8181ed366f

@xen2 Is it safe to assume the 'New game' template is the 'master' template and the other templates should be adjusted to that? ie. Remove PrivateAssets="contentfiles;analyzers" from the PackageReference? It also seems Stride.Core.Assets.CompilerApp should change from IncludeAssets="build" to IncludeAssets="build;buildTransitive"

Having said that, even removing private asset rules from Stride.Engine package reference, it seems the content files from Stride.Shaders.Compiler are not included in the output. I'm guessing it's not transitive? Also, using the workaround, it appears the glslangValidator for all platforms ares included in the output instead of just the Windows variant (ie. Linux/OSX)

manio143 commented 8 months ago

I agree with aligning samples to follow the new project template. Default references for Stride packages and a specific build include for asset compiler.

As for all platforms being included in the output - that is necessary currently given that NuGet doesn't differentiate RID for packages, only TFM. Windows, Linux and OSX use all the same TFM net6.0 thus all native files are being included, regardless of the platform in the NuGet package. We could potentially write a custom target based on StridePlatform to run for the executable project to exclude those native deps from being copied to output, but I don't know how much hacking around that would invoke.