Fix GH#24998: cannot use shortcut (show-corrupted-measures)

[Jojo-Schmitz]

Resolves: #24998

[Jojo-Schmitz]

Anything still for me to do on this one?

[cbjeukendrup]

Not everything from #25011 (comment) has been addressed yet.

• Adding the "check for corruptions" command could be done in a follow-up PR
• perhaps decoupling corruption checking from autosave too
• but the mentioned optimisation would still be nice to make; it's not an optimisation for checking, but an optimisation for drawing. If we can see that the score has currently no known corrupted measures, by reading just one bool, that's a nice optimisation compared to always iterating over all measures on a page.
  Implementation: add a getter/setter hasCorruptedMeasures in Score, false by default; in the corruption checking method, reset it to false at the beginning, and as soon as any corrupted measure is found, set it to true; in the drawing code, read the value before undertaking any iteration through measures.

[Jojo-Schmitz]

Such an optimization would mean: if it has been detected to be corrupt once, there's no need to check again, unless that corruption got fixed meanwhile, whoch could have happened only if it got modified since the last check.
But if there were no corruptions on last check, it would need to get checked again, if the score got changed meanwhile, wouldn't it?
So if the score got changed, we need to check for corruptions (old ones that got fix, new ones that crept it), else we don't, And for that we could use the existing getter Score::dirty(), couldn't we?

Hmm, maybe not. Maybe I'm over-thinking this. Initialize a new m_corrupt to false, set to false right before checking in sanityCheckLocal(), set to true if any of the checks fails, check whether set in paintPageDebug() together with the check for showCorruptedMeasures, reset to ' false' on next check, should that succeed (rather than rechecking on next change).

That in turn seems to ask for another menu entry to trigger a recheck and with that fullfill your 1st point.

Your 2nd point, disabling the sanity check on autosave, seems quite easy to do too.

See what I've done now...

[Jojo-Schmitz]

Anything else I'd need to do on this?

[cbjeukendrup]

This would now only be available in dev mode, while it would be useful in normal mode too. Let's make the Diagnostics > Engraving menu available in normal mode too; in normal mode, it should only contain the show-corrupted-measures and check-sanity actions.

[Jojo-Schmitz]

I don't get this

[cbjeukendrup]

if (globalConfiguration()->devModeEnabled()) {
   // show all those submenus, including the full version of the Engraving submenu
} else {
    // show only an Engraving submenu, that contains only `show-corrupted-measures` and `check-for-score-corruptions
}

[cbjeukendrup]

Also, do we actually register a handle for the new check-sanity / check-for-score-corruptions action anywhere? It looks like it would currently be non-functional...

[Jojo-Schmitz]

You lost me, I'm affraid, register what, where, why?

[cbjeukendrup]

With the UiAction, we register the UI details of the action (name, icon, context, etc.). But what should happen when executing the action, is defined by registering a handler for it, for example in notationactioncontroller.cpp.

What you'd need to do concretely, is:

• In NotationActionController, create a new method checkForScoreCorruptions()
• At the bottom of NotationActionController::init(), add

      dispatcher()->reg(this, "check-for-score-corruptions", [this] { checkForScoreCorruptions(); });

This means, that whenever the check-for-score-corruptions action is triggered, the new checkForScoreCorruptions() method is called.

For implementing this method, you could call currentMasterNotation()->masterScore()->sanityCheck(); and then based on that, show an error dialog, based on ProjectActionsController::askIfUserAgreesToOpenCorruptedProject.

[Jojo-Schmitz]

Isn't that done already in

    // Register engraving debugging options actions
    for (auto& [code, member] : engravingDebuggingActions) {
        dispatcher()->reg(this, code, [this, member = member]() {
            EngravingDebuggingOptions options = engravingConfiguration()->debuggingOptions();
            options.*member = !(options.*member);
            engravingConfiguration()->setDebuggingOptions(options);
        });
    }

[cbjeukendrup]

For show-corrupted-measures yes, but for check-for-score-corruptions no (see also the definition of engravingDebuggingActions)

[Jojo-Schmitz]

As by that time the score is open already, there's no point in asking whether or not to open the score anyway.
Not even any OK/Cancel makes sense here.

But I guess you lost me completly here...

[cbjeukendrup]

Indeed, it would be only an OK button in this case.

So, the idea is, this new "Check for score corruptors" command would call sanityCheck, and if there are any corruptions, report those to the user, similarly to how they are reported when opening a corrupted file, except that of course any other button than OK and "Show details" wouldn't make sense here.

What we could additionally do, is showing an info dialog when the score is not corrupted.

[Jojo-Schmitz]

Like this?

[Jojo-Schmitz]

Rebased to resolve a merge conflict. Please review