Task-play fails when using AppClock

[adcxyz]

Environment

• SuperCollider version: current develop branch
• Operating system: macOS

Steps to reproduce

// Routine works fine with AppClock:
Routine { "rout_appclock".postln }.play(AppClock); 

// Task fails: 
Task { "task_appclock".postln }.play(AppClock);
	// -> fails with schedAbs not understood, Receiver Meta_AppClock

Expected vs. actual behavior

Tasks should be fully playable with AppClock.
#5626 introduced clock.schedAbs in PauseStream:play,
which assumes that any clock understands schedAbs.
So maybe we need to add an AppClock:schedAbs method?

// full error dump
ERROR: Message 'schedAbs' not understood.
Perhaps you misspelled 'sched', or meant to call 'schedAbs' on another receiver?
RECEIVER:
class AppClock (0x11a993bc0) {
  instance variables [19]
    name : Symbol 'AppClock'
    nextclass : instance of Meta_ApplicationStart (0x11a3cff40, size=19, set=5)
    superclass : Symbol 'Clock'
    subclasses : nil
    methods : nil
    instVarNames : nil
    classVarNames : instance of SymbolArray (0x11a993d40, size=1, set=2)
    iprototype : nil
    cprototype : instance of Array (0x11a993e00, size=1, set=2)
    constNames : nil
    constValues : nil
    instanceFormat : Integer 0
    instanceFlags : Integer 0
    classIndex : Integer 42
    classFlags : Integer 0
    maxSubclassIndex : Integer 42
    filenameSymbol : Symbol '/Users/adc/src/_SC/supercollider/SCClassLibrary/Common/Core/Clock.sc'
    charPos : Integer 935
    classVarIndex : Integer 399
}
ARGS:
   Float 77.216567   3DE4CE32 40534DDC
Instance of Function {    (0x1229e5018, gc=94, fmt=00, flg=00, set=02)
  instance variables [2]
    def : instance of FunctionDef in Method PauseStream:play
    context : Frame (0x122d20378) of PauseStream:play
}
CALL STACK:
	DoesNotUnderstandError:reportError
		arg this = <instance of DoesNotUnderstandError>
	Nil:handleError
		arg this = nil
		arg error = <instance of DoesNotUnderstandError>
	Thread:handleError
		arg this = <instance of Thread>
		arg error = <instance of DoesNotUnderstandError>
	Object:throw
		arg this = <instance of DoesNotUnderstandError>
	Object:doesNotUnderstand
		arg this = <instance of Meta_AppClock>
		arg selector = 'schedAbs'
		arg args = [*2]
	PauseStream:play
		arg this = <instance of Task>
		arg argClock = <instance of Meta_AppClock>
		arg doReset = false
		arg quant = nil
		var tempClock = <instance of Meta_AppClock>
		var time = 77.216567491
	Interpreter:interpretPrintCmdLine
		arg this = <instance of Interpreter>
		var res = nil
		var func = <instance of Function>
		var code = "Task { "task_appclock".postl..."
		var doc = nil
		var ideClass = <instance of Meta_ScIDE>
	Process:interpretPrintCmdLine
		arg this = <instance of Main>
^^ ERROR: Message 'schedAbs' not understood.
Perhaps you misspelled 'sched', or meant to call 'schedAbs' on another receiver?
RECEIVER: AppClock

[adcxyz]

This seems to fix it, but I don't know if there are better solutions?

+ AppClock {
	*schedAbs { | time, item |
		scheduler.schedAbs(time, item);
	}
}

[adcxyz]

@jamshark70 @telephon, opinions?

[jamshark70]

#5626 introduced clock.schedAbs in PauseStream:play,
which assumes that any clock understands schedAbs.

I think it's absolutely wrong if there's a clock class that doesn't understand schedAbs -- a bit startled to see that AppClock doesn't have this.

I agree with the proposed fix.

[jamshark70]

One thing to note in the documentation is that AppClock scheduling is inexact:

(
var time = SystemClock.seconds + 1;
time.debug("schedAbs at");
SystemClock.schedAbs(time, {
	var wake = SystemClock.seconds;
	wake.debug("awoke");
	(wake - time).debug("drift");
	nil
});
)

schedAbs at: 533.926872446
-> SystemClock
awoke: 533.926872446
drift: 0.0

(
var time = SystemClock.seconds + 1;
time.debug("schedAbs at");
AppClock.schedAbs(time, {
	var wake = SystemClock.seconds;
	wake.debug("awoke");
	(wake - time).debug("drift");
	nil
});
)

schedAbs at: 548.512533949
-> AppClock
awoke: 548.513002882
drift: 0.00046893299997919

This may be one reason why JMc didn't add schedAbs to AppClock -- if user expectation is that the function should awake at exactly the absolute scheduling time, AppClock can't satisfy this.

But, I think it would be sufficient to make a note of this in the documentation. (Sooner or later, someone will complain "but I scheduled my thing for exactly beat 101 and it woke up at 101.00037472"...)

Also, are there any comprehensive unit tests for Routine and the different types of PauseStream vs all possible clock types? This is exactly the sort of thing that unit testing should have caught, but didn't.

EDIT: Also, AppClock *sched has the line this.prSchedNotify;. This appears to be an empty stub presently, but I suppose additional scheduling methods should not omit it anyway.

[adcxyz]

Hi @jamshark70,

would these test methods suffice ?

// in TestTask.sc
	test_play_with_SystemClock {
		var ok = false;
		var task = Task { 0.001.wait; ok = true; };
		task.play(SystemClock);
		0.002.wait;
		task.stop;
		this.assert(ok, "Task plays with SystemClock");
	}

	test_play_with_TempoClock {
		var ok = false;
		var task = Task { 0.001.wait; ok = true; };
		task.play(TempoClock.default);
		0.002.wait;
		task.stop;
		this.assert(ok, "Task plays with TempoClock.default");
	}

	test_play_with_AppClock {
		var ok = false;
		var task = Task { 0.001.wait; ok = true; };
		task.play(AppClock);
		0.002.wait;
		task.stop;
		this.assert(ok, "Task plays with AppClock");
	}

	test_play_with_LinkClock {
		var ok = false;
		var task = Task { 0.001.wait; ok = true; };
		task.play(LinkClock);
		0.002.wait;
		task.stop;
		this.assert(ok, "Task plays with LinkClock");
	}

[jamshark70]

would these test methods suffice ?

Pretty good. Maybe EventStreamPlayer tests would be useful too, perhaps under TestPattern? (Though maybe I'm just being over-cautious, perhaps drop that idea.)

An alternate form might be:

	test_play_with_SystemClock {
		var ok = false;
		var condition = CondVar.new;
		var task = Task { 0.001.wait; ok = true; condition.signalOne };
		task.play(SystemClock);
		condition.waitFor(0.002);
		this.assert(ok, "Task plays with SystemClock");
	}

If the test is successful, it isn't necessary to wait for the full timeout period.

LinkClock is actually an instance-based clock, like TempoClock -- so test_play_with_LinkClock is actually redirecting to TempoClock.default which is not a LinkClock by default. The LinkClock test should read:

	test_play_with_LinkClock {
		var ok = false;
		var condition = CondVar.new;
		var task = Task { 0.001.wait; ok = true; condition.signalOne };
		var clock = LinkClock.new;
		task.play(LinkClock);
		condition.waitFor(0.002);
		clock.stop;
		this.assert(ok, "Task plays with LinkClock");
	}

[dyfer]

I'm adding this to the 3.13.0 milestone since this is a regression from 3.12 and I think it would be good to fix that...
EDIT: linked PR added to the milestone instead

[telephon]

This, in AppClock, should be enough:

*schedAbs { arg time, item;
		scheduler.schedAbs(time, item);
		this.prSchedNotify;
	}