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

[MU3] Make AutoSize default for new scores but disable it for older scores. #6948

Merged
merged 1 commit into from
Nov 29, 2020

Conversation

njvdberg
Copy link
Contributor

@njvdberg njvdberg commented Nov 26, 2020

Resolves: https://trello.com/c/11WgLsL1/23-disabling-vertical-frame-enable-auto-size-is-not-saved-in-score-file

In the VBox::VBox() constructor, the variable _isAutoSizeEnabled was always set to true for score with a version number >= 3.02. However, the default for that property is false. So, when this option was switched of, the property has the default value and isn't written into the score file. Next, when reading the file, there no such property but if the score version was >= 3.02, the property was set to true again.

This is solved by setting the default of _isAutoSizeEnabled to true. But when reading an score < 3.02, this property in set to false after reading the properties in Box::read(). Also, Box::getProperty() will always return false for older scores.

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • I created the test (mtest, vtest, script test) to verify the changes I made

@njvdberg njvdberg changed the title Make AutoSize default for new scores but disables it for older scores. [MU3] Make AutoSize default for new scores but disables it for older scores. Nov 26, 2020
@RobFog
Copy link

RobFog commented Nov 26, 2020

"disables" should "disable"?

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Nov 26, 2020

One vtest diff, before:
mmrest-6-ref
after
mmrest-6-1
So a matter of title frame size I guess

@njvdberg njvdberg changed the title [MU3] Make AutoSize default for new scores but disables it for older scores. [MU3] Make AutoSize default for new scores but disable it for older scores. Nov 27, 2020
@Jojo-Schmitz
Copy link
Contributor

That vtest fail might be easily fixed by updating the scorefile it stems from I guess?

@njvdberg njvdberg force-pushed the SaveVerticalFrameSizeSwitch branch from e1454ae to 26a8d5a Compare November 27, 2020 09:16
@njvdberg
Copy link
Contributor Author

The vtest violation was caused by reading a 2.x file. In both read114.cpp and read206.cpp the auto resize had to be disabled explicitly.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Nov 27, 2020

Now, mtests fail (and macos uploading artifacts)

@njvdberg njvdberg force-pushed the SaveVerticalFrameSizeSwitch branch from 26a8d5a to d76e0ef Compare November 27, 2020 11:14
@njvdberg
Copy link
Contributor Author

Now, mtests fail (and macos uploading artifacts)

The mtest was my mistake, the files got a new tag.
But why uploading artifact fails every now and then puzzles me. It seems to something in the system, not in the sources.

@Jojo-Schmitz
Copy link
Contributor

yes, likely not your fault

@vpereverzev vpereverzev merged commit 5838228 into musescore:3.x Nov 29, 2020
igorkorsukov added a commit to igorkorsukov/MuseScore that referenced this pull request Feb 5, 2021
vpereverzev pushed a commit that referenced this pull request Feb 5, 2021
@njvdberg njvdberg deleted the SaveVerticalFrameSizeSwitch branch March 10, 2021 12:47
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.

4 participants