11565 confirmation dialog for reset preferences and make sure all preferences reset correctly

[krasko78]

Resolves: #11565

@cbjeukendrup @avvvvve @mathesoncalum
This PR replaces PR 25400 because I was unable to rebase due to my commits starting with "#" (a known issue it turned out) and it was easier to create a new branch and cherry-pick my changes to it. I will close 25400.

The first 24 commits are the same as those in PR 25400. The next address the code review comments left in PR 25400.
NOTES:

• As far as the dialog is concerned: whether the Cancel button will be specified as the default button or not does not make any difference now that the "Reset" button is the accented one. This means that pressing Space or Enter will execute the reset (which I personally am not fond of but at least is consistent behavior).

• The Cancel button should appear to the left of the Reset button on Mac and Linux (I didn't find any other dialog to use the ResetRole so I moved it to the right of RejectedRole)

• I've made the "First-time launch" wizard not appear after the reset which partially fixes #13299 too.

• I signed the CLA

• The title of the PR describes the problem it addresses

• Each commit's message describes its purpose and effects, and references the issue it resolves

• If changes are extensive, there is a sequence of easily reviewable commits

• The code in the PR follows the coding rules

• There are no unnecessary changes

• The code compiles and runs on my machine, preferably after each commit individually

• I created a unit test or vtest to verify the changes I made (if applicable)

[krasko78]

Whaaaaaat???? All checks have passed??? From the first time??? No kidding!!! 😂

[mathesoncalum]

What prompted these changes? After a factory reset I would expect the first launch wizard to appear again.

[krasko78]

This was a request by @avvvvve in the former PR and there is an open issue about it:

So some people think the wizard should not appear.

[mathesoncalum]

Ah thanks - I missed that message.

My only concern with this implementation is that revertToFactorySettings is also called when you do a factory reset from the "Help" menu, in which case @avvvvve I'm reasonably certain we should show the wizard again, right?

In practice your changes don't actually cause a problem here because keepDefaultSettings = false means we recursively remove the settings directory, so when you later call setHasCompletedFirstLaunchSetup(true), the setting can't be found and we do show the first launch wizard again.

To me this feels a little bit odd and potentially not very future-proof. Is there maybe somewhere else we can put this logic so that we only set the value to true when it's absolutely necessary?

[krasko78]

That's why I put it there - so it be executed both when resetting the preferences from the Preferences dialog and when doing a factory reset via the Help menu. In fact, #13299 is specifically about the factory reset.

Deleting the settings directory is not related here since the settings are kept in an .INI file in another location (at least on Windows) and although the settings have been cleared, as soon as setHasCompletedFirstLaunchSetup(true) is called, it will create the setting and the wizard won't appear even when doing a factory reset. Just double-checked it on my machine (again, Windows here).

Since this obviously needs a discussion between you, guys, how about we completely remove it from this PR and once you have a decision, implement it in #13299?

[mathesoncalum]

#13299 is about Preferences -> Reset preferences.

Just resetting preferences shouldn't cause the app to forget that you're an existing user who's already dismissed the start-up wizard, or what your recent files are. I'd expect that for "Revert to factory settings" in the Help menu. In fact both commands appear to do the same thing, except that the latter at least has a warning.

I think the way this part is worded might be causing some confusion. What they're saying here is that they would expect "revert to factory settings" to "cause the app to forget that you're an existing user..." (and thus show the wizard).

Deleting the settings directory is not related

This is interesting - there may indeed be a difference on Mac in this area. On my end I'm still seeing the first time launch wizard after a factory reset from the Help menu. Regardless, I would recommend dropping this commit from the PR until we know for certain what the desired behaviour is.

[avvvvve]

Just to be extra clear:
After 'Revert to factory settings', show the startup wizard on next launch.
After 'Reset preferences', do not show the startup wizard on next launch.

[krasko78]

I don't know why I thought #13299 was about factory reset. Now that I read that story again, I am wondering where my head was previously. So sorry about that! This actually makes our lives easier. Thanks guys! I'll move that line.

[mathesoncalum]

Thanks for doing those tweaks @krasko78 - looks good to me now!

[avvvvve]

@krasko78 A couple UI notes (1-2) and one functionality note (3)

1. Accented button

As far as the dialog is concerned: whether the Cancel button will be specified as the default button or not does not make any difference now that the "Reset" button is the accented one. This means that pressing Space or Enter will execute the reset (which I personally am not fond of but at least is consistent behavior).

Going back on my word here. After trying this out and seeing how it feels, I agree that Space/Enter should execute the accented button AND that for that reason, we should make Cancel the accented button instead of Reset/Revert.
Would it be possible to do that while keeping the order of the buttons as-is? i.e.

• Mac: [Cancel] [Reset]
• Windows: [Reset] [Cancel]

2. Dialog title
   Both dialogs have the generic "MuseScore Studio Development" title in the top bar. For reset preferences, it should say "Reset preferences" and for factory settings it should say "Revert to factory settings"

3. Permission confirmations on Mac
   On Mac when I reset preferences, I get the macOS system dialog "MuseScore Studio would like to access files in your Documents folder". I assume resetting preferences is also wiping the permission I previously granted MuseScore to read my files so it can display previews of them in the "Home" tab. I'm wondering if that's necessary to do during "Reset preferences"—it strikes me as something we could reserve for reverting to factory settings only.

Screen.Recording.2024-12-31.at.9.54.28.AM.mov

[krasko78]

Don't you have a New Year coming guys? :)

@avvvvve:

1. No problem. I'll do this for factory reset as well.
2. I am almost 100% sure we cannot change the title of the dialog. Do you happen to have an example of such a dialog in MuseScore at hand?
3. I'll check to see what might be causing it but I am not a MacOS user so I might not be able to figure it out.

Happy New Year guys!!! :)

[cbjeukendrup]

New Year is still a few hours away (here at least)... but happy new year too!

Re. 2: technically, it's possible; see StandardDialog.qml and InteractiveProvider. It is possible to change the window title of a StyledDialogView, using its title property, but in the case of StandardDialog, this property is shadowed by the property alias title: mainPanel.title property. FWIW, the default title "MuseScore Studio [Development]" comes from DialogView::beforeOpen().

Since this PR is already quite big, I wouldn't mind if you'd do this in a separate PR, but that's up to you.

Re. 3: it is unlikely that this is because resetting preferences erases any permission settings, because those should be only accessible by the OS itself (otherwise they would be pretty useless, if apps can manage their own permissions).
I'm not sure though what is the reason for this dialog. The only thing I can think of is that QSettings on macOS uses NSUserDefaults, and perhaps NSUserDefaults is also used to identify builds (normally you would only see that dialog once per build).
Or... ISTR that for builds that are not codesigned in any way, which is the case with PR builds, you would see such a dialog for every interaction with the file system, so not once per build. So we'd need to check it in Nightly Builds (which are signed and even notarised).

It might also be interesting to figure out why the Documents folder is being touched. The best way to find out might be to set some breakpoints in filesystem.cpp, although that won't catch calls that go directly via Qt APIs. I don't think there is any OS-specificness here, so this can be done on Windows as well. It may turn out that there is a good reason for it, but it might be good to check.

[krasko78]

I can now wish you all a Happy New Year officially!!! Best wishes for everyone and their beloved ones!!! And may it bring you more frequent MuseScore Studio releases!!! :))))

Thanks Casper for the detailed comment. :)))

1. Done.
2. Done.
3. The Documents folder is accessed because MuseScore reloads the sound fonts when the Folders -> SoundFonts folder setting is reset. We don't check if a setting being reset has its default value and send a valueChanged notification unconditionally. Maybe we should optimize this?

Regarding the permission alert: Per what Casper has said, let's see if it will disappear in the nightly builds.

[avvvvve]

@krasko78 this looks great to me! And sounds good about waiting to see how the 'Documents' alert behaves in the nightlies. Not a huge deal anyway.

And happy new year to you too! ✨

[krasko78]

I am bringing back this include. QT Creator was flagging it previously as not needed and ineed everything was building and working just fine for weeks until now when all of a sudden (on my machine at least) both Qt Creator and Visual Studio can't find muse::async::Asyncable without it. Go figure.