monome / teletype

monome eurorack module
GNU General Public License v2.0
197 stars 83 forks source link

Each execution state needs an I variable #94

Closed burnsauce closed 6 years ago

burnsauce commented 6 years ago
1:
L 1 4: SCRIPT 2; A I
2:
L 5 8: B I

A -> 8
B -> 8

I solved this problem with the while loop by tracking per execution level in es_state, which made sense but was not ideal. I is in ss.variables. Does ss.variables.i[EXEC_DEPTH] offend anyone? If not, I can implement a fix that way. I'm amenable to some refactoring if someone sees an abstraction that works better.

samdoshi commented 6 years ago

Counterpoint (and untested on hardware):

1:
L 1 4: SCRIPT 2; A I

2:
I 100

A -> 100

I would expect A to be 100 after executing script 1. With the change you are suggesting it would be just 4 wouldn't it?

burnsauce commented 6 years ago

The nature of a for loop suggests that I should contain the iterator of the current loop. As it stands, calling a script with a loop from a loop doesn't work as expected. In other words, nested loops need their own iterators.

This is the corner case of L and SCRIPT. W and SCRIPT absolutely required per exec depth tracking. I think it makes sense for L, too.

On Tue, Sep 5, 2017, 02:52 Sam Doshi notifications@github.com wrote:

Counterpoint (and untested on hardware):

1: L 1 4: SCRIPT 2; A I

2: I 100

A -> 100

I would expect A to be 100 after executing script 1. With the change you are suggesting it would be just 4 wouldn't it?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/94#issuecomment-327087219, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJqMJqOTWgMuaebRh2HyYUDEei68JLsks5sfO-SgaJpZM4PMZ2v .

burnsauce commented 6 years ago

Come to think of it, won't IF and SCRIPT present the same corner case?

On Tue, Sep 5, 2017, 03:06 Poindexter Frink sliderule@gmail.com wrote:

The nature of a for loop suggests that I should contain the iterator of the current loop. As it stands, calling a script with a loop from a loop doesn't work as expected. In other words, nested loops need their own iterators.

This is the corner case of L and SCRIPT. W and SCRIPT absolutely required per exec depth tracking. I think it makes sense for L, too.

On Tue, Sep 5, 2017, 02:52 Sam Doshi notifications@github.com wrote:

Counterpoint (and untested on hardware):

1: L 1 4: SCRIPT 2; A I

2: I 100

A -> 100

I would expect A to be 100 after executing script 1. With the change you are suggesting it would be just 4 wouldn't it?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/94#issuecomment-327087219, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJqMJqOTWgMuaebRh2HyYUDEei68JLsks5sfO-SgaJpZM4PMZ2v .

samdoshi commented 6 years ago

I still disagree with regards to I, plus it will be a breaking change.

Also, can you explain the issue with IF and SCRIPT for me.

burnsauce commented 6 years ago

Not at the hardware, but I expect this to misbehave:

1: A 0 IF EZ A: SCRIPT 2 ELSE: A 1

2: IF NZ A: A 2

I think A will be 1.

On Tue, Sep 5, 2017, 03:19 Sam Doshi notifications@github.com wrote:

I still disagree with regards to I, plus it will be a breaking change.

Also, can you explain the issue with IF and SCRIPT for me.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/94#issuecomment-327092475, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJqMODCXX5CGMAheF47BOjvLw2KwhJlks5sfPYGgaJpZM4PMZ2v .

samdoshi commented 6 years ago

Gotcha.

I'm wondering if we should instead think about pushing and popping the execution state then? And track the execution depth via the height of the stack.

Or maybe keeping exec_state_t for the entire execution context, but having a member for things that need pushing and popping via SCRIPT:

e.g.

typedef struct {
    bool if_else_condition;
} nested_exec_depth_t;  // naming things is hard.

typedef struct {
    nested_exec_depth_t nested[EXEC_DEPTH];
    uint8_t exec_depth;
} exec_state_t;

Plus high level functions in state.c to push, pop, and obtain the depth.

burnsauce commented 6 years ago

I think a stack is appropriate. I'll bang it out tomorrow afternoon.

On Tue, Sep 5, 2017, 04:00 Sam Doshi notifications@github.com wrote:

Gotcha.

I'm wondering if we should instead think about pushing and popping the execution state then? And track the execution depth via the height of the stack.

Or maybe keeping exec_state_t for the entire execution context, but having a member for things that need pushing and popping via SCRIPT:

e.g.

typedef struct { bool if_else_condition; } nested_exec_depth_t; // naming things is hard. typedef struct { nested_exec_depth_t nested[EXEC_DEPTH]; uint8_t exec_depth; } exec_state_t;

Plus high level functions in state.c to push, pop, and obtain the depth.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/94#issuecomment-327101192, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJqMBJbHT9MV-AiibxRNJE5Aykzjxcsks5sfP-OgaJpZM4PMZ2v .

samdoshi commented 6 years ago

I think a stack is appropriate. I'll bang it out tomorrow afternoon.

Can you try and add some tests for it (if possible?). Feels like the kind of thing that someone could end up breaking in the future by accident without some safeguards.

burnsauce commented 6 years ago

No problem.

On Tue, Sep 5, 2017, 04:13 Sam Doshi notifications@github.com wrote:

I think a stack is appropriate. I'll bang it out tomorrow afternoon.

Can you try and add some tests for it (if possible?). Feels like the kind of thing that someone could end up breaking in the future by accident without some safeguards.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/94#issuecomment-327104233, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJqMBktN8KQ_EqMownSY8-rlf2o6R0zks5sfQK0gaJpZM4PMZ2v .

burnsauce commented 6 years ago

Working on a test rig now, but SCRIPT now behaves like this:

1:
SCRIPT 1
A + A 1

> A 0
> <F1>
> A => 9
> A 0
> SCRIPT 1
> A => 8

Notes:

Breaking behaviour on L: I is transient and associated with a script's execution. This means that it no longer serves as a general-purpose scene variable.

1:
A 0; B 0
L 1 4: SCRIPT 2; A + A I

2:
L 1 4: B + B I

> <F1>
> A => 10 (previously: 16)
> B => 40
burnsauce commented 6 years ago

I realize that I could design I to have multiple behaviours and be non-breaking.

Essentially, leave I in ss.variables, use internal, exec depth tracked I's in loop mode, then update ss.variables.i to be correct per the old behaviour when the loop is done.

The I operator would check if there is a loop running to return one or the other.

Would that be preferable?

tehn commented 6 years ago

for me this doesn't represent a huge breaking issue. i doubt that there are a lot of people that have a lot of scripts relying on calling SCRIPT within L using the I variable.

it makes sense to me to make the I var local to the loop. if someone wants to get I into a script, just mind the ordering:

1:
L 1 4: A I; SCRIPT 2

2:
B ADD B A

what's most important to me is predictable, intuitive behavior. if we have to explain a detail in the docs, that's fine.