Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add skin mounting flow #30226

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
4 changes: 3 additions & 1 deletion osu.Game/Database/RealmArchiveModelImporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,9 @@ public async Task<ExternalEditOperation<TModel>> BeginExternalEditing(TModel mod

Directory.CreateDirectory(mountedPath);

foreach (var realmFile in model.Files)
// Detach files from the model to avoid realm contention when copying to the external location.
// This is safe as we are not modifying the model in any way.
foreach (var realmFile in model.Files.Detach())
Copy link
Contributor Author

@smallketchup82 smallketchup82 Oct 11, 2024

Choose a reason for hiding this comment

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

I ran into an issue where detaching the skin doesn't actually end up detaching the files, resulting in a Realms.Exceptions.RealmException: Realm accessed from incorrect thread. exception.

I couldn't find a way to fix it apart from this. Fortunately, we only access the realm files here to mount them, so we aren't modifying the files in a way that requires them to be attached.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Two questions:

  • Where / when is the "skin" being "detached"?
  • Does detach even... work for SkinInfo? It sure isn't registered in the automapper detach machinery.

c.CreateMap<RealmKeyBinding, RealmKeyBinding>();
c.CreateMap<BeatmapMetadata, BeatmapMetadata>();
c.CreateMap<BeatmapUserSettings, BeatmapUserSettings>();
c.CreateMap<BeatmapDifficulty, BeatmapDifficulty>();
c.CreateMap<RulesetInfo, RulesetInfo>();
c.CreateMap<ScoreInfo, ScoreInfo>();
c.CreateMap<RealmUser, RealmUser>();
c.CreateMap<RealmFile, RealmFile>();
c.CreateMap<RealmNamedFileUsage, RealmNamedFileUsage>();

Maybe adding SkinInfo in there just fixes this. I dunno.

Copy link
Contributor Author

@smallketchup82 smallketchup82 Oct 14, 2024

Choose a reason for hiding this comment

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

The skin is being detached here

CurrentSkin.Value.SkinInfo in SkinEditor returns a Live<SkinInfo>. My original idea for fixing the issue with "realm being accessed from an incorrect thread" was to detach the Live<SkinInfo> so that we're only working with a SkinInfo. Though, simply stopping here (without detaching Files) doesn't work and results in the thread error. Adding in the .Detach() to model.Files fixes it.

I got here via trial and error so I don't know for certain. But my best guess is that it likely has to do with RealmNamedFileUsage being in the automapper detach machinery in which you specified. Maybe it's only detaching the Files that ends up fixing the issue, not necessarily the detaching of the Live<SkinInfo>.

At any rate, I guess detaching the Live<SkinInfo> serves only to convert from Live<SkinInfo> to SkinInfo. But a quick test without detaching the Live<SkinInfo> seems to work fine. So maybe that's unnecessary?

{
string sourcePath = Files.Storage.GetFullPath(realmFile.File.GetStoragePath());
string destinationPath = Path.Join(mountedPath, realmFile.Filename);
Expand Down
10 changes: 10 additions & 0 deletions osu.Game/Localisation/EditorStrings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ public static class EditorStrings
/// </summary>
public static LocalisableString TimelineShowTimingChanges => new TranslatableString(getKey(@"timeline_show_timing_changes"), @"Show timing changes");

/// <summary>
/// "Edit externally"
/// </summary>
public static LocalisableString EditExternally => new TranslatableString(getKey(@"edit_externally"), @"Edit externally");

/// <summary>
/// "Finish editing and import changes"
/// </summary>
public static LocalisableString FinishEditingExternally => new TranslatableString(getKey(@"Finish editing and import changes"), @"Finish editing and import changes");

/// <summary>
/// "Show ticks"
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions osu.Game/OsuGame.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,7 @@ protected override void LoadComplete()
loadComponentSingleFile(beatmapSetOverlay = new BeatmapSetOverlay(), overlayContent.Add, true);
loadComponentSingleFile(wikiOverlay = new WikiOverlay(), overlayContent.Add, true);
loadComponentSingleFile(skinEditor = new SkinEditorOverlay(ScreenContainer), overlayContent.Add, true);
loadComponentSingleFile(new ExternalEditOverlay(), overlayContent.Add, true);

loadComponentSingleFile(new LoginOverlay
{
Expand Down
268 changes: 268 additions & 0 deletions osu.Game/Overlays/ExternalEditOverlay.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,268 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using System.IO;
using System.Threading.Tasks;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Extensions;
using osu.Framework.Extensions.Color4Extensions;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Graphics.Shapes;
using osu.Framework.Input.Events;
using osu.Framework.Logging;
using osu.Framework.Platform;
using osu.Framework.Testing;
using osu.Game.Database;
using osu.Game.Graphics;
using osu.Game.Graphics.Containers;
using osu.Game.Graphics.Sprites;
using osu.Game.Graphics.UserInterface;
using osu.Game.Graphics.UserInterfaceV2;
using osu.Game.Input.Bindings;
using osu.Game.Localisation;
using osu.Game.Online.Multiplayer;
using osu.Game.Screens.OnlinePlay.Match.Components;
using osu.Game.Skinning;
using osuTK;
using osuTK.Graphics;

namespace osu.Game.Overlays
{
public partial class ExternalEditOverlay : OsuFocusedOverlayContainer
{
private const double transition_duration = 300;
private FillFlowContainer flow = null!;

[Cached]
private readonly OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Purple);
Copy link
Member

Choose a reason for hiding this comment

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

It should inherit from the skin editor's colours, no?


[Resolved]
private GameHost gameHost { get; set; } = null!;

private ExternalEditOperation<SkinInfo>? editOperation;

private Bindable<Skin>? skinBindable;
private SkinManager? skinManager;

protected override bool DimMainContent => false;

[BackgroundDependencyLoader]
private void load()
{
RelativeSizeAxes = Axes.Both;
InternalChild = new Container
{
RelativeSizeAxes = Axes.Both,
Masking = true,
Children = new Drawable[]
{
// Since we're drawing this overlay on top of another overlay (SkinEditor), the dimming effect isn't applied. So we need to add a dimming effect manually.
new Box
{
Colour = Color4.Black.Opacity(0.5f),
RelativeSizeAxes = Axes.Both,
},
new Container
{
Masking = true,
CornerRadius = 20,
AutoSizeAxes = Axes.Both,
AutoSizeDuration = 500,
AutoSizeEasing = Easing.OutQuint,
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
Children = new Drawable[]
{
new Box
{
Colour = colourProvider.Background5,
RelativeSizeAxes = Axes.Both,
},
flow = new FillFlowContainer
{
Margin = new MarginPadding(20),
AutoSizeAxes = Axes.Both,
Direction = FillDirection.Vertical,
Anchor = Anchor.TopCentre,
Origin = Anchor.TopCentre,
Spacing = new Vector2(15),
}
}
}
}
};
}

public async Task Begin(SkinInfo skinInfo, Bindable<Skin> skinBindable, SkinManager skinManager)
{
Show();
showSpinner("Mounting external skin...");

await Task.Delay(500).ConfigureAwait(true);

try
{
editOperation = await skinManager.BeginExternalEditing(skinInfo).ConfigureAwait(false);
}
catch (Exception ex)
{
Logger.Log($"Failed to initialize external edit operation: {ex}", LoggingTarget.Database, LogLevel.Error);
Schedule(() => showSpinner("Export failed!"));
await Task.Delay(1000).ConfigureAwait(true);
Hide();
}

this.skinBindable = skinBindable;
this.skinManager = skinManager;

Schedule(() =>
{
flow.Children = new Drawable[]
{
new OsuSpriteText
{
Text = "Skin is mounted externally",
Font = OsuFont.Default.With(size: 30),
Anchor = Anchor.TopCentre,
Origin = Anchor.TopCentre,
},
new OsuTextFlowContainer
{
Padding = new MarginPadding(5),
Anchor = Anchor.TopCentre,
Origin = Anchor.TopCentre,
Width = 350,
AutoSizeAxes = Axes.Y,
Text = "Any changes made to the exported folder will be imported to the game, including file additions, modifications and deletions.",
},
new PurpleRoundedButton
{
Text = "Open folder",
Width = 350,
Anchor = Anchor.TopCentre,
Origin = Anchor.TopCentre,
Action = openDirectory,
Enabled = { Value = false }
},
new DangerousRoundedButton
{
Text = EditorStrings.FinishEditingExternally,
Width = 350,
Anchor = Anchor.TopCentre,
Origin = Anchor.TopCentre,
Action = () => finish().FireAndForget(),
Enabled = { Value = false }
}
};
});

Scheduler.AddDelayed(() =>
{
foreach (var b in flow.ChildrenOfType<RoundedButton>())
b.Enabled.Value = true;
openDirectory();
}, 1000);
}

private void openDirectory()
{
if (editOperation == null)
return;

gameHost.OpenFileExternally(editOperation.MountedPath.TrimDirectorySeparator() + Path.DirectorySeparatorChar);
}

private async Task finish()
{
showSpinner("Cleaning up...");
await Task.Delay(500).ConfigureAwait(true);

try
{
await editOperation!.Finish().ConfigureAwait(false);
}
catch (Exception ex)
{
Logger.Log($"Failed to finish external edit operation: {ex}", LoggingTarget.Database, LogLevel.Error);
showSpinner("Import failed!");
await Task.Delay(1000).ConfigureAwait(true);
Hide();
}

Schedule(() =>
{
var oldSkin = skinBindable!.Value;
var newSkinInfo = oldSkin.SkinInfo.PerformRead(s => s);

// Create a new skin instance to ensure the skin is reloaded
// If there's a better way to reload the skin, this should be replaced with it.
skinBindable.Value = newSkinInfo.CreateInstance(skinManager!);

oldSkin.Dispose();

Hide();
});
}

protected override void PopIn()
{
this.FadeIn(transition_duration, Easing.OutQuint);
}

protected override void PopOut()
{
this.FadeOut(transition_duration, Easing.OutQuint).Finally(_ =>
{
// Set everything to a clean state
editOperation = null;
skinManager = null;
skinBindable = null;
flow.Children = Array.Empty<Drawable>();
});
}

public override bool OnPressed(KeyBindingPressEvent<GlobalAction> e)
{
if (e.Repeat)
return false;

switch (e.Action)
{
case GlobalAction.Back:
case GlobalAction.Select:
if (editOperation == null) return base.OnPressed(e);

finish().FireAndForget();
return true;
}

return base.OnPressed(e);
}

private void showSpinner(string text)
{
foreach (var b in flow.ChildrenOfType<RoundedButton>())
b.Enabled.Value = false;

flow.Children = new Drawable[]
{
new OsuSpriteText
{
Text = text,
Font = OsuFont.Default.With(size: 30),
Anchor = Anchor.TopCentre,
Origin = Anchor.TopCentre,
},
new LoadingSpinner
{
Anchor = Anchor.TopCentre,
Origin = Anchor.TopCentre,
State = { Value = Visibility.Visible }
},
};
}
}
}
11 changes: 11 additions & 0 deletions osu.Game/Overlays/SkinEditor/SkinEditor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ public partial class SkinEditor : VisibilityContainer, ICanAcceptFiles, IKeyBind
[Resolved]
private IDialogOverlay? dialogOverlay { get; set; }

[Resolved]
private ExternalEditOverlay? externalEditOverlay { get; set; }

public SkinEditor()
{
}
Expand Down Expand Up @@ -157,6 +160,7 @@ private void load(EditorClipboard clipboard)
{
new EditorMenuItem(Web.CommonStrings.ButtonsSave, MenuItemType.Standard, () => Save()),
new EditorMenuItem(CommonStrings.Export, MenuItemType.Standard, () => skins.ExportCurrentSkin()) { Action = { Disabled = !RuntimeInfo.IsDesktop } },
new EditorMenuItem(EditorStrings.EditExternally, MenuItemType.Standard, () => _ = editExternally()) { Action = { Disabled = !RuntimeInfo.IsDesktop } },
new OsuMenuItemSpacer(),
new EditorMenuItem(CommonStrings.RevertToDefault, MenuItemType.Destructive, () => dialogOverlay?.Push(new RevertConfirmDialog(revert))),
new OsuMenuItemSpacer(),
Expand Down Expand Up @@ -274,6 +278,13 @@ protected override void LoadComplete()
selectedTarget.BindValueChanged(targetChanged, true);
}

private async Task editExternally()
{
var skin = currentSkin.Value.SkinInfo.PerformRead(s => s.Detach());

await externalEditOverlay!.Begin(skin, currentSkin, skins).ConfigureAwait(false);
}

public bool OnPressed(KeyBindingPressEvent<PlatformAction> e)
{
switch (e.Action)
Expand Down
2 changes: 1 addition & 1 deletion osu.Game/Screens/Edit/Editor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1210,7 +1210,7 @@ private IEnumerable<MenuItem> createFileMenuItems()
saveRelatedMenuItems.AddRange(export.Items);
yield return export;

var externalEdit = new EditorMenuItem("Edit externally", MenuItemType.Standard, editExternally);
var externalEdit = new EditorMenuItem(EditorStrings.EditExternally, MenuItemType.Standard, editExternally);
saveRelatedMenuItems.Add(externalEdit);
yield return externalEdit;
}
Expand Down
3 changes: 2 additions & 1 deletion osu.Game/Screens/Edit/ExternalEditScreen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using osu.Game.Graphics.Sprites;
using osu.Game.Graphics.UserInterface;
using osu.Game.Graphics.UserInterfaceV2;
using osu.Game.Localisation;
using osu.Game.Online.Multiplayer;
using osu.Game.Overlays;
using osu.Game.Screens.OnlinePlay.Match.Components;
Expand Down Expand Up @@ -156,7 +157,7 @@ private async Task begin()
},
new DangerousRoundedButton
{
Text = "Finish editing and import changes",
Text = EditorStrings.FinishEditingExternally,
Width = 350,
Anchor = Anchor.TopCentre,
Origin = Anchor.TopCentre,
Expand Down
Loading
Loading