Fix #24922, 26612, 29611: Refactor key signatures

[XiaoMigros]

Resolves: #24922
Resolves: #26612
Resolves: #29611

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

[mike-spa]

@XiaoMigros left a few comments. Please also rebase if possible

[mike-spa]

This doesn't look right to me, or I'm not understanding it. It looks like you're delegating the properties of this key signature to the "initial" key signature that established the current key... why? Pid::IS_COURTESY looks especially strange, cause being a courtesy is certainly a property of the individual key sig, not a general one

[XiaoMigros]

The principal reason for this is that when switching from a non-C key to C, you can't modify Pid::IS_COURTESY (or any other property) of the new C key because only the generated courtesy key signature on the previous system has a clickable hitbox.

I also found allowing property delegation for key signatures also solves some other minor bugs (such as the properties window still showing clickable options for generated keysigs, but not doing anything), but I'll admit I'm not particularly happy with this solution (especially since we have to go looking for the initial key signature in this fashion.

[mike-spa]

Ah, got it thanks. Then if this delegation is meant to only apply from the courtesy onto the corresponding "main" signature, I'd suggest to be more explicit about it, for instance:

• check if the current keySig is a courtesy (if it isn't, no need to delegate)
• if you're on a courtesy keySig segment, you'll find the corresponding main keySig just a few segments ahead, you can probably do with a single s->next1(SegmentType::KEY_SIG) call, no need to use tick2Segment which is quite expensive

[mike-spa]

Final couple of comments

[XiaoMigros]

SEGV in the VTests, not sure if that's due to the property delegation changes or the rebase. Will investigate...

[mike-spa]

@XiaoMigros I've pushed the crash fix directly to your branch to speed things up. I'm now going to add some quick UI elements that we've agreed with our design team to expose your new option for naturals, then we'll merge :)

[XiaoMigros]

Thank you :))

[bkunda]

Looks good from my end! Approved.