Add ability to create new difficulties from within the editor #14799

bdach · 2021-09-18T19:34:01Z

Closes #12035.

UX likely is not final, but it's functional for now.

2021-09-18.20-42-12.mp4

The general flow is mostly a logical continuation of the preceding changes. By far the most finicky part here is the database stuff - there's a new BeatmapManager method here to facilitate appending a difficulty to an existing set, and some added safety checks in Save() to ensure uniquity of difficulty names - without them two or more difficulties could point at one .osu file and throw exceptions. With the changes here, an error notification will show instead:

I am aware of one more remaining problem - after a restart, if you go to edit an existing map with multiple difficulties, and save one of them, the rest will get truncated, but I'd prefer to tackle that one separately as this diff is a bit on the larger side already.

bdach · 2021-09-19T13:17:16Z

Upon investigating the issue mentioned above (with difficulties getting truncated) I've decided to bundle the fix for that here, as the fix is short and the issue is nigh untestable anyway (and I've tried, believe me). Sometimes after launch EF - or more concretely, BeatmapStore.Beatmaps - will return BeatmapInfos that have incomplete BeatmapSet.Beatmap lists and will only contain that one difficulty, hence the explicit re-query during editor load added in 406d9c2.

peppy · 2021-09-19T13:21:27Z

I'm actively wrangling with moving that stuff to realm, so should be fine to continue working around issues with local hacks as required.

osu.Game/Beatmaps/BeatmapInfo.cs

peppy · 2021-09-20T07:57:04Z

osu.Game/Graphics/UserInterface/DrawableOsuMenuItem.cs

@@ -118,6 +118,7 @@ public TextContainer()
                {
                    NormalText = new OsuSpriteText
                    {
+                        AlwaysPresent = true,


This definitely requires an inline comment explaining why it's required.

peppy · 2021-09-20T08:14:06Z

osu.Game/Beatmaps/BeatmapMetadata.cs

@@ -128,5 +129,7 @@ public bool Equals(BeatmapMetadata other)
                   && AudioFile == other.AudioFile
                   && BackgroundFile == other.BackgroundFile;
        }
+
+        public BeatmapMetadata DeepClone() => (BeatmapMetadata)MemberwiseClone();


I'm not sure if this is what we want. It means that individual difficulties can have differing artist/titles, which while technically is supported by stable and by the lazer database structure, is usually not what a mapper wants:

20210920.171322.dotnet.mp4

A good initial starting point might be to drop this and just have all difficulties share metadata until we have a better UX flow for exceptional cases in place.

peppy · 2021-09-20T08:18:35Z

It looks like on this branch, difficulty name changes no longer save:

20210920.171624.dotnet.mp4

I can't immediately see why this is happening, you might have more insight into this?

bdach · 2021-09-20T08:23:00Z

No idea off the top of my head. May be more EF chicanery. Will verify.

bdach · 2021-09-20T22:45:06Z

Fired some commits re: the red review above.

61e2617 should also fix the difficulty names not saving, although I'm not very confident of that fix as database stuff seems quite brittle to the touch the longer I stare at it in the context of this pull (see my discord messages from earlier today for context).

bdach · 2021-09-20T22:46:55Z

osu.Game/Screens/Edit/Editor.cs

@@ -519,7 +544,7 @@ public override bool OnExiting(IScreen next)
            // To update the game-wide beatmap with any changes, perform a re-fetch on exit.
            // This is required as the editor makes its local changes via EditorBeatmap
            // (which are not propagated outwards to a potentially cached WorkingBeatmap).
-            var refetchedBeatmap = beatmapManager.GetWorkingBeatmap(Beatmap.Value.BeatmapInfo);
+            var refetchedBeatmap = beatmapManager.GetWorkingBeatmap(editorBeatmap.BeatmapInfo);


The reason this is changed is that GetWorkingBeatmap() silently stores back the provided BeatmapInfo to the WorkingBeatmap. And because the beatmapset is refetched anew from the db context in BDL, the old code would pass a stale BeatmapInfo reference to the working beatmap, leading to things like the difficulty name updating on the carousel in song select but not the beatmap wedge on the left.

bdach · 2021-09-20T22:50:00Z

osu.Game/Screens/Edit/Editor.cs

+                    playableBeatmap.BeatmapInfo = refetchedBeatmapSet.Beatmaps.Single(b => b.ID == playableBeatmap.BeatmapInfo.ID);
+                    playableBeatmap.BeatmapInfo.Metadata ??= refetchedBeatmapSet.Metadata;


Note that this is now assigning to BeatmapInfo rather than just BeatmapInfo.BeatmapSet as the old code used to do, as per the comment above. As it turns out those two references could differ before, which is why the difficulty name changes didn't save.

As for the metadata shuffling, I needed to do this to prevent crashes in LegacyBeatmapEncoder, which would sometimes access EditorBeatmap.BeatmapInfo.Metadata (e.g. when trying to encode path to the background file) which isn't guaranteed to be present. This is a carbon copy of similar workarounds in BeatmapManager.GetWorkingBeatmap() and BeatmapCarousel.createCarouselSet().

peppy · 2021-10-01T09:33:47Z

osu.Game/Beatmaps/BeatmapManager.cs

+        /// <param name="beatmapSkin">The skin specific to the beatmap being added.</param>
+        /// <returns>The usable <see cref="WorkingBeatmap"/> corresponding to <paramref name="beatmap"/> and associated with the supplied <paramref name="beatmapSet"/>.</returns>
+        /// <exception cref="InvalidOperationException">If <paramref name="beatmapSet"/> does not exist in the database.</exception>
+        public virtual WorkingBeatmap AddToExistingSet(BeatmapSetInfo beatmapSet, IBeatmap beatmap, ISkin beatmapSkin = null)


I am suddenly regretting not merging this PR before my recent changes..

I'll close and rebase on top whatever the new structure ends up being. No worry.

Either way works. Was more of a joke than a serious issue (unlike the other comment further down).

peppy · 2021-10-01T09:53:51Z

I really want to get this in, and the code looks mostly fine, but I am still unfortunately able to break this in serious ways:

20211001.185226.dotnet.mp4

I can still get beatmaps into a state they disappear from my client permanently. Even after a restart they are gone.

peppy · 2021-10-01T09:55:50Z

As an aside, I was also looking at getting the song setup screen to appear (and focus the difficulty name entry field) when creating a new difficulty, which seems to work quite well using the existing isNewBeatmap bool. Maybe best left for a follow-up PR? Feels weird to be dumped into the compose mode when you haven't yet named your difficulty.

bdach · 2021-10-01T13:23:02Z

I am still unfortunately able to break this in serious ways

upon seeing that i'm leaning to closing this and reopening wholesale after realm. i have a distinct feeling that even if i fix whatever fresh hell is causing this there will be another terminal breakage somewhere else.

the way EF is set up is way too sensitive what with random attached or non attached model references being passed around (which was part of why i was arguing at length about that in discord today, until you explained what the plan for editor was which i can now agree with).

peppy · 2021-10-01T13:28:44Z

Bit unfortunate. I think it's fine to leave this dangling and push forward as progress is made over the next few weeks (keeping it roughly in sync).

bdach · 2022-01-23T19:02:14Z

I've attempted to come back to this today but too many things changed to make that feasible, so I will close this for now.

However, I have rewritten this under realm, and an early version is available for preview at https://github.com/bdach/osu/tree/new-difficulty-creation-v2. I won't be PRing it yet, because I need to figure out a few kinks (highlights: bdach@007e886, bdach@fe2b5f5), and I'd also like to split off a few incidental fixes and general changes out of there too (bdach@b2ae987, bdach@b482de0, maybe bdach@4e8ba02 too). I'll try to get the individual pieces together over the coming days.

bdach added 9 commits September 18, 2021 16:43

Split method for difficulty switch menu creation

3993bc3

Add menu for creating new difficulties

ccb4652

Fix menus potentially shrinking on hover

811dc75

Add flow for creating new difficulties

a938f73

Add test for new difficulty creation

3ad548b

Add failing tests for difficulty name clash cases

b052afb

Add protections against creating difficulty with same name as existing

af3ff03

Add editor override to respect IsolateSavingFromDatabase

01dfb77

Extract helper method for populating beatmap defaults

e475a2b

bdach added the area:editor label Sep 18, 2021

pull-request-size bot added the size/L label Sep 18, 2021

bdach added 2 commits September 19, 2021 14:55

Re-query beatmap set during editor load to prevent difficulty truncation

406d9c2

Fix difficulty creation tests not waiting for switch

c83cb58

Merge branch 'master' into new-difficulty-creation

16a7bfc

peppy reviewed Sep 20, 2021

View reviewed changes

osu.Game/Beatmaps/BeatmapInfo.cs Show resolved Hide resolved

peppy requested changes Sep 20, 2021

View reviewed changes

bdach added 3 commits September 20, 2021 22:34

Add comment about IsPresent usage

2f9463d

Share metadata between beatmaps in set rather than cloning

4324bcb

Fix changes to editor's BeatmapInfo not persisting to DB

61e2617

bdach commented Sep 20, 2021

View reviewed changes

Merge branch 'master' into new-difficulty-creation

a84a93a

peppy reviewed Oct 1, 2021

View reviewed changes

bdach closed this Jan 23, 2022

bdach mentioned this pull request Jan 23, 2022

Fix saved beatmap filename not including author name correctly #16586

Merged

bdach mentioned this pull request Feb 2, 2022

Add ability to create new difficulties from within the editor #16754

Merged

bdach deleted the new-difficulty-creation branch November 25, 2022 21:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to create new difficulties from within the editor #14799

Add ability to create new difficulties from within the editor #14799

bdach commented Sep 18, 2021

bdach commented Sep 19, 2021 •

edited

peppy commented Sep 19, 2021

peppy Sep 20, 2021

peppy Sep 20, 2021

peppy commented Sep 20, 2021

bdach commented Sep 20, 2021

bdach commented Sep 20, 2021

bdach Sep 20, 2021

bdach Sep 20, 2021

peppy Oct 1, 2021

bdach Oct 1, 2021

peppy Oct 1, 2021 •

edited

peppy commented Oct 1, 2021

peppy commented Oct 1, 2021

bdach commented Oct 1, 2021

peppy commented Oct 1, 2021

bdach commented Jan 23, 2022

		playableBeatmap.BeatmapInfo = refetchedBeatmapSet.Beatmaps.Single(b => b.ID == playableBeatmap.BeatmapInfo.ID);
		playableBeatmap.BeatmapInfo.Metadata ??= refetchedBeatmapSet.Metadata;

Add ability to create new difficulties from within the editor #14799

Add ability to create new difficulties from within the editor #14799

Conversation

bdach commented Sep 18, 2021

bdach commented Sep 19, 2021 • edited

peppy commented Sep 19, 2021

peppy Sep 20, 2021

Choose a reason for hiding this comment

peppy Sep 20, 2021

Choose a reason for hiding this comment

peppy commented Sep 20, 2021

bdach commented Sep 20, 2021

bdach commented Sep 20, 2021

bdach Sep 20, 2021

Choose a reason for hiding this comment

bdach Sep 20, 2021

Choose a reason for hiding this comment

peppy Oct 1, 2021

Choose a reason for hiding this comment

bdach Oct 1, 2021

Choose a reason for hiding this comment

peppy Oct 1, 2021 • edited

Choose a reason for hiding this comment

peppy commented Oct 1, 2021

peppy commented Oct 1, 2021

bdach commented Oct 1, 2021

peppy commented Oct 1, 2021

bdach commented Jan 23, 2022

bdach commented Sep 19, 2021 •

edited

peppy Oct 1, 2021 •

edited