Merge pull request #18703 from frenzibyte/editor-metadata-name-unification

Use unified name for background/track files on editor import

osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs

@@ -98,11 +98,14 @@ namespace osu.Game.Tests.Visual.Editing
                 using (var zip = ZipArchive.Open(temp))
                     zip.WriteToDirectory(extractedFolder);
 
-                bool success = setup.ChildrenOfType<ResourcesSection>().First().ChangeAudioTrack(Path.Combine(extractedFolder, "03. Renatus - Soleily 192kbps.mp3"));
+                bool success = setup.ChildrenOfType<ResourcesSection>().First().ChangeAudioTrack(new FileInfo(Path.Combine(extractedFolder, "03. Renatus - Soleily 192kbps.mp3")));
 
                 File.Delete(temp);
                 Directory.Delete(extractedFolder, true);
 
+                // ensure audio file is copied to beatmap as "audio.mp3" rather than original filename.
+                Assert.That(Beatmap.Value.Metadata.AudioFile == "audio.mp3");
+
                 return success;
             });
 

osu.Game/Screens/Edit/Setup/FileChooserLabelledTextBox.cs

@@ -1,92 +0,0 @@
-// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
-// See the LICENCE file in the repository root for full licence text.
-
-using System;
-using System.Collections.Generic;
-using System.IO;
-using System.Linq;
-using System.Threading.Tasks;
-using osu.Framework.Allocation;
-using osu.Framework.Bindables;
-using osu.Framework.Extensions;
-using osu.Framework.Extensions.ObjectExtensions;
-using osu.Framework.Graphics;
-using osu.Framework.Graphics.Containers;
-using osu.Framework.Graphics.UserInterface;
-using osu.Game.Database;
-using osu.Game.Graphics.UserInterfaceV2;
-using osuTK;
-
-namespace osu.Game.Screens.Edit.Setup
-{
-    /// <summary>
-    /// A labelled textbox which reveals an inline file chooser when clicked.
-    /// </summary>
-    internal class FileChooserLabelledTextBox : LabelledTextBoxWithPopover, ICanAcceptFiles
-    {
-        private readonly string[] handledExtensions;
-
-        public IEnumerable<string> HandledExtensions => handledExtensions;
-
-        private readonly Bindable<FileInfo?> currentFile = new Bindable<FileInfo?>();
-
-        [Resolved]
-        private OsuGameBase game { get; set; } = null!;
-
-        public FileChooserLabelledTextBox(params string[] handledExtensions)
-        {
-            this.handledExtensions = handledExtensions;
-        }
-
-        protected override void LoadComplete()
-        {
-            base.LoadComplete();
-
-            game.RegisterImportHandler(this);
-            currentFile.BindValueChanged(onFileSelected);
-        }
-
-        private void onFileSelected(ValueChangedEvent<FileInfo?> file)
-        {
-            if (file.NewValue == null)
-                return;
-
-            this.HidePopover();
-            Current.Value = file.NewValue.FullName;
-        }
-
-        Task ICanAcceptFiles.Import(params string[] paths)
-        {
-            Schedule(() => currentFile.Value = new FileInfo(paths.First()));
-            return Task.CompletedTask;
-        }
-
-        Task ICanAcceptFiles.Import(params ImportTask[] tasks) => throw new NotImplementedException();
-
-        protected override void Dispose(bool isDisposing)
-        {
-            base.Dispose(isDisposing);
-
-            if (game.IsNotNull())
-                game.UnregisterImportHandler(this);
-        }
-
-        public override Popover GetPopover() => new FileChooserPopover(handledExtensions, currentFile);
-
-        private class FileChooserPopover : OsuPopover
-        {
-            public FileChooserPopover(string[] handledExtensions, Bindable<FileInfo?> currentFile)
-            {
-                Child = new Container
-                {
-                    Size = new Vector2(600, 400),
-                    Child = new OsuFileSelector(currentFile.Value?.DirectoryName, handledExtensions)
-                    {
-                        RelativeSizeAxes = Axes.Both,
-                        CurrentFile = { BindTarget = currentFile }
-                    },
-                };
-            }
-        }
-    }
-}

osu.Game/Screens/Edit/Setup/LabelledFileChooser.cs

@@ -0,0 +1,131 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Threading.Tasks;
+using osu.Framework.Allocation;
+using osu.Framework.Bindables;
+using osu.Framework.Extensions;
+using osu.Framework.Extensions.ObjectExtensions;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Cursor;
+using osu.Framework.Graphics.UserInterface;
+using osu.Framework.Localisation;
+using osu.Framework.Platform;
+using osu.Game.Database;
+using osu.Game.Graphics.UserInterfaceV2;
+using osuTK;
+
+namespace osu.Game.Screens.Edit.Setup
+{
+    /// <summary>
+    /// A labelled drawable displaying file chooser on click, with placeholder text support.
+    /// todo: this should probably not use PopoverTextBox just to display placeholder text, but is the best way for now.
+    /// </summary>
+    internal class LabelledFileChooser : LabelledDrawable<LabelledTextBoxWithPopover.PopoverTextBox>, IHasCurrentValue<FileInfo?>, ICanAcceptFiles, IHasPopover
+    {
+        private readonly string[] handledExtensions;
+
+        public IEnumerable<string> HandledExtensions => handledExtensions;
+
+        [Resolved]
+        private OsuGameBase game { get; set; } = null!;
+
+        /// <summary>
+        /// The initial path to use when displaying the <see cref="FileChooserPopover"/>.
+        /// </summary>
+        /// <remarks>
+        /// Uses a <see langword="null"/> value before the first selection is made
+        /// to ensure that the first selection starts at <see cref="GameHost.InitialFileSelectorPath"/>.
+        /// </remarks>
+        private string? initialChooserPath;
+
+        private readonly BindableWithCurrent<FileInfo?> current = new BindableWithCurrent<FileInfo?>();
+
+        public Bindable<FileInfo?> Current
+        {
+            get => current.Current;
+            set => current.Current = value;
+        }
+
+        public LocalisableString Text
+        {
+            get => Component.PlaceholderText;
+            set => Component.PlaceholderText = value;
+        }
+
+        public CompositeDrawable TabbableContentContainer
+        {
+            set => Component.TabbableContentContainer = value;
+        }
+
+        public LabelledFileChooser(params string[] handledExtensions)
+            : base(false)
+        {
+            this.handledExtensions = handledExtensions;
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            game.RegisterImportHandler(this);
+            Current.BindValueChanged(onFileSelected);
+        }
+
+        private void onFileSelected(ValueChangedEvent<FileInfo?> file)
+        {
+            if (file.NewValue != null)
+                this.HidePopover();
+
+            initialChooserPath = file.NewValue?.DirectoryName;
+        }
+
+        Task ICanAcceptFiles.Import(params string[] paths)
+        {
+            Schedule(() => Current.Value = new FileInfo(paths.First()));
+            return Task.CompletedTask;
+        }
+
+        Task ICanAcceptFiles.Import(params ImportTask[] tasks) => throw new NotImplementedException();
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            if (game.IsNotNull())
+                game.UnregisterImportHandler(this);
+        }
+
+        protected override LabelledTextBoxWithPopover.PopoverTextBox CreateComponent() => new LabelledTextBoxWithPopover.PopoverTextBox
+        {
+            Anchor = Anchor.Centre,
+            Origin = Anchor.Centre,
+            RelativeSizeAxes = Axes.X,
+            CornerRadius = CORNER_RADIUS,
+            OnFocused = this.ShowPopover,
+        };
+
+        public Popover GetPopover() => new FileChooserPopover(handledExtensions, Current, initialChooserPath);
+
+        private class FileChooserPopover : OsuPopover
+        {
+            public FileChooserPopover(string[] handledExtensions, Bindable<FileInfo?> currentFile, string? chooserPath)
+            {
+                Child = new Container
+                {
+                    Size = new Vector2(600, 400),
+                    Child = new OsuFileSelector(chooserPath, handledExtensions)
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                        CurrentFile = { BindTarget = currentFile }
+                    },
+                };
+            }
+        }
+    }
+}

osu.Game/Screens/Edit/Setup/ResourcesSection.cs

@@ -10,15 +10,14 @@ using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Localisation;
 using osu.Game.Beatmaps;
-using osu.Game.Graphics.UserInterfaceV2;
 using osu.Game.Overlays;
 
 namespace osu.Game.Screens.Edit.Setup
 {
     internal class ResourcesSection : SetupSection
     {
-        private LabelledTextBox audioTrackTextBox;
-        private LabelledTextBox backgroundTextBox;
+        private LabelledFileChooser audioTrackChooser;
+        private LabelledFileChooser backgroundChooser;
 
         public override LocalisableString Title => "Resources";
 
@@ -42,76 +41,81 @@ namespace osu.Game.Screens.Edit.Setup
         {
             Children = new Drawable[]
             {
-                backgroundTextBox = new FileChooserLabelledTextBox(".jpg", ".jpeg", ".png")
+                backgroundChooser = new LabelledFileChooser(".jpg", ".jpeg", ".png")
                 {
                     Label = "Background",
                     FixedLabelWidth = LABEL_WIDTH,
-                    PlaceholderText = "Click to select a background image",
-                    Current = { Value = working.Value.Metadata.BackgroundFile },
                     TabbableContentContainer = this
                 },
-                audioTrackTextBox = new FileChooserLabelledTextBox(".mp3", ".ogg")
+                audioTrackChooser = new LabelledFileChooser(".mp3", ".ogg")
                 {
                     Label = "Audio Track",
                     FixedLabelWidth = LABEL_WIDTH,
-                    PlaceholderText = "Click to select a track",
-                    Current = { Value = working.Value.Metadata.AudioFile },
                     TabbableContentContainer = this
                 },
             };
 
-            backgroundTextBox.Current.BindValueChanged(backgroundChanged);
-            audioTrackTextBox.Current.BindValueChanged(audioTrackChanged);
+            if (!string.IsNullOrEmpty(working.Value.Metadata.BackgroundFile))
+                backgroundChooser.Current.Value = new FileInfo(working.Value.Metadata.BackgroundFile);
+
+            if (!string.IsNullOrEmpty(working.Value.Metadata.AudioFile))
+                audioTrackChooser.Current.Value = new FileInfo(working.Value.Metadata.AudioFile);
+
+            backgroundChooser.Current.BindValueChanged(backgroundChanged);
+            audioTrackChooser.Current.BindValueChanged(audioTrackChanged);
+
+            updatePlaceholderText();
         }
 
-        public bool ChangeBackgroundImage(string path)
+        public bool ChangeBackgroundImage(FileInfo source)
         {
-            var info = new FileInfo(path);
-
-            if (!info.Exists)
+            if (!source.Exists)
                 return false;
 
             var set = working.Value.BeatmapSetInfo;
 
+            var destination = new FileInfo($@"bg{source.Extension}");
+
             // remove the previous background for now.
             // in the future we probably want to check if this is being used elsewhere (other difficulties?)
             var oldFile = set.Files.FirstOrDefault(f => f.Filename == working.Value.Metadata.BackgroundFile);
 
-            using (var stream = info.OpenRead())
+            using (var stream = source.OpenRead())
             {
                 if (oldFile != null)
                     beatmaps.DeleteFile(set, oldFile);
 
-                beatmaps.AddFile(set, stream, info.Name);
+                beatmaps.AddFile(set, stream, destination.Name);
             }
 
-            working.Value.Metadata.BackgroundFile = info.Name;
+            working.Value.Metadata.BackgroundFile = destination.Name;
             header.Background.UpdateBackground();
 
             return true;
         }
 
-        public bool ChangeAudioTrack(string path)
+        public bool ChangeAudioTrack(FileInfo source)
         {
-            var info = new FileInfo(path);
-
-            if (!info.Exists)
+            if (!source.Exists)
                 return false;
 
             var set = working.Value.BeatmapSetInfo;
 
+            var destination = new FileInfo($@"audio{source.Extension}");
+
             // remove the previous audio track for now.
             // in the future we probably want to check if this is being used elsewhere (other difficulties?)
             var oldFile = set.Files.FirstOrDefault(f => f.Filename == working.Value.Metadata.AudioFile);
 
-            using (var stream = info.OpenRead())
+            using (var stream = source.OpenRead())
             {
                 if (oldFile != null)
                     beatmaps.DeleteFile(set, oldFile);
-                beatmaps.AddFile(set, stream, info.Name);
+
+                beatmaps.AddFile(set, stream, destination.Name);
             }
 
-            working.Value.Metadata.AudioFile = info.Name;
+            working.Value.Metadata.AudioFile = destination.Name;
 
             music.ReloadCurrentTrack();
 
@@ -119,16 +123,31 @@ namespace osu.Game.Screens.Edit.Setup
             return true;
         }
 
-        private void backgroundChanged(ValueChangedEvent<string> filePath)
+        private void backgroundChanged(ValueChangedEvent<FileInfo> file)
         {
-            if (!ChangeBackgroundImage(filePath.NewValue))
-                backgroundTextBox.Current.Value = filePath.OldValue;
+            if (!ChangeBackgroundImage(file.NewValue))
+                backgroundChooser.Current.Value = file.OldValue;
+
+            updatePlaceholderText();
         }
 
-        private void audioTrackChanged(ValueChangedEvent<string> filePath)
+        private void audioTrackChanged(ValueChangedEvent<FileInfo> file)
         {
-            if (!ChangeAudioTrack(filePath.NewValue))
-                audioTrackTextBox.Current.Value = filePath.OldValue;
+            if (!ChangeAudioTrack(file.NewValue))
+                audioTrackChooser.Current.Value = file.OldValue;
+
+            updatePlaceholderText();
+        }
+
+        private void updatePlaceholderText()
+        {
+            audioTrackChooser.Text = audioTrackChooser.Current.Value == null
+                ? "Click to select a track"
+                : "Click to replace the track";
+
+            backgroundChooser.Text = backgroundChooser.Current.Value == null
+                ? "Click to select a background image"
+                : "Click to replace the background image";
         }
     }
 }