fix #282246: "other" appearance in time signature properties does not work #5315
Conversation
This bug occurs when the user chooses an "other" time signature property. This fix works by changing the implementation to be closer to MuseScore 2. This works by simply only checking to use the numbers if the numerator string is empty. This way, if the denominator string is empty, the numerator string which will have the custom symbol, will line up in the middle. This was how the implementation worked in MuseScore 2.
e7e30dc
to
cd3b382
Compare
|
It's better to create another branch and commit changes on it, so that later if you want to merge musescore/master to your fork's master, it won't cause any conflict. |
@Howard-C my mistake, I am still a bit new to git. I will do this in the future. Thanks! |
No problem. You can "drop" your own commit after this PR gets merged because the next time you sync your fork that commit will come back without any possible conflict. |
| ns = toTimeSigString(_numeratorString.isEmpty() ? QString::number(_sig.numerator()) : _numeratorString); | ||
| ds = toTimeSigString(_denominatorString.isEmpty() ? QString::number(_sig.denominator()) : _denominatorString); | ||
| if (_numeratorString.isEmpty()) { | ||
| ns = toTimeSigString(_numeratorString.isEmpty() ? QString::number(_sig.numerator()) : _numeratorString); |
There was a problem hiding this comment.
If _numeratorString is empty in the previous line, it is empty here too and this line could be simplified to:
ns = toTimeSigString( QString::number(_sig.numerator()) );
|
According to my tests, this PR also fixes issue #286478 (Create time signature only with nominator not possible). |
|
As far as I can test, this PR works correctly and does solve both #282246 and #286478 issues. |
|
Any reason for not merging this PR? Ver. 3.4 Beta still does not support numerator-only time signatures or mensural "time signatures", which were correctly supported by ver. 2.x |
| ds = toTimeSigString(_denominatorString.isEmpty() ? QString::number(_sig.denominator()) : _denominatorString); | ||
| if (_numeratorString.isEmpty()) { | ||
| ns = toTimeSigString(_numeratorString.isEmpty() ? QString::number(_sig.numerator()) : _numeratorString); | ||
| ds = toTimeSigString(_denominatorString.isEmpty() ? QString::number(_sig.denominator()) : _denominatorString); |
There was a problem hiding this comment.
I think this broke exist existing scores that had no text specified at all. Those should simply use the default numerator & denominator values, but here the denominator is left empty.
There was a problem hiding this comment.
Or, perhaps more commonly (not sure), it seems some scores have the numerator set but not denominator even though the user did not specifically intend that. This is the case for me, anyhow.
This commit fixes #282246. This fix works by changing the implementation closer to that of MuseScore 2. The issue is that "other" time signatures are created by changing the numerator string to the symbol, and the denominator string to empty, but during formatting, if the denominator is empty, it is replaced with the actual number of the time signature. The fix only replaces the denominator if the numerator and the denominator are empty. This way, the denominator will stay empty, and the formatting will be correct. This is how it was done in previous versions of MuseScore.