Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🪲 For-list command stops when placed in an if_pressed statement #5681

Closed
boryanagoncharenko opened this issue Jul 30, 2024 · 6 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@boryanagoncharenko
Copy link
Collaborator

Describe the bug
In level 10, the following code should output all elements of the list if the key x is pressed. However, the for loop outputs the first element of the array and then the program stops:

lijstje is kip, haan, kuiken
if x is pressed
    for dier in lijstje
        print dier
else
    print 'onbekend dier'

Add a screenshot (optional)
Screenshot 2024-07-30 at 13 13 35

@boryanagoncharenko boryanagoncharenko added the bug Something isn't working label Jul 30, 2024
@boryanagoncharenko boryanagoncharenko self-assigned this Jul 30, 2024
@boryanagoncharenko
Copy link
Collaborator Author

The issue seems to be that Skulpt stops the program when a time.sleep() is encountered (and there is one added at the end of the for-loop body). This seems to have been an issue before skulpt/skulpt#764 but it should have been already addressed. Not sure if the issue persists in our fork.

We could:

  • Try syncing with the latest version of Skulpt and/or attempting to fix the matter there.
  • Just remove all time.sleep statements from the body, even though that might lead to some confusion in the output.

@Felienne
Copy link
Member

The issue seems to be that Skulpt stops the program when a time.sleep() is encountered (and there is one added at the end of the for-loop body). This seems to have been an issue before skulpt/skulpt#764 but it should have been already addressed. Not sure if the issue persists in our fork.

We could:

  • Try syncing with the latest version of Skulpt and/or attempting to fix the matter there.

I am ok with trying that but we should totally check with @jpelay whether/how that would impact us since we did a lot of work on top of Skulpt with the debugger.

  • Just remove all time.sleep statements from the body, even though that might lead to some confusion in the output.

Do you mean only in the if_pressed or in general? It is potentially a bit confusing but if it solves this problem, I am ok with it (how often, in practice are kids going to put a loop within an if statement) But if you mean in general, that would be a no for me because the small wait is very helpful!

@boryanagoncharenko
Copy link
Collaborator Author

I am ok with trying that but we should totally check with @jpelay whether/how that would impact us since we did a lot of work on top of Skulpt with the debugger.

Actually, I just checked that syncing the fork would not resolve the issue. The problem still exists in Skulpt and can be reproduced with the following programs:

import turtle

t = turtle.Turtle()

for c in ['red', 'green', 'yellow', 'blue']:
    t.color(c)
    t.forward(75)
    t.left(90)
    time.sleep(1)
    

and

step = 1
while step < 4:
    step = step + 1
    print('test')
    time.sleep(1)

Do you mean only in the if_pressed or in general? It is potentially a bit confusing but if it solves this problem, I am ok with it (how often, in practice are kids going to put a loop within an if statement) But if you mean in general, that would be a no for me because the small wait is very helpful!

Yes, I meant removing the time.sleep only for commands placed in an if-pressed statement. Ideally, I'd find a way to keep the delay because it is handy, but I agree this might be an overkill for now.

@jpelay
Copy link
Member

jpelay commented Aug 29, 2024

Actually, I just checked that syncing the fork would not resolve the issue. The problem still exists in Skulpt and can be reproduced with the following programs:

Yeah, that's what I was going to say. Our version of Skulpts dates late 2023, and this issue is from 2018.

@Felienne
Copy link
Member

Yes, I meant removing the time.sleep only for commands placed in an if-pressed statement

I am ok with that!

mergify bot pushed a commit that referenced this issue Sep 13, 2024
This PR addresses the following issues:
- When the body of an if-pressed command contains more than one turtle functions, only the first one will be executed.
- When the body of an if-pressed command contains time.sleep(), the execution would terminate when reaching this line.

Fixes  #5729 #5681

**How to test**
Run locally, go to level 15 and run the following scenarios:
1. Check that the code below outputs all items in the list. Note that they should not appear at once, but with a tiny delay in between:
```
lijstje is "1", "2", "3", "4", "5"
if x is pressed
    for dier in lijstje
        print dier
else
    print 'onbekend dier'
```
2. Check that when pressing x, all statements are executed. Note that the waiting-for-key-press modal should appear and then disappear while the action is being executed. Note that pressing the x button while the turtle is moving should not trigger a new run. If an error occurs, the keys should not be animated anymore.
```
i = 0
while i < 20
    if x is pressed
        turn 10
        color 'blue'
        turn 90
        forward 50
        color 'red'
        turn 90
        forward 50
        color 'orange'
        turn 90
        forward 50
        color 'green'
        turn 90
        forward 50
    else
        turn -15
        color 'blue'
        turn -90
        forward 50
        color 'red'
        turn -90
        forward 50
        color 'orange'
        turn -90
        forward 50
        color 'green'
        turn -90
        forward 50
    i = i + 1
```
@boryanagoncharenko
Copy link
Collaborator Author

Closed by PR #5763

@Felienne Felienne moved this from ToBeDiscussed to Done in Hedy organization board Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

3 participants