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 #291688, Fix: #285855 #5972

Merged
merged 1 commit into from
Apr 21, 2020
Merged

Conversation

njvdberg
Copy link
Contributor

Resolves: https://musescore.org/en/node/291688
Resolves: https://musescore.org/en/node/285855

In Excerpt::cloneStaves() clones TimeSig's and KeySig's if source staff 1 is not mapped to a track. To prevent copying of precaution segments only TimeSig's and KeySig's at the beginning of the measure are copied. This is done by getting the tick and clone the element when it is at the start of the measure only.
However, the test was using element->tick() instead of segment->rtick().

  • 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

Solves:
    #291688 - Time signature change doesn't appear in parts which don't have
    #285855 - Creating part from voice 2 does not respect key change

Used element->tick() instead of segment->rtick() to check whether the element
was at the beginning of the measure.
@MarcSabatella
Copy link
Contributor

This is crazy - seriously, all this time it was a one-line fix? I figured it would be relatively simple, but... wow.

I take it that https://musescore.org/en/node/283964 is not fixed by this, that seems the other critical issue with this feature. Having this working for 3.5 would be very good...

@njvdberg
Copy link
Contributor Author

Easy indeed :-). At this very moment I'm looking into #281666 - Parts Dialog Problem with Voices, this look pretty straight forward too.
Then I'll have a look at #283964 - Linked parts for single voices don't work. I don't it is already solved but will find out soon.

@RobFog
Copy link

RobFog commented Apr 21, 2020

/OCD The commit message and PR title could be made more consistent by removing the colon after the second "fix". Probably also makes sense to lower-case the second "fix".

@anatoly-os anatoly-os merged commit eeba202 into musescore:master Apr 21, 2020
@njvdberg njvdberg deleted the voice-parts branch April 23, 2020 11:42
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

5 participants