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

preferences to disable chord symbol playback when opening old scores, or creating new ones #6259

Merged
merged 3 commits into from
Oct 5, 2020

Conversation

MarcSabatella
Copy link
Contributor

This adds checks for the version that used when a file was saved,
so we can force older scores (both pre-3.0 and 3.0 - 3.4.2)
to have the playHarmony style setting off by default,
for better playback compatibility with older versions.
This code is a hack but is set up in a way
to easily allow other new style settings to be handled in a similar way.
New scores continue to have chord symbol playback enabled by default.
In order to get scores created from (possibly older) templates
to also have chord symbol playback enabled by default,
a second hack resets this value after the template is loaded.

Note: I can't say I'm especially proud of the code, but I didn't see a betterway to accomplish the goal and have it handle templates, command-line conversion, custom default style files, musescore.com, etc. Well, we could at least avoid the second hack to correct templates by somehow having a "template load mode" flag passed to the various read functions, or add a member to the Score itself that we could check. But I really didn't think it worth making that level of change for this.

@MarcSabatella
Copy link
Contributor Author

See also #6247 which isn't technically required for this change, but it meant to be an adjunct to it. That is, #6247 should definitely be merged so that everyone reading the same score hears the same thing by default. Then this PR can also be merged if we agree it is good for older scores to not sound different in 3.5 compared to whatever version they were created in. I am about 60% in agreement with that myself.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jun 25, 2020

Quite a few mtest failures in Travis, all due to the changed default it seems, the need to add <harmonyPlay>0</harmonyPlay> to several test scores, mainly in compat206 and compat114. But also in a few other places, seems there the detection of being pre 3.5 goes wrong? Seems the ref files are lacking the <programVersion>3.x.y</programVersion>
Actually no, it does detect them to be pre 3.5, that's why it needs that <harmonyPlay>0</harmonyPlay> apparently. Strange that it does need that even on scores that don't have any chord symbols or harmony channel?

@MarcSabatella
Copy link
Contributor Author

Since this solution is something of a hack to begin with, I'm inclined to hack my hack to skip the special processing in test mode. That should fix a good chunk of tests, ideally leaving just the compact tests, which I can fix manually.

But it will be a time consuming process, I can't guarantee when I will get to it. And I probably won't undertake it without a clear consensus that this is what we should do.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jun 25, 2020

Yes, disabling in test mode (which doesn't do any playback anyhow) seems a good idea.

@MarcSabatella
Copy link
Contributor Author

See https://musescore.org/en/node/307961 for some further discussion and strong arguments in favor of going ahead and doing this compatibility business. I remain torn. I really hate to give up chord symbol playback by default for the many, many older lead sheets out there, but a missed opportunity for improvement isn't as bad as actively harming playback of scores that are better off without it. I suspect there are way more of the former than the latter, so the overall cost/benefit might be a wash in some sort of semi-objective sense. Are thousands of very mildly disappointed users better than a handful of irate ones? I honestly don't know.

Anyhow, I have updated this PR to not do the magic hackery in test mode. I'm still too ambivalent to want to spend the time and effort actually updating tons of test references.

@anatoly-os anatoly-os removed this from the MuseScore 3.5 RC milestone Jul 21, 2020
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jul 21, 2020

Maybe postpone and revisit this for 3.5.1, depending on the feedback we get for 3.5 Release?
And then together with #5889, so enabling them is made easier?
Or even with an additional dialog "Do you want to enable chord symbols playback? Yes | No | Save reply and don't ask again"?

@MarcSabatella
Copy link
Contributor Author

Agreed. BTW, I have the mtests fixed, but the script tests apparently do not run in test mode, so they will need to be updated manually. Tempted to do it by updating the initial scores to 3.5.

This adds checks for the version that was used when a file was saved,
so we can force older scores (both pre-3.0 and 3.0 - 3.4.2)
to have the playHarmony style setting off by default,
for better playback compatibility with older versions.
Whether or not this happens is controlled by an advanced preference.
I also added an advanced preference to force the style off
when creating new scores.
It's basically the same thing as creating an MSS file
and setting that as your default style,
except this preference also overrides the style setting
when creating a score from a template.

The default for these preferences is off,
meaning, don't disable.
Thus, whatever is in your own default style or the templates rules,
*unless* you explicitly force the play harmony setting to be disabled.
@MarcSabatella
Copy link
Contributor Author

I have updated this PR to use a couple of advanced preferences. One to force chord symbol playback off for old scores (once created before 3.5), another to force it off for newly created scores. In both cases, the style option is simply forced off, so the setting gets saved in the score normally.

I have both of these defaulting to "off", meaning, we don't disable chord symbol playback. It's important to not that this doesn't mean chord symbol playback is actually forced on; it means we honor what's in the score, template, and/or default style.

I know there will still be those wishing we'd change the default so that old scores don't playback chord symbols by default. Despite the number of times this has come up now on the forum & issue tracker, I still think it's likely to be minority overall. It's just that it's also likely to be a more vocal minority - people don't generally posting unless they are not happy with something. But in any case, with my PR, changing the default becomes as simple as flipping the default on the preference.

@Jojo-Schmitz
Copy link
Contributor

No Ui for this, just an advanced preference?

@MarcSabatella
Copy link
Contributor Author

For now, yes.

@MarcSabatella MarcSabatella changed the title disable chord symbol playback by default for older scores preferences to disable chord symbol playback when opening old scores, or creating new ones Aug 9, 2020
@MarcSabatella
Copy link
Contributor Author

Based on continued discussion on the forum, I have changed the default for the first of the new preferences - the one that turns off the chord symbol playback style setting in older scores - to true. So, by default right in this PR right now, older (pre=3.5) scores will not play chord symbols by default, but new ones still will. The preferences still exist to change this behavior; all I did here in this new commit was switch the default. The tests are also updated to I should be able to easily toggle this back at any time.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Aug 13, 2020

Should also fix a potential issue on musescore.com, without the need of changing the default settings there.
I'm happy with this. In addition to #5889

@MarcSabatella
Copy link
Contributor Author

Note to self: templates. With the disableOld preference set (as it now would be by default), the affects scores created from old template. Probably shouldn't. Or, at least, we should update our own templates. Marking WIP for now. Also, since there seems to be consensus on forum this is right solution, I'll look at exposing the preferences better.

@MarcSabatella MarcSabatella added the work in progress not finished work or not addressed review label Aug 14, 2020
@MarcSabatella MarcSabatella removed the work in progress not finished work or not addressed review label Aug 17, 2020
@MarcSabatella
Copy link
Contributor Author

I think this is mergeable as is. It probably wouldn't hurt to move these preferences somewhere outside the "advanced" space, but it doesn't really seem crucial, and I haven't seen any sort of consensus on this. That could always be looked at later.

@Jojo-Schmitz
Copy link
Contributor

Moving these preferences somewhere outside the "advanced" space would be something to consider for 3.6, as it would requite new translatable strings.
But having this in its current state in 3.5.1 is vital! There were already complains on musescore.com, from people uploading a score created with 3.4.2 and a) getting caught by surprise that there chord symbols play back and b) not having the option to disable that without having to update to 3.5 first.

@Jojo-Schmitz
Copy link
Contributor

This problem comes up time and time again, we need this fixed in 3.5.1, and esp. on the musescore.com backend!

@MarcSabatella
Copy link
Contributor Author

Ping. Along with the repeat playback not working in continuous view, this seems one of the most important changes to get into 3.5.1, and the musescore.com backend. It would have been better to have had it in the nightly builds all along to get feedback on the solution, although realistically, if people were giving feedback on pre-releases more regularly, we never would have gotten into this situation to begin with :-)

@vpereverzev vpereverzev merged commit 136451e into musescore:3.x Oct 5, 2020
igorkorsukov added a commit to igorkorsukov/MuseScore that referenced this pull request Feb 12, 2021
…when opening old scores, or creating new ones
vpereverzev pushed a commit that referenced this pull request Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants