Reset properties properly

[bakajikara]

Resolves: #15758
Resolves: #15765
Resolves: #15771
Resolves: #22835

Previously, property resets were done by overwriting with a specific default value, but now they are done correctly.

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

[its-not-nice]

@bakajikara This is great!

To nitpick: the fix for #15771 is not yet perfect as - to use the example described in the issue - the 'Reset' option is still shown as enabled if you select two items of different types (e.g. a note and a dynamic), even if they both have default offsets (and the option is therefore disabled when you select either of them individually).

Everything else seems good as far as I can tell!

[bakajikara]

@oktophonie
Thanks for pointing that out! I've just pushed a fix for it — should work correctly now.

[krasko78]

Hi @bakajikara! Great work! I think I am seeing a little problem here for something that is probably an edge case but still theoretically possible I guess: now that the loop is not exited as soon as isUndefined becomes true because we need to wait for isModified to become true as well, we risk setting isUndefined back to false in the subsequent iterations. I am imagining the following situation:

1. User selects: element E1 of type T1, element E2 of type T2 and element E3 of type T1 (same as first).
2. They all have a height property.
3. The default height for type T1 is 10 and the default height for type T2 is 20.
4. The height of E1 is 10. The height of E2 is 20. The height of E3 is also 10.
5. isUndefined should be true because the three heights are not the same but it will end up being false.

I suggest changing line 715 to something like:
if (!isUndefined && propertyValue != elementCurrentValue) {
isUndefined = true;
}

Also lines 717-729 could be optimized a bit to not do anything if isModified is already true.

P.S. I was able to reproduce it with a "p" dynamic marking, note and a descrescendo marking. "p" comes with a default vertical offset of 2, the note with a default vertical offset of 0 and the descrescendo with a default vertical offset of 1.75. Change the 1.75 to 2 so it matches the "p" (but not the note). Select first the "p", then the note and then the descrescendo. The X offset shows 0 and the Y offset shows 2. Both should show "--" instead.