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 #294873: AUTO sharp/flat change to natural-sharp/natural-flat if there's double-sharp/double-flat before #5337

Closed
wants to merge 1 commit into from

Conversation

Harmoniker1
Copy link
Contributor

@Harmoniker1 Harmoniker1 commented Sep 20, 2019

Resolves: https://musescore.org/node/294873.

This fix is based on the following logic:

  • If the user put sharp/flat at one note, and put double-sharp/double-flat at a note before it, that sharp/flat automatically changes to natural-sharp/natural-flat. If the double-sharp/double-flat no longer exists, that natural-sharp/natural-flat reverts to sharp/flat;
  • If a natural-sharp/natural-flat is changed to regular sharp/flat by the user, that sharp/flat won't get changed later;
  • If the accidental should normally be sharp/flat but the user put natural-sharp/natural-flat intendedly, then that natural-sharp/natural-flat won't get changed later;
  • If a note has regular sharp/flat (natural-sharp/natural-flat are not affected) but a note in front of it is added natural-sharp/natural-flat, that sharp/flat is removed.

The most probable cause of #279179 and #293984 is isMicrotonal(), which treats natural-sharp/natural-flat/sharp-sharp wrongly as microtonal. PR #5341 fixes these two, but it still treats those three as microtonal. In order to implement #294873 this has to be changed.

@Harmoniker1 Harmoniker1 changed the title [WIP] fix #279179, fix #293984: natural accidental [WIP] fix #279179, fix #293984: natural-sharp/natural-flat Sep 20, 2019
@Harmoniker1 Harmoniker1 force-pushed the accidental branch 4 times, most recently from ec3b08c to 76c87b5 Compare September 20, 2019 13:16
@mattmcclinch
Copy link
Contributor

Another problem with this is that if a note has an implied (non-written) sharp/flat, and the user puts a double-sharp/double-flat at a note before it, then the accidental is displayed as a sharp/flat at first, but then on the next layout it is changed to natural-sharp/natural-flat.

I am not sold on the idea of having natural-sharps and natural-flats appear automatically. Especially if it means that they do not display correctly if you try to add them manually, but I know that is something that you are trying to fix. Is it really necessary to do this in order to resolve the related issues?

@Harmoniker1
Copy link
Contributor Author

Harmoniker1 commented Sep 20, 2019

Is it really necessary to do this in order to resolve the related issues?

No, but I recall someone in the forum asking this and I think it's best to implement this while fixing the issues, if I can, ultimately.

@Harmoniker1
Copy link
Contributor Author

Another problem with this is that if a note has an implied (non-written) sharp/flat, and the user puts a double-sharp/double-flat at a note before it, then the accidental is displayed as a sharp/flat at first, but then on the next layout it is changed to natural-sharp/natural-flat.

Fixed this ;-)

@mattmcclinch
Copy link
Contributor

I have an alternate solution to these two issues that does not involve automatically creating natural-sharps and natural-flats, and it does not suffer from your problem. It sort of depends on Accidental::isMicrotonal() not being changed, though, since your problem seems to be related to that particular change. My solution can be found here: mattmcclinch@c6d21d9.

@Harmoniker1
Copy link
Contributor Author

Harmoniker1 commented Sep 21, 2019

Thanks!
But apart from that I think what really blocks me is this function: changeAccidental2(), I cannot understand its effect on the change of accidentals, because it merely deals with the pitch, and everytime I change something about it the functionality of changeAccidental() alters...
For example if I don't execute this while placing a natural-sharp/flat, it turns to be natural rather than sharp/flat; if I put it in a different place then placing accidentals (except from natural-sharp/flat) is completely impossible, they won't come out on the score... I'm so confused!

@Harmoniker1
Copy link
Contributor Author

Harmoniker1 commented Sep 21, 2019

It doesn't seem to be changeAccidental2(), because if I move this before the action of undoAddElement() the result is still (wrongly) the same.
I tracked all the way down to undoAddElement() itself and the accidentalType is still correct, if input a natural-flat then it's indeed NATURAL_FLAT, I wonder what can go wrong after that...

@mattmcclinch
Copy link
Contributor

I thought you would be able to figure it out from what I showed you. Here it is again, but this time it is rebased on top of your changes: mattmcclinch@37bae0b.

@mattmcclinch
Copy link
Contributor

Oh, sorry, I just realized that mattmcclinch@37bae0b breaks some of the functionality that you added regarding automatically removing certain natural-sharps and natural-flats that were added automatically. But I think that a clue to solving your problem is still there.

@Harmoniker1
Copy link
Contributor Author

Harmoniker1 commented Sep 21, 2019

@mattmcclinch
It kind of works but I don't understand, you set accVal to SHARP or FLAT and the parameters of setAccidentalVal() don't have anything related to natural-sharp/flat, why can a natural-sharp/flat be attached?
And this is basically the same as not changing isMicrotonal() regarding the logic of not letting a natural-sharp/flat go into the if, but else instead. I doubt if a complete fix can be made based on your revision because we set totally different commands for natural-sharp/flat, mine: score()->undoAdd/RemoveElement() but yours: as->setAccidentalVal(), and yours doesn't even involve acci, which comes back to thing I said in the first paragraph ;-)

@mattmcclinch
Copy link
Contributor

I believe that mattmcclinch@c6d21d9 is a complete fix for both issues, is it not? You are trying to accomplish something else based on a forum comment that you did not link to, and as far as I know, does not have a corresponding issue in the tracker.

@Harmoniker1
Copy link
Contributor Author

@mattmcclinch
See https://musescore.org/en/node/279179#comment-895726, it suggests even more than I said.
And I still don't understand why your fix fixes it.

@Harmoniker1
Copy link
Contributor Author

Congratulations to myself! A complete fix completed!

@Harmoniker1 Harmoniker1 changed the title [WIP] fix #279179, fix #293984: natural-sharp/natural-flat fix #279179, fix #293984: natural-sharp/natural-flat Sep 22, 2019
@mattmcclinch
Copy link
Contributor

I found another problem with this. With your changes, SHARP_SHARP is automatically converted to SHARP2. That is to say, if you try to add the sharp-sharp symbol to a note, it will display as double-sharp.

@Harmoniker1
Copy link
Contributor Author

I found another problem with this. With your changes, SHARP_SHARP is automatically converted to SHARP2. That is to say, if you try to add the sharp-sharp symbol to a note, it will display as double-sharp.

Oops, fixed.

@Harmoniker1 Harmoniker1 changed the title fix #279179, fix #293984: natural-sharp/natural-flat fix #294873: AUTO sharp/flat change to natural-sharp/natural-flat if there's double-sharp/double-flat before Sep 25, 2019
@Harmoniker1 Harmoniker1 force-pushed the accidental branch 2 times, most recently from 45f2310 to 8e97caa Compare September 25, 2019 05:56
…there's double-sharp/double-flat before

Resolves: https://musescore.org/node/294873.

This fix is based on the following logic:
- If the user put sharp/flat at one note, and put double-sharp/double-flat at a note before it, that sharp/flat automatically changes to natural-sharp/natural-flat. If the double-sharp/double-flat no longer exists, that natural-sharp/natural-flat reverts to sharp/flat;
- If a natural-sharp/natural-flat is changed to regular sharp/flat by the user, that sharp/flat won't get changed later;
- If the accidental should normally be sharp/flat but the user put natural-sharp/natural-flat intendedly, then that natural-sharp/natural-flat won't get changed later;
- If a note has regular sharp/flat (natural-sharp/natural-flat are not affected) but a note in front of it is added natural-sharp/natural-flat, that sharp/flat is removed.

The most probable cause of #279179 and #293984 is isMicrotonal(), which treats natural-sharp/natural-flat/sharp-sharp wrongly as microtonal. PR musescore#5341 fixes these two, but it still treats those three as microtonal. In order to implement #294873 this has to be changed.
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.

2 participants