Is there any documentation of the undo system in MuseScore 3?
Query for the MuseScore C++ developer community....
Is there any documentation of the undo system in MuseScore 3?
In particular I'm interested in how to ensure a plugin that changes to the state of the score are accounted for when using QML operations.
The driver for this question can be found here:
https://musescore.org/en/node/291414
-Dale
Comments
And what is meant by a "dock-type plugin" in the C++ code, which apparently has a bearing on it.
In reply to And what is meant by a "dock… by [DELETED] 1831606
Not really sure, but those that have an Ui and can get docked?
In reply to Not really sure, but those… by Jojo-Schmitz
The only plugin I found that uses it is random2.qml.
In its MuseScore declarations I see:
MuseScore {
version: "3.0"
description: "Create random score."
menuPath: "Plugins.random2"
requiresScore: false
pluginType: "dock"
dockArea: "left"
width: 150
height: 75
This suggests it's a UI related item.
I don't know why that would matter but it does.
From mscorePlugins.cpp (Line 460ish) function pluginTriggered():
In reply to The only plugin I found that… by DLLarson
Here's the Git Blame analysis for where this was introduced...
A long time ago.
In reply to Here's the Git Blame… by DLLarson
Maybe creating a trivial UI after this fashion (my intended uses would need a small UI dialog saying what change in offTime you wish to impose) would make the difference...
In reply to Maybe creating a trivial UI… by [DELETED] 1831606
As a test I commented out the dock qualifier in the C++ code so the startCmd and endCmd are always called and verified this in the debugger.
It made no difference in undo.
In reply to As a test I commented out… by DLLarson
Sounds like some breakpointing of endCmd and watching its behavior in, say, TempoChange.qml is in order...
In reply to Sounds like some… by [DELETED] 1831606
Is RandomChange.qml provably undoable?
In reply to Sounds like some… by [DELETED] 1831606
>Sounds like some breakpointing of endCmd and watching its behavior in, say, TempoChange.qml is in order...
That's precisely what I did. I followed the code from "start" through "end" . There isn't much to see really. It sets up a stack, calls the plugin, and tears it down. The problem seems to be that nothing adds to the stack related to state changes.
It's time for a knowledgeable developer to weigh in on this part of the code. It's just too fuzzy (yep I said it) and there is some important tribal knowledge missing here.
-Dale
In reply to >Sounds like some… by DLLarson
Is it possible that other attribute-modifying API availabilities are expressed (on the C++ side) with some kind of wrapper or interface function that does exactly that, records in that stack exactly what quantity in some inviolable way and how it changed?
In reply to Is it possible that other… by [DELETED] 1831606
From my brief visit to undo.cpp, it certainly seems as though that if you want to be undoable, you have to push undo records on the undo stack; there must be other stuff in the QML API that does that for other quantities. Are there any other qml-changeable note properties at all, now that I think about it? How does the PRE do it, separate stack or not?
In reply to From my brief visit to undo… by [DELETED] 1831606
I finally got it all working.
You can find my latest patch here:
https://github.com/DLLarson/MuseScore/tree/add-onofftimeoffsets
I will say, the small amount of code needed provide the new features does not reflect at all the time needed to arrive at that result. While the MuseScore C++ code is clean and well presented, it is almost devoid of any comments to help new programmers glean actual intention. It took me a lot of building and single stepping to figure this out.
We had an old saying that "The code doesn't lie!" While this is true, a few comments would have certainly have helped! It's almost as if comments were/are forbidden in the code. Rant off...
With regard to the implementation I wonder if we should replace the "offTimeOffset" value with a length value instead. Even in the Piano Roll UI you set length rather than note offTimeOffset. We could call it "onTimeLength" to be consistent with its associated "onTimeOffest".
It addition, it doesn't seem like 'onTimeLength' should ever be negative but the UI allows it. This seem wrong to me. It makes the note vanish in the piano roll editor.
Anyway...
I haven't submitted the patch to the upstream yet until we're happy with the results.
-Dale
In reply to I finally got it all working… by DLLarson
This is terrific. Haven't checked out (inspected) the code yet -- I will.
I think that the reverse is true, though --- and I'd already submitted a bug report -- the user-visible artifact should be off-time, not length; if you change the on-time, you should not have to change the length, right (but that's controversial).
I'll get to this soon, thanks again. I had already downloaded the tree and started studying the api system....
In reply to This is terrific. Haven't… by [DELETED] 1831606
>I think that the reverse is true, though -...
Ok... Glad I asked. I'm not a subject matter expert by any stretch.
Can off time ever be less than on time?
I would like the code to enforce value limits to prevent possible crashes.
-Dale
In reply to >I think that the reverse is… by DLLarson
"Length" can't be less than 0 (but in MS2 PRE it often got that way by bugs and the app would crash). Off-time can never be less than on time (or allowed to be set so). length = off_time - on_time. None of these can be negative. Requests to set either quantity so that this is not so should be rejected, actually. It shouldn't be possible to set bad values via QML.
In reply to "Length" can't be less than… by [DELETED] 1831606
Thanks for the additional info!
>It shouldn't be possible to set bad values via QML.
Yep, my point exactly!
In reply to Thanks for the additional… by DLLarson
Refuse any request to create 0 or negative length.
In reply to "Length" can't be less than… by [DELETED] 1831606
Can the off and on times be equal? i.e., zero length?
In reply to Can the off and on times be… by DLLarson
Also, is the top end to these numbers?
The default seems to be 1000.
In reply to Can the off and on times be… by DLLarson
No. A zero length note cannot be expressed in MIDI. MS has adequate UI to silence notes.
In reply to No. A zero length note… by [DELETED] 1831606
1000 is effectively "full" for all notes, "per mille ‰" vs %. All notes are born at 1000. More than 1000 is quite useful to force artificial legatos. I'd cap it at 2000, though.
In reply to 1000 is effectively "full"… by [DELETED] 1831606
How about I set the limit to 2 * NoteEvent::NOTE_LENGTH. NoteEvent::NOTE_LENGTH is set to 1000 in the code.
In reply to How about I set the limit to… by DLLarson
yeah, and check for all the zeros and negatives etc. Wish I could get a mac build. I'm edging closer to trying to build it myself ....
In reply to I finally got it all working… by DLLarson
It's almost as if comments were/are forbidden in the code.
I don't think Werner knows how to enter a comment on single line. 🤣
In reply to It's almost as if comments… by mike320
Seeing a few
//TODO-ws
in the code I'm sure this isn't the case ;-)In reply to Seenind quite a few //TODO… by Jojo-Schmitz
Ok... I have all the limit checks in the code, tested, and pushed up to my repository.
https://github.com/DLLarson/MuseScore/tree/add-onofftimeoffsets
It's ready to go if this looks copacetic to you.
One issue...I did notice that my builds against 'master' still have the "no plugins" problem. It's like the patch for 3.2.2 didn't make it into master?
I haven't tried building it for the 3.2.2 branch yet.
-Dale
In reply to Ok... I have all the limit… by DLLarson
Copacetic (word of tantalizing mysterious etymology). Now I see how it's done --- I have no knowledge apropos to the branch/version issue. You'd better settle that with whomever appropriate...
In reply to Copacetic (word of… by [DELETED] 1831606
> have no knowledge apropos to the branch/version issue.
I can handle that stuff.
I'm more interested if the feature meets your expectations as we've defined it.
Once I submit a pull request I'm sure it will get more attention from others prior to being accepted. Perhaps they even have nightly builds that can be tested.
Some additional feedback...
I rebuilt this new feature onto the current 3.2.2. branch and the plugin menus are working. This suggests that the 3.2.2 plugin fix did not make it into the master branch.
Executing the plugin test from the menu works well in 3.2.2. You can undo and redo the script changes in one shot.
I think it's ready to go!
-Dale
In reply to Ok... I have all the limit… by DLLarson
Master hasn't the plugin issue fixed yet
In reply to Master hasn't the plugin… by Jojo-Schmitz
Thanks for the confirmation!
-Dale
In reply to Thanks for the confirmation!… by DLLarson
You could revert 968ca5e for master to have the same "fix" as is in 3.2.2
Or pull https://github.com/musescore/MuseScore/pull/5185 for a real fix, that still has issues though, see #291584: Problem when reload the first plugin in interaction with a second plugin in Plugin Manager list and #291583: Switching between workspaces erases the enabled plugins
In reply to You could revert 968ca5e for… by Jojo-Schmitz
Will there be nightly builds that will allow me to test this on my Mac?
In reply to Will there be nightly builds… by [DELETED] 1831606
Once a PR got made and merged... So patience ;-) So far wa can only 'force' a nighly for Windows on a PR.
Or DIY B-)
In reply to Once a PR got made and… by Jojo-Schmitz
I assume the PR will be posted to this thread, so I'll know. I may yet DIM soon.... I downloaded the MS tree and it wasn't the expected gigabytes, but I need all that other stuff...(I read the how-to).
In reply to I assume the PR will be… by [DELETED] 1831606
Here's the pull request:
https://github.com/musescore/MuseScore/pull/5190
-Dale
In reply to I assume the PR will be… by [DELETED] 1831606
I assume the PR will be posted to this thread
If someone creates a suggestion issue, the PR can be linked to the the issue by calling it fix #1234 - change on/off times (or what ever is appropriate) and then git will update the issue automatically without needing to look at the PR to see its status.
In reply to I assume the PR will be… by [DELETED] 1831606
A concern has been posted in the pull request for this feature.
I've created a issue/suggestion to track more of this discussion and copied the concern into it.
We should probably continue here:
https://musescore.org/en/node/291708
-Dale
In reply to A concern has been posted in… by DLLarson
rather #291708: Can a plugin access on-time/off-time? :-)
In reply to You could revert 968ca5e for… by Jojo-Schmitz
I think I'll wait until the "powers" move it to master.
It's easy to checkout 3.2.2 and git cherry-pick my changes onto it.
In reply to I think I'll wait until the … by DLLarson
Well, yeah, if I want to make the build technology work on my system, which, based on all my professional experience, has no reason to work without pain.
In reply to Well, yeah, if I want to… by [DELETED] 1831606
It can be a big chore.
The instructions for Windows targets was pretty accurate and worked well for me. But I still had to install a lot of things to feed the beast.
Once it was set up it was simple to use. I just had to get there.
I have no idea how that all transfers to a Mac.
I'm equipped for Linux and Windoze only.