fix(msbuild): Stride.AssetCompiler.targets - use Path.Combine for paths - #3340
Open
VaclavElias wants to merge 1 commit into
Open
fix(msbuild): Stride.AssetCompiler.targets - use Path.Combine for paths#3340VaclavElias wants to merge 1 commit into
VaclavElias wants to merge 1 commit into
Conversation
- Use Path.Combine for StrideCompileAssetBuildPath and StrideCompileAssetUpToDateCheckFileBase - Fixes malformed paths for file-based apps (dotnet run app.cs) whose intermediate output paths are already rooted
Contributor
Author
|
References: https://learn.microsoft.com/en-us/dotnet/core/sdk/file-based-apps Additional context, this change will allow to run Stride (code-only) from any C# file e.g. Without this PR, we have to add also these two lines #:package Stride.CommunityToolkit.Bepu@1.0.0-dev
#:package Stride.CommunityToolkit.Skyboxes@1.0.0-dev
#:package Stride.CommunityToolkit.Windows@1.0.0-dev
using Stride.CommunityToolkit.Bepu;
using Stride.CommunityToolkit.Engine;
using Stride.CommunityToolkit.Rendering.ProceduralModels;
using Stride.CommunityToolkit.Skyboxes;
using Stride.Core.Mathematics;
using Stride.Engine;
using var game = new Game();
game.Run(start: Start);
void Start(Scene rootScene)
{
game.SetupBase3DScene();
game.AddSkybox();
var entity = game.Create3DPrimitive(PrimitiveModelType.Capsule);
entity.Transform.Position = new Vector3(0, 8, 0);
entity.Scene = rootScene;
} |
xen2
requested changes
Aug 10, 2026
| relative to the project for a normal project, but absolute for a file-based app | ||
| (dotnet run app.cs), whose obj/bin live under the SDK's temp cache. Path.Combine returns | ||
| the second path unchanged when it is already rooted, so the relative case is unaffected. --> | ||
| <StrideCompileAssetBuildPath Condition="'$(StrideCompileAssetBuildPath)' == ''">$([System.IO.Path]::Combine('$(ProjectDir)', '$(BaseIntermediateOutputPath)stride\assetbuild\data'))</StrideCompileAssetBuildPath> |
Member
There was a problem hiding this comment.
Let's use this instead:
$([MSBuild]::NormalizePath('$(ProjectDir)', '$(BaseIntermediateOutputPath)', 'stride\assetbuild\data'))
It would match other use (i.e. line 65) and it applies GetFullPath which resolves ..\ from the resulting path and OK with missing trailing slash).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Classification
Bug fix to ensure correct path handling in asset build processes.
PR Summary
Replaced string concatenation with
System.IO.Path.CombineforStrideCompileAssetBuildPathandStrideCompileAssetUpToDateCheckFileBaseinStride.AssetCompiler.targets, ensuring intermediate output paths are handled correctly for both normal projects and file-based apps (dotnet run app.cs), whoseobj/binpaths are already rooted under the SDK's temp cache.Stride.AssetCompiler.targets: usePath.Combineinstead of concatenation forStrideCompileAssetBuildPathandStrideCompileAssetUpToDateCheckFileBase; added explanatory commentsTesting notes
This PR fixes
StrideCompileAssetBuildPathandStrideCompileAssetUpToDateCheckFileBaseto usePath.Combineinstead of string concatenation, so absolute intermediate paths (e.g. file-basedapps via
dotnet run app.cs, whose obj/bin live under the SDK's temp cache) are handled correctlyinstead of producing a malformed path.
Verified on Windows.
Path.Combine/Path.IsPathRootedrootedness semantics can differ slightlyacross platforms, so it would be good to double check normal + file-based app builds on
Linux/macOS, if a maintainer has the setup handy.