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

'Show stem slash' property for all grace notes #20890

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

XiaoMigros
Copy link
Contributor

@XiaoMigros XiaoMigros commented Jan 8, 2024

This PR introduces a property 'Show stem slash' for all grace notes. The slash is shown by default for acciaccaturas, and is toggleable for all grace notes on a per-beam basis.

image

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

@XiaoMigros XiaoMigros force-pushed the custom-stem-slash branch 4 times, most recently from 2e9405e to 3e20e58 Compare January 9, 2024 00:07
Copy link

@bkunda bkunda left a comment

Choose a reason for hiding this comment

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

What a great little feature!

Two problems that need resolving before merging:

  • Stem slashes are not populating to parts
  • The new "show stem slash" checkbox should be disabled when "stemless" is selected

Here is the test score I am using:
stem-slash-feature.mscz.zip

Thanks!

@XiaoMigros XiaoMigros force-pushed the custom-stem-slash branch 18 times, most recently from 70bebe8 to b2f89ae Compare January 21, 2024 20:12
@XiaoMigros
Copy link
Contributor Author

Point 2 has been addressed! For point 1, it seems that the property doesn't set for parts or linked staves, until the value is edited manually (works fine after that). I haven't yet been able to work out why...

@cbjeukendrup
Copy link
Contributor

Random guess: perhaps you need to add something in the copy constructor of Chord? (chord.cpp line 281)

@XiaoMigros
Copy link
Contributor Author

Ah, so that's what that method is for...

@XiaoMigros XiaoMigros force-pushed the custom-stem-slash branch 12 times, most recently from f223d8f to a8d57be Compare January 24, 2024 19:12
@XiaoMigros
Copy link
Contributor Author

XiaoMigros commented Jan 25, 2024

The changes are all accounted for @bkunda!

@cbjeukendrup
Copy link
Contributor

@XiaoMigros A rebase is needed due to recent changes in the MusicXML module

@mike-spa mike-spa merged commit 13c8098 into musescore:master Feb 5, 2024
11 checks passed
@XiaoMigros XiaoMigros deleted the custom-stem-slash branch February 5, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants