Skip to content

fix script connection object trigger method#1201

Merged
rryan merged 3 commits intomixxxdj:masterfrom
Be-ing:script_connection_object_trigger_method
Mar 1, 2017
Merged

fix script connection object trigger method#1201
rryan merged 3 commits intomixxxdj:masterfrom
Be-ing:script_connection_object_trigger_method

Conversation

@Be-ing
Copy link
Copy Markdown
Contributor

@Be-ing Be-ing commented Mar 1, 2017

follow up from #1198

Comment thread src/controllers/controllerengine.cpp Outdated
}

coScript->emitValueChanged();
QScriptValueList args;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To reduce duplication -- want to make this a helper function on ControllerEngineConnection and call it from here and ControlObjectScript?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's quite a bit of work to do on these classes and I don't really want to go down that rabbit hole for 2.1. I don't understand why ControllerEngineConnection is even a class. It seems like it should be a struct of ControllerEngineConnectionScriptValue.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea it could just as easily be a struct.

It should be a tiny change -- I don't want to leave an opportunity for someone to change one place but not the other.

@rryan
Copy link
Copy Markdown
Member

rryan commented Mar 1, 2017

Nice, thanks!

@rryan rryan merged commit d08ef75 into mixxxdj:master Mar 1, 2017
@Be-ing Be-ing deleted the script_connection_object_trigger_method branch March 2, 2017 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants