Skip to content
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
// Copyright (c) .NET Foundation and Contributors (https://dotnetfoundation.org/ & https://stride3d.net) and Silicon Studio Corp. (https://www.siliconstudio.co.jp)
// Distributed under the MIT license. See the LICENSE.md file in the project root for more information.
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using System.Threading.Tasks.Dataflow;
using Stride.Core;
using Stride.Core.Annotations;
using Stride.Core.Assets.Analysis;
using Stride.Core.Assets.Editor.Components.AddAssets;
using Stride.Core.Assets.Editor.Components.Properties;
Expand All @@ -19,8 +16,6 @@
using Stride.Core.Assets.Editor.ViewModel.Progress;
using Stride.Core.Assets.Templates;
using Stride.Core.Assets.Tracking;
using Stride.Core;
using Stride.Core.Annotations;
using Stride.Core.Diagnostics;
using Stride.Core.Extensions;
using Stride.Core.IO;
Expand All @@ -30,9 +25,9 @@
using Stride.Core.Presentation.Dirtiables;
using Stride.Core.Presentation.Interop;
using Stride.Core.Presentation.Services;
using Stride.Core.Translation;
using Stride.Core.Presentation.ViewModels;
using System.Collections;
using Stride.Core.Presentation.Windows;
using Stride.Core.Translation;

namespace Stride.Core.Assets.Editor.ViewModel
{
Expand Down Expand Up @@ -552,7 +547,7 @@ private string ComputeNamespace(DirectoryBaseViewModel directory)
private async Task<string> GetAssetCopyDirectory(DirectoryBaseViewModel directory, UFile file)
{
var path = directory.Path;
var message = Tr._p("Message", "Do you want to place the resource in the default location ?");
var message = Tr._p("Message", "Do you want to place the resource in the default location?");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I understand why you might want to fix that, it will break existing translations since the key string will be different. Either revert that change or update all the impacted .po and .pot files.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I completely missed this change, it might have just been picked up by some auto cleanup. I'll revert it for now

var finalPath = Path.GetFullPath(Path.Combine(directory.Package.Package.ResourceFolders[0], path, file.GetFileName()));
var pathResult = await Dialogs.MessageBoxAsync(message, MessageBoxButton.YesNo, MessageBoxImage.Question);
if (pathResult == MessageBoxResult.No)
Expand Down Expand Up @@ -587,39 +582,99 @@ [new FilePickerFilter("") { Patterns = [file.GetFileExtension()]}],

private async Task<List<AssetViewModel>> InvokeAddAssetTemplate(LoggerResult logger, string name, DirectoryBaseViewModel directory, TemplateAssetDescription templateDescription, [CanBeNull] IList<UFile> files, PropertyContainer? customParameters)
{
List<AssetViewModel> newAssets = new List<AssetViewModel>();
const int DialogClosed = 0;
const int DialogYes = 1;
const int DialogNo = 2;
const int DialogYesToAll = 3;
const int DialogNoToAll = 4;

var newAssets = new List<AssetViewModel>();
IReadOnlyList<DialogButtonInfo> copyPromptWithToAllButtons = DialogHelper.CreateButtons(
[
Tr._p("Button", "Yes"),
Tr._p("Button", "No"),
Tr._p("Button", "Yes to all"),
Tr._p("Button", "No to all")
], 1, 2);

IReadOnlyList<DialogButtonInfo> overwritePromptWithToAllButtons = DialogHelper.CreateButtons(
[
Tr._p("Button", "Yes"),
Tr._p("Button", "No"),
Tr._p("Button", "Yes to all")
], 1, 2);

IReadOnlyList<DialogButtonInfo> dialogDefaultButtons = DialogHelper.CreateButtons(
[
Tr._p("Button", "Yes"),
Tr._p("Button", "No")
], 1, 2);

var yesToAll = false;
var overwriteAll = false;
var finalPath = string.Empty;

if (files is not null)
{
for (int i = 0; i < files.Count; i++)
for (var i = 0; i < files.Count; i++)
{
var file = files[i];
bool inResourceFolder = directory.Package.Package.ResourceFolders.Any(x => file.FullPath.StartsWith(x.FullPath, StringComparison.Ordinal));
var inResourceFolder = directory.Package.Package.ResourceFolders.Any(x => file.FullPath.StartsWith(x.FullPath, StringComparison.Ordinal));

if (inResourceFolder)
{
continue;
}

if (!yesToAll)
{
var message = Tr._p("Message", "Source file '{0}' is not inside of your project's resource folders, do you want to copy it?").ToFormat(file.FullPath);

var message = Tr._p("Message", "Source file '{0}' is not inside of your project's resource folders, do you want to copy it?").ToFormat(file.FullPath);
var copyResult = await Dialogs.MessageBoxAsync(message, files.Count > 1 && i != files.Count - 1 ? copyPromptWithToAllButtons : dialogDefaultButtons, MessageBoxImage.Warning);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking i != files.Count - 1 makes checking files.Count > 1 unnecessary.


var copyResult = await Dialogs.MessageBoxAsync(message, MessageBoxButton.YesNo, MessageBoxImage.Warning);
if (copyResult is DialogClosed or DialogNo)
{
continue;
}

if (copyResult != MessageBoxResult.Yes)
continue;
if (copyResult is DialogNoToAll)
{
break;
}

if (copyResult is DialogYesToAll)
{
yesToAll = true;
}

string finalPath = await GetAssetCopyDirectory(directory, file);
if (copyResult is DialogYes or DialogYesToAll)
{
finalPath = await GetAssetCopyDirectory(directory, file);
}
}
else
{
// If "Yes to all" we're going to assume they want to use the same directory as the initial file.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a valid assumption? It sounds like it would flatten any hierarchy during the copy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding, but I believe this assumption is valid as this is consistent with the current behavior. If you select No to the default location, and specify a directory in the window that pops up, it puts it right where you tell it to without sub folder generation vs saying Yes to the default, which puts it in Resources/subfolder

finalPath = Path.Combine(Path.GetDirectoryName(finalPath), file.GetFileName());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
finalPath = Path.Combine(Path.GetDirectoryName(finalPath), file.GetFileName());
finalPath = Path.Join(Path.GetDirectoryName(finalPath), file.GetFileName());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not opposed to the change personally, my only hang up is that the rest of Stride uses Path.Combine. I could not find any uses of Path.Join in the code base, so I'm hesitant to deviate from that consistency.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path.Join is newer, faster, and likely exhibits behavior closer to coder intentions. The primary difference between the two is:

  • Path.Join is essentially concatenation.
  • Path.Combine checks if later parameters are rooted and excludes earlier parameters if they are.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While @ds5678 remark is valid, let's keep Path.Combine for now. We can explore Path.Join as a separate cleanup task throughout the whole codebase.

}

try
{
Directory.CreateDirectory(Path.GetDirectoryName(finalPath));
if (File.Exists(finalPath))
{
message = Tr._p("Message", "The file '{0}' already exists, it will get overwritten if you continue, do you really want to proceed?").ToFormat(finalPath);
if (!overwriteAll)
{
var message = Tr._p("Message", "The file '{0}' already exists, it will get overwritten if you continue, do you really want to proceed?").ToFormat(finalPath);

copyResult = await Dialogs.MessageBoxAsync(message, MessageBoxButton.YesNo, MessageBoxImage.Warning);
var copyResult = await Dialogs.MessageBoxAsync(message, files.Count > 1 && i != files.Count - 1 ? overwritePromptWithToAllButtons : dialogDefaultButtons, MessageBoxImage.Warning);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking i != files.Count - 1 makes checking files.Count > 1 unnecessary.


// Abort if the user says no or closes the prompt
if (copyResult != MessageBoxResult.Yes)
{
return newAssets;
overwriteAll = copyResult is DialogYesToAll;

if (copyResult is DialogClosed or DialogNo)
{
return newAssets;
}
}
File.Copy(file.FullPath, finalPath, true);
}
Expand All @@ -632,7 +687,7 @@ private async Task<List<AssetViewModel>> InvokeAddAssetTemplate(LoggerResult log
}
catch (Exception ex)
{
message = Tr._p("Message", $"An error occurred while copying the asset to the resources folder : {ex.Message}");
var message = Tr._p("Message", "An error occurred while copying the asset to the resources folder: {0}").ToFormat(ex.Message);
await Dialogs.MessageBoxAsync(message, MessageBoxButton.OK, MessageBoxImage.Error);
return newAssets;
}
Expand Down