fix #88861: allow mid-score staff name changes #4958
Conversation
|
Which kind of tests could verify the new behaviour and indicate that the behaviour has been changed? |
|
|
||
| score->undo(new ChangePart(part, new Instrument(instrument), newPartName)); | ||
| if (_tickStart == Fraction(-1, 1)) { |
There was a problem hiding this comment.
Which user scenario does run this branch of code?
There was a problem hiding this comment.
This should be hit if there are no instrument changes at all. We have made the distinction between tick -1 and tick 0 for this reason elsewhere as well.
| @@ -522,12 +524,21 @@ void ScoreView::elementPropertyAction(const QString& cmd, Element* e) | |||
| // editFretDiagram(static_cast<FretDiagram*>(e)); | |||
| else if (cmd == "staff-props") { | |||
| Fraction tick = {-1,1}; | |||
There was a problem hiding this comment.
In principle, we'd also hit that test for tick -1 if any elements other than chordrests, notes, measures, or instrument names are capable of displaying staff properties in their context menu.
|
Regarding tests, that's a good question, we don't really have a great story for code that is invoked in dialogs or popup menus. But the interesting thing about this PR is, it actually is just a change in UI behavior, the underlying code was already there. I guess code to test the underlying behavior could be added now that there is a supported way to invoke it. Previously, hand-editing the MSCX file was the only way to get that behavior. |
|
The following screenshots visualise the behaviour in 3.2.3. The issues are pretty visible if one looks at the instrument names:
There are different (and correct) results with your build: So, I suggest adding at least a visual test. |
|
I will do that, but again, I need to emphasize - my code did not cause this visual result. The underlying support was already there, so the vtest would produce exactly the same result with or without my change here. My code merely makes it easier to create such a score; currently hand-editing the mscx file would be required. Still, worth adding no doubt, so I'll have that later today. |
MarcSabatella
force-pushed
the
instrument-change-name
branch
from
af4053d
to
75437bf
Compare
|
Done. |
See https://musescore.org/en/node/88861, and a number of other impassioned requests over the years. Originally when 2.0 came out, we had an Instrument Change element that affected sound only, not transposition or other staff properties. I got that working for 2.1, but it was not easily possible given the internal architecture to change staff names as well. For 3.0, it finally became possible, and I added that capability - so if a flute changes to clarinet, the staff name changes from that point forward as well. But there is no way currently to customize the staff names associated with the change. So if you don't like "Bb Cl." as your staff name for the clarinet, too bad. So it's been long on my todo list to add a way tyo do this, we just didn't have any consensus on the best UI for it.
Meanwhile, Tantacrul's video provided an easy answer. He was having trouble figuring out how to do the instrument change to begin with, and tried Staff Properties and wondered why that didn't work. See https://musescore.org/en/handbook/developers-handbook/ux-design/design-reviews-and-responses/tantacrul-music-software#19%3A11_-_Instrument_change.
My PR here address both his concerns about how to change instruments as well as the concerns in the issue at hand about changing staff names along with it. With my change here, you still start by adding the instrument change element to your score, but then if you do Staff Properties after right-clicking a spot after the change, any changes you make in the "Part" section of the dialog affect the instrument change. So you can use the Change Instrument button here instead of the Change Instrument menu item exactly as Tantacrul expected, and more importantly, staff name (and other) changes are applied to that instrument change as well.
I should not that this is a UI change only - all the underlying support was already there. And I realzie there is also the future possibility of merging isntrument changes and staff type changes, which I totally support. But meanwhile, this simple PR adds a feature we currently lack - the ability to customzie staff names after an isntrument change - and addresses a concern Tantracrul spent a couple minutes on.