fix #100791 - call fixTicks when Tempotext is being changed #2470
Conversation
|
I can't reproduce the bug and I don't see why this would fix it. Could you explain? |
|
I explained here : https://musescore.org/en/node/100791#comment-460731 |
|
See https://musescore.org/en/node/100791#comment-460786 for the specific steps to reproduce the bug. What's weird is I would have sworn we had a bug just like this fixed a year or so ago. |
|
Hmm, reported almost a year ago, but apparently never fixed: https://musescore.org/en/node/70366 |
|
The same fix fixes this bug too |
| @@ -190,6 +190,7 @@ void TempoText::textChanged() | |||
| } | |||
| } | |||
| } | |||
| score()->fixTicks(); | |||
There was a problem hiding this comment.
I guess this is not needed if the tempo is not actually changed right? So that should move just before score()->setPlaylistDirty();
Also what happens if the tempo is changed via the inspector (without followtext?), the bug will still be here I guess?
There was a problem hiding this comment.
I moved score()->fixTicks(); just before score()->setPlaylistDirty();.
To handle the case where inspector changes tempo text, adding the line:
if(val1!=val2) score->fixTicks();
just after the line:
https://github.com/musescore/MuseScore/blob/master/mscore/inspector/inspectorBase.cpp#L385 solves the problem.
But score->fixTicks(); should execute here, only in the case when 'this' is specifically of type Ms::InspectorTempoText and not just any Ms::InspectorBase. Can I use a dynamic_cast to check for this (like an instanceof check)? Its usually not thought of as good coding practice to use a dynamic_cast, if I am not wrong, but I don't see another way.
There was a problem hiding this comment.
No. We need a better place.
There was a problem hiding this comment.
Maybe here
MuseScore/libmscore/tempotext.cpp
Line 249 in be98902
These whole fermata handling need an overhaul...
There was a problem hiding this comment.
That fixed it. I see why it should be at that place. I'll push these two changes to the PR.
|
could you squash the commits? |
Change fix- tempo text and inspector changes Delete line
|
Squashed. |
No description provided.