Code review comment for lp:~mixxxdevelopers/mixxx/features_direct_bind

Thanks! -- here are my comments sofar:

* ControllerEngine::callFunctionOnObjects should check isFunction() before call()'ing the QScriptValue

* ControllerEngine::resolveFunction should check isFunction() in addition to isValid()

* In ControllerEngine::internalExecute(QScriptValue, QScriptValue) return false if !isFunction() to match execute(QScriptValue, QScriptValueList)

* In ControllerEngine you changed a bunch of debug outputs to say MidiScriptEngine instead of ControllerEngine -- please change those back :)

* In connectControl() in the first if(disconnect) path you set cb.key = key which seems redundant with a few lines above.

* ControllerEngineConnectionScriptValue::disconnect() shadows QObject::disconnect(). We probably shouldn't do that.

* connectControl() connects the valueChanged() signal from the ControlObject, not the ControlObjectThread which causes priority-inversion between the Engine and the Qt main-thread. This was a bug that was fixed in the control-system rewrite so please re-add that.

* MidiController::bindScriptFunction() calling preset() and casting the result -- that's for use by the superclass which doesn't know about MidiControllerPresets. You can just use m_preset. (though see my comment below)

* I think the m_scriptBindings cache in Controller/MidiController isn't really necessary. Instead of the cache living in MixxxControl/Controller and having to pre-cache every function we think we might want to call, why not put the cache in ControllerEngine::resolveFunction. Change ControllerEngine::resolveFunction(QString) to take a boolean shouldCache flag that tells it to first look in the QScriptValue cache and then just call resolveFunction() every time we want to call a method. I think this would be a lot cleaner and this branch would become almost totally contained to ControllerEngine and it would be just as fast as what is written now (and a little more robust to cache misses since it will cache on first use).

* thanks for the tests :)