Skip to content

Commit 8fca8f7

Browse files
gcharpe1604gcharpe1604
committed
feat: refactor GlideBlock to remove noteCounter and fix double-execution side-effects
1 parent e3fddb8 commit 8fca8f7

3 files changed

Lines changed: 130 additions & 48 deletions

File tree

‎js/blocks/OrnamentBlocks.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,11 @@ function setupOrnamentBlocks(activity) {
421421
logo.notation.notationBeginSlur(turtle);
422422
}
423423

424-
tur.singer.glideOverride = Singer.noteCounter(logo, turtle, args[1]);
424+
// Signal the start of a glissando to the singer
425+
tur.singer.inGlide = true;
426+
tur.singer.glideDuration = 0;
427+
tur.singer.glideStartTime = tur.singer.turtleTime;
428+
tur.singer.glideBuffer = [];
425429

426430
const listenerName = "_glide_" + turtle;
427431
logo.setDispatchBlock(blk, turtle, listenerName);
@@ -431,6 +435,13 @@ function setupOrnamentBlocks(activity) {
431435
logo.notation.notationEndSlur(turtle);
432436
}
433437

438+
// Play the buffered glissando
439+
if (tur.singer.glideBuffer.length > 0) {
440+
Singer.playGlideBuffer(activity, turtle);
441+
}
442+
443+
// Signal the end of the glissando
444+
tur.singer.inGlide = false;
434445
tur.singer.glide.pop();
435446
};
436447

‎js/blocks/__tests__/OrnamentBlocks.test.js‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ describe("setupOrnamentBlocks", () => {
8080
setSlur: jest.fn(),
8181
setStaccato: jest.fn()
8282
},
83-
noteCounter: jest.fn((logo, turtle, val) => val)
83+
noteCounter: jest.fn((logo, turtle, val) => val),
84+
playGlideBuffer: jest.fn()
8485
};
8586

8687
activity = {
@@ -94,7 +95,14 @@ describe("setupOrnamentBlocks", () => {
9495
ithTurtle(turtle) {
9596
if (!this.turtleObjs[turtle]) {
9697
this.turtleObjs[turtle] = {
97-
singer: { staccato: [], glide: [], justCounting: [] }
98+
singer: {
99+
staccato: [],
100+
glide: [],
101+
justCounting: [],
102+
glideBuffer: [],
103+
glideDuration: 0,
104+
inGlide: false
105+
}
98106
};
99107
}
100108
return this.turtleObjs[turtle];
@@ -241,7 +249,8 @@ describe("setupOrnamentBlocks", () => {
241249
turtleObj.singer.glide = [];
242250
const result = glideBlock.flow([0.1, 8], logo, turtleIndex, "blkGlide");
243251
expect(turtleObj.singer.glide).toContain(0.1);
244-
expect(Singer.noteCounter).toHaveBeenCalledWith(logo, turtleIndex, 8);
252+
expect(turtleObj.singer.inGlide).toBe(true);
253+
expect(turtleObj.singer.glideBuffer).toEqual([]);
245254
expect(logo.notation.notationBeginSlur).toHaveBeenCalledWith(turtleIndex);
246255
expect(logo.setDispatchBlock).toHaveBeenCalledWith(
247256
"blkGlide",
@@ -259,11 +268,11 @@ describe("setupOrnamentBlocks", () => {
259268
expect(activity.errorMsg).toHaveBeenCalledWith(NOINPUTERRORMSG, "blkGlide");
260269
expect(result).toEqual([8, 1]);
261270
});
262-
it("should call notationEndSlur in __listener when justCounting is empty", () => {
271+
it("should call playGlideBuffer and notationEndSlur in __listener when justCounting is empty", () => {
263272
const glideBlock = createdBlocks["glide"];
264273
const turtleIndex = 0;
265274
const turtleObj = activity.turtles.ithTurtle(turtleIndex);
266-
turtleObj.singer.glide = []; // ← empty, flow() will push 0.1
275+
turtleObj.singer.glide = [];
267276
turtleObj.singer.justCounting = [];
268277

269278
let capturedListener = null;
@@ -273,17 +282,20 @@ describe("setupOrnamentBlocks", () => {
273282

274283
glideBlock.flow([0.1, 8], logo, turtleIndex, "blkGlide");
275284
expect(capturedListener).not.toBeNull();
285+
turtleObj.singer.glideBuffer = [{ note: 60 }];
276286
capturedListener({});
277287

288+
expect(Singer.playGlideBuffer).toHaveBeenCalledWith(activity, turtleIndex);
278289
expect(logo.notation.notationEndSlur).toHaveBeenCalledWith(turtleIndex);
279-
expect(turtleObj.singer.glide).toHaveLength(0); // pushed 0.1 then popped
290+
expect(turtleObj.singer.glide).toHaveLength(0);
291+
expect(turtleObj.singer.inGlide).toBe(false);
280292
});
281293

282294
it("should skip notationEndSlur in __listener when justCounting is non-empty", () => {
283295
const glideBlock = createdBlocks["glide"];
284296
const turtleIndex = 0;
285297
const turtleObj = activity.turtles.ithTurtle(turtleIndex);
286-
turtleObj.singer.glide = []; // ← empty, flow() will push 0.1
298+
turtleObj.singer.glide = [];
287299
turtleObj.singer.justCounting = [true];
288300

289301
let capturedListener = null;
@@ -296,7 +308,7 @@ describe("setupOrnamentBlocks", () => {
296308
capturedListener({});
297309

298310
expect(logo.notation.notationEndSlur).not.toHaveBeenCalled();
299-
expect(turtleObj.singer.glide).toHaveLength(0); // pushed 0.1 then popped
311+
expect(turtleObj.singer.glide).toHaveLength(0);
300312
});
301313
});
302314

‎js/turtle-singer.js‎

Lines changed: 98 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,10 @@ class Singer {
174174
this.staccato = [];
175175
this.glide = [];
176176
this.glideOverride = 0;
177+
this.inGlide = false;
178+
this.glideDuration = 0;
179+
this.glideBuffer = [];
180+
this.glideStartTime = 0;
177181
this.swing = [];
178182
this.swingTarget = [];
179183
this.swingCarryOver = 0;
@@ -1889,7 +1893,7 @@ class Singer {
18891893
}
18901894

18911895
// Duration is the duration of the note to be played. doWait sets the wait time for the turtle before the next block is executed
1892-
const duration = noteBeatValue;
1896+
const duration = tur.singer.inGlide ? 0 : noteBeatValue;
18931897
// For the outermost note (when nesting), calculate the time for the next note
18941898
if (duration > 0) {
18951899
tur.singer.previousTurtleTime = tur.singer.turtleTime;
@@ -1924,6 +1928,14 @@ class Singer {
19241928
}
19251929
}
19261930
}
1931+
1932+
if (tur.singer.inGlide) {
1933+
const waitSeconds = isOsc ? noteValue / 1000 : bpmFactor / noteValue;
1934+
tur.singer.turtleTime += waitSeconds;
1935+
if (!tur.singer.suppressOutput) {
1936+
tur.doWait(Math.max(waitSeconds - turtleLag, 0));
1937+
}
1938+
}
19271939
let forceSilence = false;
19281940
if (tur.singer.skipFactor > 1) {
19291941
if (tur.singer.skipIndex % tur.singer.skipFactor > 0) {
@@ -2331,18 +2343,49 @@ class Singer {
23312343
future
23322344
);
23332345
}
2334-
} else if (last(tur.singer.instrumentNames)) {
2346+
} else {
23352347
if (!tur.singer.suppressOutput) {
2336-
// If we are in a glide, use setNote after the first note
2337-
if (tur.singer.glide.length > 0) {
2348+
let instrument = last(tur.singer.instrumentNames);
2349+
let filtersToUse = filters;
2350+
if (!instrument) {
2351+
if (
2352+
tur.singer.voices.length > 0 &&
2353+
last(tur.singer.voices)
2354+
) {
2355+
instrument = last(tur.singer.voices);
2356+
filtersToUse = null;
2357+
} else {
2358+
instrument = DEFAULTVOICE;
2359+
filtersToUse = null;
2360+
}
2361+
}
2362+
2363+
if (tur.singer.inGlide) {
2364+
if (d === 0) {
2365+
tur.singer.glideDuration += noteBeatValue;
2366+
}
2367+
tur.singer.glideBuffer.push({
2368+
note: notes[d],
2369+
instrumentName: instrument,
2370+
paramsEffects: deepClone(paramsEffects),
2371+
filters: filtersToUse
2372+
? deepClone(filtersToUse)
2373+
: null,
2374+
future:
2375+
future +
2376+
(typeof Tone !== "undefined"
2377+
? Tone.now()
2378+
: 0)
2379+
});
2380+
} else if (tur.singer.glide.length > 0) {
23382381
if (tur.singer.glideOverride === 0) {
23392382
activity.logo.synth.trigger(
23402383
turtle,
23412384
notes[d],
23422385
beatValue,
2343-
last(tur.singer.instrumentNames),
2386+
instrument,
23442387
paramsEffects,
2345-
filters,
2388+
filtersToUse,
23462389
true,
23472390
future
23482391
);
@@ -2354,9 +2397,9 @@ class Singer {
23542397
turtle,
23552398
notes[d],
23562399
beatValueOverride,
2357-
last(tur.singer.instrumentNames),
2400+
instrument,
23582401
paramsEffects,
2359-
filters,
2402+
filtersToUse,
23602403
false,
23612404
future
23622405
);
@@ -2367,43 +2410,14 @@ class Singer {
23672410
turtle,
23682411
notes[d],
23692412
beatValue,
2370-
last(tur.singer.instrumentNames),
2413+
instrument,
23712414
paramsEffects,
2372-
filters,
2415+
filtersToUse,
23732416
false,
23742417
future
23752418
);
23762419
}
23772420
}
2378-
} else if (
2379-
tur.singer.voices.length > 0 &&
2380-
last(tur.singer.voices)
2381-
) {
2382-
if (!tur.singer.suppressOutput) {
2383-
activity.logo.synth.trigger(
2384-
turtle,
2385-
notes[d],
2386-
beatValue,
2387-
last(tur.singer.voices),
2388-
paramsEffects,
2389-
null,
2390-
false,
2391-
future
2392-
);
2393-
}
2394-
} else {
2395-
if (!tur.singer.suppressOutput) {
2396-
activity.logo.synth.trigger(
2397-
turtle,
2398-
notes[d],
2399-
beatValue,
2400-
DEFAULTVOICE,
2401-
paramsEffects,
2402-
null,
2403-
false,
2404-
future
2405-
);
2406-
}
24072421
}
24082422
}
24092423
}
@@ -2606,6 +2620,51 @@ class Singer {
26062620

26072621
activity.stageDirty = true;
26082622
}
2623+
2624+
/**
2625+
* Plays the buffered glissando notes.
2626+
*
2627+
* @static
2628+
* @param {Object} activity - Activity object.
2629+
* @param {Number} turtle - Turtle index.
2630+
*/
2631+
static playGlideBuffer(activity, turtle) {
2632+
const tur = activity.turtles.ithTurtle(turtle);
2633+
const buffer = tur.singer.glideBuffer;
2634+
if (buffer.length === 0) return;
2635+
2636+
const totalDur = tur.singer.glideDuration;
2637+
const bpmFactor =
2638+
TONEBPM / (tur.singer.bpm.length > 0 ? last(tur.singer.bpm) : Singer.masterBPM);
2639+
2640+
// Find portamento
2641+
const portamento = tur.singer.glide.length > 0 ? last(tur.singer.glide) : 0;
2642+
2643+
for (let i = 0; i < buffer.length; i++) {
2644+
const item = buffer[i];
2645+
const setNote = i > 0;
2646+
const duration = i === 0 ? totalDur : 0;
2647+
const future =
2648+
typeof Tone !== "undefined" ? Math.max(item.future - Tone.now(), 0) : 0;
2649+
2650+
// Apply portamento to paramsEffects
2651+
if (item.paramsEffects) {
2652+
item.paramsEffects.doPortamento = true;
2653+
item.paramsEffects.portamento = portamento;
2654+
}
2655+
2656+
activity.logo.synth.trigger(
2657+
turtle,
2658+
item.note,
2659+
duration > 0 ? bpmFactor / duration : 0,
2660+
item.instrumentName,
2661+
item.paramsEffects,
2662+
item.filters,
2663+
setNote,
2664+
future
2665+
);
2666+
}
2667+
}
26092668
}
26102669

26112670
// Maintain CommonJS compatibility for tests

0 commit comments

Comments
 (0)