`finishTrial()` should clear timeouts and the screen by default.

[jodeleeuw]

Discussed in https://github.com/jspsych/jsPsych/discussions/3241

^{Originally posted by Shaobin-Jiang February 29, 2024} I use the on_load event quite a lot to tweak with the default behavior of jsPsych and from time to time I would have to manually end the experiment. I recently encountered a bug, though, when I called finishTrial in on_load but accidentally forgot to set the trial_duration of an html-keyboard-response plugin to null, so that when I was half way in the next trial, the finishTrial from within the previous trial, having reached the time limit, was executed, causing the current trial to be ended, resulting in a break down of the entire experiment.

I located the bug quick enough, and to be frank, this was expected, as I only called finishTrial in my on_load and did not clear the timeout. The finishTrial function of its own accord does not include the latter; that is quite clear in the docs. However, this led me to wonder whether this is right. I mean, I have written quite a number of plugins myself and as far as trial duration is concerned, I would have to call clearAllTimeouts aside from finishTrial; oh, and besides these two I would have to clear the screen as well. And from what I see in the official plugins, these three tasks co-occur quite frequently as well. So why not just put them together in one function and let finishTrial also clear timeouts and the content of the screen?

I would propose something like this:

// clear all timeouts and the content of the screen by default
jsPsych.finishTrial(data_obj);

// only end the trial but do not clear timeouts or the screen
jsPsych.finishTrial(data_obj, { clear_timeout: false, clear_screen: false });

Quoting from this answer:

The finishTrial() method isn't really intended to be called outside the plugin context. You can do it, but it is likely to create problems.

I do not quite agree because using it properly never caused problems for me; the only problem that arose from using it was due to my not clearing the timeouts and the experiment content with it. And for that, I am much in favor of the idea of grouping the three tasks together.

[bjoluc]

Agreed. It was a bad pattern from early days of the library.

@jodeleeuw So do you think we should make clearing the screen and resetting the timeouts the default in v8, without opt-out parameters (I do)?

[jodeleeuw]

So do you think we should make clearing the screen and resetting the timeouts the default in v8, without opt-out parameters (I do)?

Yes, let's do it.

[jodeleeuw]

I've implemented this in #3339.

I decided not to create the arguments for clear_screen and clear_timeouts. I think it's best if plugins never leave anything on the display_element, and if a developer really wants a timeout to persist beyond the trial they can just use the built-in setTimeout instead of the pluginAPI version.