New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

migrate legacy controller system to QJSEngine #2682

Merged

Holzhaus merged 169 commits into mixxxdj:master from Be-ing:migrate-to-QJSEngine

Jun 14, 2020

Contributor

Be-ing commented Apr 19, 2020 •

edited by ronso0

Loading

This builds on @ferranpujolcamins's work to migrate to QJSEngine for ES7 support (#1795). This PR ports the Components library to use ES6 classes.

Note: Mixco mappings for Korg NanoKontrol2 and MAudio Xponent are removed, unmaintainable

ferranpujolcamins and others added 30 commits

November 24, 2018 13:28


          Replace QtScript classes with QtQml script classes

e2441bd

Don't bind "this" in beginTimer and makeConnection callbacks

Adapt ControllerEngine::checkException for QJSEngine

Rename checkException to handleEvaluationException


          Move controllerengine files into new engine subfolder

8e21856


          Expose Controller and ControllerEngine scripting apis through a proxy

3bf236a

Since QJSEngine takes ownership of exposed QObjects it tried to delete
controller and controllerengine when QJSEngine was destroyed (i.e script reload
and mixxx shutdown). See https://bugreports.qt.io/browse/QTBUG-41171

This also has the benefit that we have a tighter control on what's exposed.
Since Signals and slots, properties and children of object are available
as properties of the created QJSValue, lots of undesired things were leaked
into the JS engine.


          Add checks for JS engine evaluation errors

08dd110


          Remove unused evaluate slot in ControllerEngine

6ddae3d


          Refactor exceptions

9b3b28f


          Add nullengine exception handling

50f8059


          Fix tests build

2b14555


          Add controller script name to assertion message

a3475d8

This helps inspecting the test results in eclipse test inspector.


          Fix script error not displaying file name

9009fc9


          Remove comment

d7b6811


          Adapt hid devices to new JS Engine

6bd1f15

Remove bytearrayclass since now QJSEngine has built in support for
QByteArray, which gets converted to a JS ArrayBuffer.


          Add QtQml dependency to travis config file

4047bf8


          Update comments

5ed5280


          Remove QScriptEngineDebugger from SkinContext

b3380ba


          Components: use Function.prototype.bind for QJSEngine

The old hacks for preserving the "this" object for
engine.makeConnection, engine.connectControl, and
engine.beginTimer do not work with QJSEngine, but QJSEngine
brings support for ES5, which supports Function.prototype.bind.
https://bugs.launchpad.net/mixxx/+bug/1733666


          Xone K2: use Function.prototype.bind for QJSEngine

ed4b00c

The old	hacks for preserving the "this" object for
engine.makeConnection, engine.connectControl, and
engine.beginTimer do not work with QJSEngine, but QJSEngine
brings support for ES5,	which supports Function.prototype.bind.
https://bugs.launchpad.net/mixxx/+bug/1733666


          Hercules P32: use Function.prototype.bind for QJSEngine

ccbc73c

The old	hacks for preserving the "this" object for
engine.makeConnection, engine.connectControl, and
engine.beginTimer do not work with QJSEngine, but QJSEngine
brings support for ES5,	which supports Function.prototype.bind.
https://bugs.launchpad.net/mixxx/+bug/1733666


          Add comment to depends.py

eb06da4


          Merge branch 'master' into migrate-to-QJSEngine

1200db3


          Add missing file to depends.py

9143bb3


          Fix usage of deprecated parameter

7c65f81


          Replace checks with VERIFY_OR_DEBUG_ASSERT

c34427d


          Replace tabs with spaces

630c781


          Rename ControllerEngine member

da214be


          Replace tabs with spaces

4bee8ab


          Rename function in ControllerEngine

98818df


          Rename variable

86efb47


          Improve comments

b69b9b9


          Rename method in ControllerEngine

cf40beb


          Components: fix HotcueButton color output

6e96bdb

Contributor Author

Be-ing commented Jun 13, 2020

Thanks for debugging that. I committed the fix 6e96bdb. Ready for merge?

Contributor Author

Be-ing commented Jun 14, 2020

The Travis failure is from an unrelated test failure, probably spurious.

Holzhaus approved these changes

View reviewed changes

Member

Holzhaus left a comment

The diff is too large to review this properly, but I didn't encounter issues during manual testing.

@mixxxdj/developers Does anyone else want to take a look or can I hit merge?

Contributor Author

Be-ing commented Jun 14, 2020

@ferranpujolcamins and I have both gone over this code thoroughly. This has been in progress since August 2018. I really don't want to keep holding it back. We've already had hassles fixing merge conflicts over that time. Let's get this in already.

Holzhaus merged commit 441e49c into mixxxdj:master

Member

Holzhaus commented Jun 14, 2020

Alright, merged.

Be-ing deleted the migrate-to-QJSEngine branch

June 14, 2020 12:12

Contributor Author

Be-ing commented Jun 14, 2020

Woo, our first major step towards 2.4!

Member

ronso0 commented Jun 14, 2020

nice!

any news about the svg script?
back then, the reason to disable it was the implementation of QScriptDebugger. Is it now possible to re-enable it? Would still be handy for setting up a large number button icon templates with Variables, especially during the skin design process.

Contributor Author

Be-ing commented Jun 14, 2020

I don't know. I haven't looked into that at all. But we do still need to remove the deprecated Qt Script from the skin system before it is removed from Qt, which I suspect will happen with Qt6.

Member

ronso0 commented Jun 14, 2020

deprecated Qt Script from the skin system

from a quick look, it looks like it's not used at all since the svg extension is disabled.
So we need to migrate that to QJsEngine, as well? Would that suit to have rudimentary skin scripting? Like 'outsourcing' nested 'visible' connections for showing a certain Spinny for example?

Contributor Author

Be-ing commented Jun 14, 2020

I don't know, but if you want to experiment with that, go for it!

Member

Holzhaus commented Jun 14, 2020

I think QML has native JS support, right? Maybe we should switch over our custom skinning system to QML before adding more features.

Contributor Author

Be-ing commented Jun 14, 2020

Yes, but that's a much bigger project than what @ronso0 is proposing.

Member

ronso0 commented Jun 14, 2020

Yeah, looking forward to a first QML POC! But looking at the 2.4 ToDo/plans I doubt this will happen before 2.5!?

I'd start experimenting, but I don't want to add tons of new stuff.
The only goal isto find a simple way to include a skin script like latenight.script that does some basic toggling and therefore allows simplifying skin templates, because toggling widgets in/visibilbe causes delays and glitches IME.

Contributor Author

Be-ing commented Jun 15, 2020

I suspect we should wait until Qt6 before fully embracing QML.

Holzhaus mentioned this pull request

Prevent wrong usage of Control Objects with assertions #2896

Merged

Be-ing mentioned this pull request

Update Novation Launchpad controller scripts #2600

Merged

2 tasks

Be-ing mentioned this pull request

add support for loading controller scripts as JS modules #2868

Merged

4 tasks

Be-ing mentioned this pull request

ControllerManager: poll every 1ms on Linux #2966

Closed

Be-ing mentioned this pull request

Traktor s3, main branch #3074

Closed

Be-ing mentioned this pull request

ControllerEngine: do not use C++ assertions for JS issues #3650

Merged

mixxxbot mentioned this pull request

qt5: Replace deprecated QtScript with QJSEngine #8986

Closed

JoergAtGithub mentioned this pull request

Changelog for 2.4.0 #11542

Merged

ronso0 added controller mappings controller backend and removed controller mappings labels

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

controller backend controller mappings