Only include su_delay_times into source when a delay unit is present.

[LeStahL]

No description provided.

[vsariola]

Did see this actually happening? Because I thought this line:

{{- if gt (.DelayTimes | len ) 0}}

would already make sure that it does not get included if there is no delay units, as I thought DelayTimes would be empty then?

[vsariola]

Let me guess what might be happening: the delay times get added to the delay table even when the unit disabled? There definitely is an oversight in this: I checked the code and I noticed the delaytime table construction does not care about the new "disabled" flag.

I think a more future proof approach would be to make a copy of the patch before compiling and just filter out all the disabled units, so when compiling, we can just assume all units are enabled. This way disabled units cannot have any effect on the resulting synth.

[LeStahL]

yes, this happened in a track with commented delay ops -

990,996d952
< ;-------------------------------------------------------------------------------
< ;    Delay times
< ;-------------------------------------------------------------------------------
< section .data.su_delay_times progbits alloc noexec write align=1
< su_delay_times:
<     dw 1116,1188,1276,1356,1422,1492,1556,1618,33075,689
< 

can try and reproduce with minimal example after evoke (we're some bytes short) tho if you think that we should rather tackle the underlying problem with the other if not being false while it should :)

[vsariola]

The issue starts from here:

sointu/vm/bytecode.go

Line 71 in 2667c3c

func NewBytecode(patch sointu.Patch, featureSet FeatureSet, bpm int) (*Bytecode, error) {

sointu/vm/bytecode.go

Line 215 in 2667c3c

func newBytecodeBuilder(patch sointu.Patch, bpm int) *bytecodeBuilder {

sointu/vm/bytecode.go

Line 223 in 2667c3c

delayTimesInt, delayIndices := constructDelayTimeTable(patch, bpm)

sointu/vm/delaytable.go

Line 110 in 2667c3c

func constructDelayTimeTable(patch sointu.Patch, bpm int) ([]int, [][]int) {

https://github.com/vsariola/sointu/blob/2667c3c72c95f7b91669d97d1435a1814cb97c01/vm/delaytable.go#L120C16-L120C17

Long story short: I think the "constructDelayTimeTable" does not check if your delay unit is disabled. It's a bug (or uh... feature). Is this your issue?

As an immediate quick fix, just delete the disabled delay unit(s) :)

[LeStahL]

Double-checked: Yes, this is exactly the code path that breaks, and removing the commented ops resolves it.

[vsariola]

Or, rather the proper "quick fix" for now, if you are can compile Sointu, is

if unit.Type == "delay" && !unit.Disabled {

On line 120 of delaytable.go

[vsariola]

What the hell I faster push that master than typing here :D Just a sec.

[LeStahL]

Thanks a lot! -- I assume we should close this PR, as it does not fix the issue but rather hides it.

[vsariola]

Fixed. Binaries should be soon available from Github Actions, as always.

[vsariola]

I thought of reopening this pull request, but I think I'll open a new issue instead: write unit tests for the disabled flag.