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

Fix #214996 - Display measure number range for multimeasure rests #3179

Closed
wants to merge 2 commits into from
Closed

Fix #214996 - Display measure number range for multimeasure rests #3179

wants to merge 2 commits into from

Conversation

emeraldimp
Copy link

@emeraldimp emeraldimp commented May 21, 2017

This pull request is to allow users to configure the display of measure number ranges for multimeasure rests. It is meant to be fairly flexible; if the multimeasure rest doesn't encompass (start on or extend over) a measure at the desired interval, then it will not show, but otherwise it will (if the setting is enabled).

I debated where to add the setting, whether to put it with the multimeasure rest settings in Score, or with the measure number settings in Header, Footer, Numbers. I eventually put it in Score.

Note that it does NOT handle the case where the measure numbers are altered within the MMR. I suspect it would be better to break the MMR in that case and print a new measure number.

Updated from #3175 to be based on master and follow spacing convention

@lasconic
Copy link
Contributor

Same remarks than for the other PR:

  • Could you open a feature request in the tracker ?https://musescore.org/en/project/issues/musescore
  • Could you reference the issue in the commit message "Fix #XXXX: Display measure number range for multimeasure rests" ?
  • Again I checked "behind bars" and I see no mention of displaying measure number range. Could attach some examples to the issue?

@emeraldimp emeraldimp changed the title Display measure number range for multimeasure rests Fix #214996 - Display measure number range for multimeasure rests Jun 3, 2017
@@ -119,6 +119,7 @@ static const StyleVal2 style114[] = {
{ StyleIdx::pageNumberOddEven, QVariant(true) },
{ StyleIdx::showMeasureNumber, QVariant(true) },
{ StyleIdx::showMeasureNumberOne, QVariant(false) },
{ StyleIdx::showMeasureNumberRange, QVariant(false) },
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Jun 3, 2017

Choose a reason for hiding this comment

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

Is this needed? 1.x files would never have this set.

@@ -149,6 +149,7 @@ struct StyleVal2 {
{ StyleIdx::pageNumberOddEven, QVariant(true) },
{ StyleIdx::showMeasureNumber, QVariant(true) },
{ StyleIdx::showMeasureNumberOne, QVariant(false) },
{ StyleIdx::showMeasureNumberRange, QVariant(false) },
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Jun 3, 2017

Choose a reason for hiding this comment

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

Is this needed? 2.x files most probably would not have this set, unless it can get added in a backward compatible way, i.e. in a way that 2.0, 2.0.1 etc. can read without any negative effect

if (smn)
if (smn && _mmRestCount && score()->styleB(StyleIdx::createMultiMeasureRests) && score()->styleB(StyleIdx::showMeasureNumberRange)) {
s = QString("%1 - %2").arg(no() + 1).arg(no() + _mmRestCount);
} else if (smn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather have an if (smn) { if (_mmRestCount && score()->styleB(StyleIdx::createMultiMeasureRests) && score()->styleB(StyleIdx::showMeasureNumberRange)) s = QString("%1 - %2").arg(no() + 1).arg(no() + _mmRestCount); else s = QString("%1").arg(no() + 1); }

@@ -799,6 +800,9 @@ void EditStyle::setValues()
radioKeySigNatNone->setChecked (lstyle.value(StyleIdx::keySigNaturals).toInt() == int(KeySigNatural::NONE));
radioKeySigNatBefore->setChecked(lstyle.value(StyleIdx::keySigNaturals).toInt() == int(KeySigNatural::BEFORE));
radioKeySigNatAfter->setChecked (lstyle.value(StyleIdx::keySigNaturals).toInt() == int(KeySigNatural::AFTER));

showMeasureNumberRange->setChecked(lstyle.value(StyleIdx::showMeasureNumberRange).toBool());

Copy link
Contributor

Choose a reason for hiding this comment

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

Why that extra empty line?

<item row="2" column="0">
<widget class="QCheckBox" name="showMeasureNumberRange">
<property name="text">
<string>Display Measure Number Range</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Sentence case here I think, Display measure number range

@anatoly-os
Copy link
Contributor

@emeraldimp hi! Are you still around?

@emeraldimp
Copy link
Author

@anatoly-os Sure am!

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jun 7, 2018

This PR needs a rebase.
Also that "Merge branch 'master' into multimeasureMeasureNumbersMaster" commit should not be in it.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jun 18, 2018

Quite a few mtests fail on Travis-CI, apparently with crashes, at least some of which due to failed assertions: ASSERT: "!strcmp(MStyle::valueType(idx),"int")" in file /home/travis/build/musescore/MuseScore/libmscore/score.h, line 820

@emeraldimp
Copy link
Author

Yes, I'm aware. I will let you know when it's ready.

@emeraldimp
Copy link
Author

It's clear I'm not going to finish this. I wrote this over a year ago, and it took that long to get any traction. It's destined for a version that will release... who knows when, but probably in a couple more years. It's not interesting, rewarding or fun to work on.

If anyone else is interested in this functionality, they're welcome to it; otherwise feel free to close.

@@ -154,7 +154,6 @@ struct StyleVal2 {
{ Sid::pageNumberOddEven, QVariant(true) },
{ Sid::showMeasureNumber, QVariant(true) },
{ Sid::showMeasureNumberOne, QVariant(false) },
{ Sid::measureNumberInterval, QVariant(5) },
Copy link
Contributor

Choose a reason for hiding this comment

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

I have the feeling this removal is causing the crashes in mtest

@anatoly-os
Copy link
Contributor

The request is not approved by feature request and related discussion. Taking into account the fact that Behind Bars doesn't have specific info about measure rests, I'm closing the PR.

@anatoly-os anatoly-os closed this Sep 9, 2018
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