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.
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
[audio-button-response] Fix undefined startTime #1194
Conversation
In the plugin `audio-button-response` when the audio is started and `context` is already defined a `startAudio` variable is used, but never defined, this causes e JS error: `Uncaught ReferenceError: startTime is not defined`. This commit fixes the issue by inlining the variable since this was not reused anywhere else.
@esolitos thanks very much for catching this! @jodeleeuw is this due to a jsPsych/plugins/jspsych-audio-slider-response.js Lines 234 to 238 in 005772d
And then startTime is compared to either performance.now or context.currentTime to get the RT:jsPsych/plugins/jspsych-audio-slider-response.js Lines 178 to 183 in 005772d
Whereas in this audio-button-response plugin, the performance.now timestamp is given a different variable name (start_time ), so it's not overwritten in the WebAudio case:jsPsych/plugins/jspsych-audio-button-response.js Lines 223 to 228 in 005772d
and the response time is always logged with a performance.now timestamp:jsPsych/plugins/jspsych-audio-button-response.js Lines 165 to 166 in 005772d
Should the audio-button-response plugin also collect an RT based on the WebAudio clock (when WebAudio is used)? |
Great catch @esolitos. @becky-gilbert I think you've identified the root cause. I'm not sure how the plugins ended up being different, but your solution seems absolutely correct here. |
@jodeleeuw thanks! |
Sure @becky-gilbert I can take care of that. :) |
@esolitos fantastic, thank you!! |
Hi @esolitos, just checking on the status of this PR. We're releasing v6.2 soon and want to include this change to the audio-button-response plugin. But I can make the changes if you don't have time to do it. Thanks again! |
hemm :D Sorry, has been a bit of a busy period. :)
I can send in those changes tonight (UTC+1 Tz).
…On Mon, 30 Nov 2020 at 22:29, Becky Gilbert ***@***.***> wrote:
Hi @esolitos <https://github.com/esolitos>, just checking on the status
of this PR. We're releasing v6.2 soon and want to include this change to
the audio-button-response plugin. But I can make the changes if you don't
have time to do it. Thanks again!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1194 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADCDG5AQMRZZRQQRVQWOJ3SSQFD7ANCNFSM4TWDHYAA>
.
|
Fixed this issue in commit 074aca7 so I'll close this pull request now. Thanks again for flagging this @esolitos! |
In the plugin
audio-button-response
when the audio is started andcontext
is already defined astartAudio
variable is used, but never defined, this causes e JS error:Uncaught ReferenceError: startTime is not defined
.This fixes the issue by inlining the variable since this was not reused anywhere else.