Fix #291708 Plugin: Expose NoteEvent's via QML Note interface. #5224

DLLarson · 2019-07-15T01:05:06Z

Adds capablity to examine and modify PlayEvent's.
PlayEvent's are wrappers for an internal object called a NoteEvent.
The name PlayEvent better reflects what it's role is and what
you are really affecting from the QML plugin. Any changes made
with this feature also creates undo frames so they can be removed from
the user interface.

DLLarson · 2019-07-15T01:36:37Z

'nix build died on:
In file included from /home/travis/build/musescore/MuseScore/mscore/plugin/api/elements.h:28:0,
from /home/travis/build/musescore/MuseScore/mscore/plugin/api/qmlpluginapi.cpp:15:
/home/travis/build/musescore/MuseScore/mscore/plugin/api/noteevent.h: In constructor ‘Ms::PluginAPI::NoteEvent::NoteEvent(Ms::NoteEvent*, QObject*)’:
/home/travis/build/musescore/MuseScore/mscore/plugin/api/noteevent.h:47:20: warning: ‘Ms::PluginAPI::NoteEvent::ne’ will be initialized after [-Wreorder]
Ms::NoteEvent* ne;

But it works fine on Windows builds. Any ideas way?

-Dale

Jojo-Schmitz · 2019-07-15T08:11:59Z

ther mtests don't build, see https://travis-ci.org/musescore/MuseScore/jobs/558741689#L4012-L4034

DLLarson · 2019-07-15T13:32:59Z

I see the error but how do I correct?

This code is building and running fine in Windows using vs2017 plus Qt 5.12.2.

What system is the failed build targeting?

-Dale

dmitrio95 · 2019-07-15T19:33:59Z

I see the error but how do I correct?

It looks like tests do not build, you probably need to add the new files to CMakeLists.txt used in mtest:

MuseScore/mtest/CMakeLists.txt

Line 110 in cc654b1

${PROJECT_SOURCE_DIR}/mscore/plugin/api/util.cpp

DLLarson · 2019-07-15T21:19:30Z

Thanks for the tip!
It now finishes without issue.
-Dale

mscore/plugin/api/elements.h

Jojo-Schmitz · 2019-07-18T17:08:03Z

It doesn't build on Travis CI, looks like you forgot some #include? Probably note.h?

DLLarson · 2019-07-18T17:40:26Z

Of course it builds fine in VS2017!

This is one of those compiler difference things that drive me nuts.

The template is declared but not instantiated at the point of failure but only uses pointers yet VS2017 is happy and Linux/gcc is not? Ugggg!

playevent.h needs a definition from elements.h but elements.h also needs playevent.h. Normally just forward defining the empty class and just using pointers is enough.
Templates seem to have other rules.

I'm at a loss. I guess I need to replicate the linux build somehow to investigate this.

-Dale

mattmcclinch · 2019-07-18T22:10:24Z

I can get this to build by making these changes: mattmcclinch@5cc9957. My solution was to add a function that takes an Ms::PluginAPI::Note* as a parameter, and returns an Ms::Note*, since class QmlPlayEventsListAccess cannot access Ms::PluginApi::Note::note() directly.

DLLarson · 2019-07-18T22:38:32Z

Matt,

Thanks for that!

I'll be looking at it right away!

Meanwhile, I'll share my discovery...

I put together a VMware system with Ubuntu 18.04 all the setup as specified on the MuseScore developer handbook.

I cloned the code, built and I got the same errors... BUT WAIT A MINUTE... those aren't errors. They are warnings. I took the next step to 'make install' and it's running flawlessly on my Ubuntu VM. I loaded the same test score and test plugin I've been using on Windows and it works perfectly.

That makes me feel much better. I'm definitely going to check out Matt's mods to see if it removes the whole situation. Fingers crossed.

The errors posted on Travis-ci may be due to toolchain version. The failed Travis build used:

Ubuntu 14.04--not even described on the developers site as a valid target build system might add.
g++ = 4.8.4.

Yet my system is:

Ubuntu 18.04
g++ 7.4.0 - a BIG difference in toolchains!

With the newer toolchains the error appears to be acceptable as a warning.

Ohhhhh the wonderful world of tools! :)

-Dale

DLLarson · 2019-07-19T00:51:10Z

Excellent! All is right with the build.

Thanks Matt for the skilled eyes on the code!

-Dale

mattmcclinch · 2019-07-19T15:29:25Z

I am honored that you accepted my changes verbatim, but honestly, I wasn't completely happy with them. Basically, I wanted something a little more general. This is what I have now, which I like a lot better: mattmcclinch@a9efab9.

DLLarson · 2019-07-19T16:12:25Z

am honored that you accepted my changes verbatim, but honestly, I wasn't completely happy with them.

I was happy to stop beating my head against a wall plus it worked!

I'll check out the improved version. Right now I'm spinning in circles trying to expose the PlayEventType enum through a Chord property. When things don't work it can be real confusion.

mattmcclinch · 2019-07-19T16:43:57Z

Right now I'm spinning in circles trying to expose the PlayEventType enum through a Chord property.

Does this not work? mattmcclinch@29a2883

DLLarson · 2019-07-19T18:12:34Z

I got it all working.

I did have what you suggested but it still denied my playEventType r/w property existence when I tried.

THEN I fixed this improper definition that the compiler and tools didn't complain about:

  DECLARE_API_ENUM( Accidental,       accidentalTypeEnum      )

That "Accidental" should be "AccidentalType". This also explained why the AccidentalType enum wasn't available in QML.

 DECLARE_API_ENUM( AccidentalType,       accidentalTypeEnum      )

Anyway, I'm running a local build with your latest tweak so I'm close to sending up the changes.

Oh, also with the new changes I can reset the user playevent and get back the automatic ornamentations. i.e., I can recover that score I hosed!

-Dale

Jojo-Schmitz · 2019-07-21T07:56:45Z

Why then still the WIP label in PR and commit title?

mattmcclinch · 2019-07-21T12:26:23Z

Don't miss this comment, which isn't showing up because the conversation has been marked as resolved. The part about the forward declaration of class Note is especially important, since although it does not affect whether or not the build succeeds, I'm sure that it does affect whether or not certain objects get cast to the right type.

DLLarson · 2019-07-21T13:37:35Z

The part about the forward declaration of class Note is especially important, since although it does not affect whether or not the build succeeds, I'm sure that it does affect whether or not certain objects get cast to the right type.

To be clear, when using reinterpret_cast<>() you are telling the compiler that you know better than it about the cast (it's the nuclear option of casts.) So we better be right--which we are here.

DLLarson · 2019-07-21T13:41:04Z

Why then still the WIP label in PR and commit title?

Fixed.

libmscore/types.h

mscore/plugin/api/elements.h

mscore/plugin/api/qmlpluginapi.h

libmscore/undo.cpp

mscore/plugin/api/cursor.cpp

Jojo-Schmitz · 2019-07-25T07:23:13Z

gui-selection-list-test-v2-uses-cursor.qml

@@ -0,0 +1,55 @@
+import QtQuick 2.0


this and other testing files don't seem to belong into this PR, but at least and for sure not into the top level directory

This was stated in the commit. It will be removed as well as the other commits.
-Dale

Oops, reading helps ;-)

It was the only way I could get an application shared to test good and bad at once. :)

dmitrio95

Also while we are at it, could you please add fix #291708 to the commit message? Something like
fix #291708: Expose PlayEvent's via QML Plugin Note interface

libmscore/rendermidi.cpp

dmitrio95 · 2019-07-26T07:24:44Z

mscore/plugin/api/elements.cpp

+void Chord::setPlayEventType(Ms::PlayEventType v)
+      {
+      chord()->score()->setPlaylistDirty();
+      chord()->score()->undo(new ChangeChordPlayEventType(chord(), v));


This function could potentially check whether the chord already has a correct play event type, then it could avoid pushing extra unnecessary commands to undo stack. That is not strictly necessary but could optimize this operation a bit.

But isn't it possible that, even though the playEventType hasn't changed, the playEventLists may have? Woudn't those changes would be lost to a missing redo operation?

Sorry, maybe I didn't understand the question properly. This function has nothing to do with changing note events at all, if a plugin needs to change play events it would modify the Note.playEvents property which would also change the play event type to User.

Ok. I understand. Check is added.

-Dale

mscore/plugin/api/playevent.h

dmitrio95 · 2019-07-29T18:21:05Z

It looks like merging #5243 caused some merge conflicts to appear so rebase is needed for this PR. The conflicts seem to be trivial though.

Adds capablity to examine and modify PlayEvent's and a Note's playEvents list. Adds access to Chord.playEventType. Also supports UNDO for these operations.

DLLarson · 2019-07-29T18:45:09Z

I was expecting the rebase on this one. Just pushed up the corrections.
-Dale

dmitrio95 · 2019-07-29T18:58:28Z

I tested this patch a bit, looks really impressive! I see now why BSG is so encouraged with these new possibilities :)

I don't catch any issues anymore so I'll merge this PR when all tests get passed.

DLLarson · 2019-07-29T19:28:12Z

I tested this patch a bit, looks really impressive! I see now why BSG is so encouraged with these new possibilities :)

He does seem dang near giddy with the prospects.

His triller.qml plugin posted on the associated issue is really pretty cool! It makes use of the ability to create PlayEvent lists in Javascript to embellish the notes.

This is automation as it should be!

The gui selection stuff was needed to make it easy so that adder was really necessary.

-Dale

DLLarson mentioned this pull request Jul 15, 2019

Fix# 291708 Plugin: Add Note.onTimeOffset and Note.offTimeOffset properties #5190

Closed

DLLarson force-pushed the qml-expose-noteevents branch 2 times, most recently from 4180c84 to ce17411 Compare July 15, 2019 04:19

DLLarson force-pushed the qml-expose-noteevents branch 2 times, most recently from 393d899 to 174368d Compare July 15, 2019 17:44

DLLarson force-pushed the qml-expose-noteevents branch from 174368d to e59ccfd Compare July 15, 2019 20:31

DLLarson force-pushed the qml-expose-noteevents branch 2 times, most recently from 6b106dc to dcc19fd Compare July 18, 2019 16:41

Jojo-Schmitz reviewed Jul 18, 2019

View reviewed changes

mscore/plugin/api/elements.h Outdated Show resolved Hide resolved

Jojo-Schmitz reviewed Jul 18, 2019

View reviewed changes

mscore/plugin/api/elements.h Outdated Show resolved Hide resolved

DLLarson force-pushed the qml-expose-noteevents branch 2 times, most recently from fd5b5bc to bc253ae Compare July 18, 2019 19:05

DLLarson force-pushed the qml-expose-noteevents branch from bc253ae to 20a93e1 Compare July 18, 2019 23:22

DLLarson force-pushed the qml-expose-noteevents branch from 20a93e1 to 0e75e6e Compare July 19, 2019 00:58

DLLarson force-pushed the qml-expose-noteevents branch 2 times, most recently from bb79ae2 to d15e70e Compare July 20, 2019 03:14

DLLarson force-pushed the qml-expose-noteevents branch from a034254 to 686a68f Compare July 21, 2019 13:32

DLLarson changed the title ~~Fix# 291708 [WIP] Plugin: Expose NoteEvent's via QML Note interface.~~ Fix# 291708 Plugin: Expose NoteEvent's via QML Note interface. Jul 21, 2019

DLLarson force-pushed the qml-expose-noteevents branch from 686a68f to ee70380 Compare July 21, 2019 13:40

DLLarson force-pushed the qml-expose-noteevents branch 2 times, most recently from 693d08e to 5fcb2af Compare July 23, 2019 22:07

dmitrio95 reviewed Jul 23, 2019

View reviewed changes

DLLarson force-pushed the qml-expose-noteevents branch 2 times, most recently from a29858c to 459804a Compare July 24, 2019 15:25

Jojo-Schmitz reviewed Jul 25, 2019

View reviewed changes

DLLarson force-pushed the qml-expose-noteevents branch from 3ec0d45 to 6844e45 Compare July 25, 2019 16:23

dmitrio95 reviewed Jul 26, 2019

View reviewed changes

dmitrio95 changed the title ~~Fix# 291708 Plugin: Expose NoteEvent's via QML Note interface.~~ Fix #291708 Plugin: Expose NoteEvent's via QML Note interface. Jul 26, 2019

dmitrio95 mentioned this pull request Jul 26, 2019

Fix #291708 Plugin: Expose Score.selection object to access to GUI selected elements. #5243

Merged

DLLarson force-pushed the qml-expose-noteevents branch 2 times, most recently from 6334905 to f731b10 Compare July 27, 2019 14:47

Fix #291708: Expose PlayEvent's via QML Plugin Note interface.

b05c823

Adds capablity to examine and modify PlayEvent's and a Note's playEvents list. Adds access to Chord.playEventType. Also supports UNDO for these operations.

DLLarson force-pushed the qml-expose-noteevents branch from f731b10 to b05c823 Compare July 29, 2019 18:37

dmitrio95 merged commit c25a17e into musescore:master Jul 29, 2019

dmitrio95 mentioned this pull request Jul 29, 2019

Fix #291790: Restore Chord.remove() and Chord.add() methods #5209

Merged

DLLarson deleted the qml-expose-noteevents branch July 30, 2019 13:30

Jojo-Schmitz mentioned this pull request Aug 1, 2019

fix compiler warnings reg. comparing signed and unsigned and some unused parameters #5250

Merged

MarcSabatella mentioned this pull request Aug 17, 2019

(Based on #5221) Accessibility fixes. Part 4 #5270

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #291708 Plugin: Expose NoteEvent's via QML Note interface. #5224

Fix #291708 Plugin: Expose NoteEvent's via QML Note interface. #5224

DLLarson commented Jul 15, 2019 •

edited

Loading

DLLarson commented Jul 15, 2019

Jojo-Schmitz commented Jul 15, 2019

DLLarson commented Jul 15, 2019

dmitrio95 commented Jul 15, 2019

DLLarson commented Jul 15, 2019

Jojo-Schmitz commented Jul 18, 2019 •

edited

Loading

DLLarson commented Jul 18, 2019 •

edited

Loading

mattmcclinch commented Jul 18, 2019

DLLarson commented Jul 18, 2019

DLLarson commented Jul 19, 2019

mattmcclinch commented Jul 19, 2019

DLLarson commented Jul 19, 2019

mattmcclinch commented Jul 19, 2019

DLLarson commented Jul 19, 2019 •

edited

Loading

Jojo-Schmitz commented Jul 21, 2019 •

edited

Loading

mattmcclinch commented Jul 21, 2019

DLLarson commented Jul 21, 2019 •

edited

Loading

DLLarson commented Jul 21, 2019

Jojo-Schmitz Jul 25, 2019 •

edited

Loading

DLLarson Jul 25, 2019

Jojo-Schmitz Jul 25, 2019

DLLarson Jul 25, 2019

dmitrio95 left a comment

dmitrio95 Jul 26, 2019

DLLarson Jul 26, 2019

dmitrio95 Jul 27, 2019

DLLarson Jul 27, 2019

dmitrio95 commented Jul 29, 2019

DLLarson commented Jul 29, 2019

dmitrio95 commented Jul 29, 2019

DLLarson commented Jul 29, 2019

Fix #291708 Plugin: Expose NoteEvent's via QML Note interface. #5224

Fix #291708 Plugin: Expose NoteEvent's via QML Note interface. #5224

Conversation

DLLarson commented Jul 15, 2019 • edited Loading

DLLarson commented Jul 15, 2019

Jojo-Schmitz commented Jul 15, 2019

DLLarson commented Jul 15, 2019

dmitrio95 commented Jul 15, 2019

DLLarson commented Jul 15, 2019

Jojo-Schmitz commented Jul 18, 2019 • edited Loading

DLLarson commented Jul 18, 2019 • edited Loading

mattmcclinch commented Jul 18, 2019

DLLarson commented Jul 18, 2019

DLLarson commented Jul 19, 2019

mattmcclinch commented Jul 19, 2019

DLLarson commented Jul 19, 2019

mattmcclinch commented Jul 19, 2019

DLLarson commented Jul 19, 2019 • edited Loading

Jojo-Schmitz commented Jul 21, 2019 • edited Loading

mattmcclinch commented Jul 21, 2019

DLLarson commented Jul 21, 2019 • edited Loading

DLLarson commented Jul 21, 2019

Jojo-Schmitz Jul 25, 2019 • edited Loading

Choose a reason for hiding this comment

DLLarson Jul 25, 2019

Choose a reason for hiding this comment

Jojo-Schmitz Jul 25, 2019

Choose a reason for hiding this comment

DLLarson Jul 25, 2019

Choose a reason for hiding this comment

dmitrio95 left a comment

Choose a reason for hiding this comment

dmitrio95 Jul 26, 2019

Choose a reason for hiding this comment

DLLarson Jul 26, 2019

Choose a reason for hiding this comment

dmitrio95 Jul 27, 2019

Choose a reason for hiding this comment

DLLarson Jul 27, 2019

Choose a reason for hiding this comment

dmitrio95 commented Jul 29, 2019

DLLarson commented Jul 29, 2019

dmitrio95 commented Jul 29, 2019

DLLarson commented Jul 29, 2019

DLLarson commented Jul 15, 2019 •

edited

Loading

Jojo-Schmitz commented Jul 18, 2019 •

edited

Loading

DLLarson commented Jul 18, 2019 •

edited

Loading

DLLarson commented Jul 19, 2019 •

edited

Loading

Jojo-Schmitz commented Jul 21, 2019 •

edited

Loading

DLLarson commented Jul 21, 2019 •

edited

Loading

Jojo-Schmitz Jul 25, 2019 •

edited

Loading