Investigate bitwise logic in note check

[KilledByAPixel]

I am trying to figure out why we wrote this like this...

https://github.com/keithclark/ZzFXM/blob/cb07fa9ca36aefd67a0c8c656d2958b62f8ed9fe/zzfxm.js#L84

Specifically the part | note | 0

I think should just be || note. From my testing that seems to work the same. Only if a note value between 0 and 1 does it cause a glitch doing it the old way.

[keithclark]

Hey Frank!

That line could benefit from a nice big comment block explaining what's happening.

note is a number representing two properties - the integer part contains the note to be played and the decimal part is the attenuation. Both are optional. (On reflection, note is not the best name for this. It should be named something like cmd or data.)

So...

• if note is 0 (or falsy): Don't change anything. Any note being played in this channel will continue until the instrument sample has finished.
• if note is -1: Stop playing the current note immediately.
• if note is >=1: Start a new note and apply attentuation if decimal part is present.
• if note is >0 and <1: Change the attenuation of whatever is playing. Don't start playing a new note.

See the README for details.

With that in mind, if we assume the result of the following statement from the note stop logic is falsy:

i == patternChannel.length + isSequenceEnd - 1 && isSequenceEnd || instrument != (patternChannel[0] || 0)

The original stop test becomes:

stop = false | note | 0;

And the suggested change becomes:

stop = false || note; 

If we pump various values from the note list above into those we get:

Value for note	Action	bitwise method	logical method
0	none	0 (falsy)	0 (falsy)
-1	stop	-1 (truthy)	-1 (truthy)
1	play new note (C-1)	1 (truthy)	1 (truthy)
1.5	play new note (C-1) with 50% attentaion	1 (truthy)	1.5 (truthy) :warning:
0.25	change attenuation to 25%	0 (falsy)	0.5 (truthy) :warning:

The only differences are the results for decimal inputs, which is expected as we've lost the bitwise operator. The side-effect here is that it's now not possible to use attenuation on a note that's currently playing (>0 to <1). Instead, changing attenuation will now cause a playing note to stop.

Try this test song with your change. I suspect you'll hear a brief single note and then silence rather than volume changes:

[[[,0,400,,2]],[[[,-1,1,.1,.2,.3,.4,.5,.6,.7,.6,.5,.4,.5,.6,.7,.8,.9,-1]]],[0],,{"title":"New Song","instruments":["Instrument 0"],"patterns":["Pattern 0"]}]

[keithclark]

We might be able to drop the trailing | 0 though... 🤔

[KilledByAPixel]

Ah got it, thanks so much for the writeup! I had forgot about the 0-1 attenuation rule.

So the reason I was trying to get this figured out is that I set up LittleJS to use deepscan to check the code and that was the only issue it had with the zzfxm stuff.

For littlejs I will change it to || note | 0 instead of just | note | 0. That seems to be equivalent in my testing and will prevent the warning from showing up.