prefersharpflat none by default for older scores

[MarcSabatella]

Resolves: #18425

The default for staff property "prefer sharps or flats for transposing instruments" has changed to a new "auto" setting that aims to simplify spellings, but this isn't compatible with older scores and leads to incorrect playback due to tpc confusion and also incorrect notation upon toggling concert pitch on/off. This PR forces older scores to default to "none" instead of "auto".

• 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)

[MarcSabatella]

My first commit fixes the problem for imported MuseScore 1-4 scores, second should do the same for MusicXML now that I've fixed the typo. I have no way of testing other imports, but probably Guitar Pro and Capella need a similar fix.

[MarcSabatella]

I don't understand the test failures. They appear to be showing extra preferSharpFlat tags being written which would be expected if the source files were 400 instead of 410, but at least the first few I looked at (beam-A, for example) are 410. So there shouldn't have been any change at all. I'd love any insight others can provide here.

I did also test myself to be sure that key signatures do continue to simplify by default in new scores, and that this survives save/reload.

[MarcSabatella]

It appears that the tests are generally reading scores via mu::engraving::compat::loadMsczOrMscx(), which in turn sets ignoreVersionError when opening scores. I can't follow all the logic from that point forward, but presumably that is how things go awry, and some additional tweak needs to be made to prevent the tests from treating 410 scores as 400 on load. Maybe?

[cbjeukendrup]

@MarcSabatella I think the problem is here:

MuseScore/src/engraving/rw/mscloader.cpp

Lines 67 to 69 in ab483e5

	if (version > 302 && MScore::testMode) {
	version = 302;
	}

But if we remove that, we'll get trouble too, since those tests have never fully been updated for the 400/410 file format.

[MarcSabatella]

@cbjeukendrup Oh my gosh, you're right, and I was looking right at that function but expecting to see something being forced to 400 so didn't even notice it being set to 302!

So needless to say this is not a good state of affairs, but also one that requires way more thinking and probably some hard decision making to deal with appropriately.

Having to update all the references to workaround that seems not right (plus, it's way more involved than I have in me). Maybe the stupid quick fix for now is to just stick a "if (!Mscore::testMode)" around the relevant code in my PR?

[sammik]

@MarcSabatella I created slightly different PR, hopefully, it wouldnt have this utests problem. Lets see #18453

[MarcSabatella]

@sammik Clever! Let me be sure I understand: the theory here is that if a key signature managed to be written with more than 6 accidentals, then clearly it couldn't really have had AUTO set, set we correct the part at that point. Meaning, most pre-4.1 will still be allowed to default to auto, which is actually nicer than mine - only those that would be likely to actually show the problem will be auto-corrected (!) upon read.

Do we think there would ever be a problem in the case of exactly six accidentals? Is it safe to assume in those cases, AUTO and NONE always do the same thing?

[sammik]

@sammik Clever! Let me be sure I understand: the theory here is that if a key signature managed to be written with more than 6 accidentals, then clearly it couldn't really have had AUTO set, set we correct the part at that point. Meaning, most pre-4.1 will still be allowed to default to auto, which is actually nicer than mine - only those that would be likely to actually show the problem will be auto-corrected (!) upon read.

Do we think there would ever be a problem in the case of exactly six accidentals? Is it safe to assume in those cases, AUTO and NONE always do the same thing?

Exactly. Just to clarify - transposing key signature with more than 6 accidentals couldnt have AUTO set.

As far as I can tell, exactly six accidentals are not exchanged, so AUTO and NONE do the same thing in that case, yes.

[MarcSabatella]

@sammik 's solution is better, I'll happily close this.