Skip to content

fix retrieval of "this" object in ControllerEngine::beginTimer#1196

Merged
daschuer merged 1 commit intomixxxdj:masterfrom
Be-ing:fix_this_beginTimer
Feb 28, 2017
Merged

fix retrieval of "this" object in ControllerEngine::beginTimer#1196
daschuer merged 1 commit intomixxxdj:masterfrom
Be-ing:fix_this_beginTimer

Conversation

@Be-ing
Copy link
Copy Markdown
Contributor

@Be-ing Be-ing commented Feb 27, 2017

This will facilitate using timers in Components, for example to implement alternate actions on double button presses or long presses.

@Be-ing Be-ing mentioned this pull request Feb 27, 2017
4 tasks
@daschuer
Copy link
Copy Markdown
Member

32bit windows build stops in the middle of an unrelated test. I assume it is a timeout.
LGTM

@daschuer daschuer merged commit 3c34566 into mixxxdj:master Feb 28, 2017
@Be-ing Be-ing deleted the fix_this_beginTimer branch February 28, 2017 21:05
@rryan
Copy link
Copy Markdown
Member

rryan commented Feb 28, 2017

I'm wondering whether this might have unintended consequences -- what if the function passed to beginTimer has a 'this' specifically bound to it via Function.prototype.bind by the programmer?

It's too bad we can't use ES6 arrow lambda (which would use 'this' from the scope it was created in) until QtScript supports ES6. I wonder whether it's better to suggest scripters use Function.prototype.bind to work around this instead of adding more 'this' hackery into the ControllerEngine itself.

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Feb 28, 2017

It's too bad we can't use ES6 arrow lambda (which would use 'this' from the scope it was created in) until QtScript supports ES6.

Yeah, last I checked no one was actively working on supporting ES6 in QtScript. :/

I wonder whether it's better to suggest scripters use Function.prototype.bind to work around this instead of adding more 'this' hackery into the ControllerEngine itself.

This PR only fixes the behavior as it was intended to work (but never did). I don't think script authors should have to do an extra step of binding the function to use this as expected, but I don't know how to how to detect if the script author has bound the function. Perhaps Mixxx could check if the function's this object is the same as that returned by getThisObjectInFunctionCall, and if it isn't, use the function's bound this object?

@rryan
Copy link
Copy Markdown
Member

rryan commented Feb 28, 2017

Yeah, last I checked no one was actively working on supporting ES6 in QtScript. :/

I know! Let's add Node and Babel as a Mixxx dependency and transpile the ES6 to ES5 on the fly :trollface:.

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Feb 28, 2017

I know! Let's add Node and Babel as a Mixxx dependency and transpile the ES6 to ES5 on the fly :trollface:.

Someone actually tried that... I haven't seen any activity from them since...

radusuciu added a commit to radusuciu/mixxx that referenced this pull request Mar 9, 2017
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.

3 participants