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

🪲 Improve long program warning for sleep programs and increase time #5576

Merged
merged 12 commits into from
May 28, 2024
34 changes: 3 additions & 31 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,41 +605,13 @@ def parse():
if transpile_result.has_music:
response['has_music'] = True

if transpile_result.has_sleep:
response['has_sleep'] = True

response['variables'] = transpile_result.roles_of_variables
except Exception:
pass

if level < 7:
Copy link
Member Author

@Felienne Felienne May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if needs to go in any case, because otherwise has_sleep will never be set on programs above 7, but I am not sure what to do with the rest.

It is a bit of magic why we need to send the list of sleep lengths to the frontend. If needed, I can probably get them from the parser, but you don't always know upfront how long they will be (it a variable is involved)

I am not even sure we need all of this complex code for the sleep prompt?

I think we could do it a lot easier in the Skulpt execution? Because I am sure the interpreter knows when to sleep?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I prefer to ship this as we have it now, and ask Jesus (in due time) to solve this in the interpreter, because this is annoying people.

with querylog.log_time('detect_sleep'):
try:
# FH, Nov 2023: hmmm I don't love that this is not done in the same place as the other "has"es
sleep_list = []
pattern = (
r'time\.sleep\((?P<time>\d+)\)'
r'|time\.sleep\(int\("(?P<sleep_time>\d+)"\)\)'
r'|time\.sleep\(int\((?P<variable>\w+)\)\)')
matches = re.finditer(
pattern,
response['Code'])
for i, match in enumerate(matches, start=1):
time = match.group('time')
sleep_time = match.group('sleep_time')
variable = match.group('variable')
if sleep_time:
sleep_list.append(int(sleep_time))
elif time:
sleep_list.append(int(time))
elif variable:
assignment_match = re.search(r'{} = (.+?)\n'.format(variable), response['Code'])
if assignment_match:
assignment_code = assignment_match.group(1)
variable_value = eval(assignment_code)
sleep_list.append(int(variable_value))
if sleep_list:
response['has_sleep'] = sleep_list
except BaseException:
pass

if not raw:
try:
if username and not body.get('tutorial') and ACHIEVEMENTS.verify_run_achievements(
Expand Down
9 changes: 5 additions & 4 deletions hedy.py
Original file line number Diff line number Diff line change
Expand Up @@ -1166,9 +1166,9 @@ def all_commands(input_string, level, lang='en'):


def all_variables(input_string, level, lang='en'):
"""Return the commands used in a program string.
"""Return all variables used in a program string.

This function is still used in the web frontend, and some tests, but no longer by 'transpile'.
This function is still used by the roles of variables detection
"""
program_root = parse_input(input_string, level, lang)
abstract_syntax_tree = ExtractAST().transform(program_root)
Expand Down Expand Up @@ -3073,7 +3073,7 @@ def get_parser(level, lang="en", keep_all_tokens=False, skip_faulty=False):


ParseResult = namedtuple('ParseResult', ['code', 'source_map', 'has_turtle',
'has_pressed', 'has_clear', 'has_music', 'commands', 'roles_of_variables'])
'has_pressed', 'has_clear', 'has_music', 'has_sleep', 'commands', 'roles_of_variables'])


def transpile_inner_with_skipping_faulty(input_string, level, lang="en", unused_allowed=True):
Expand Down Expand Up @@ -3618,11 +3618,12 @@ def transpile_inner(input_string, level, lang="en", populate_source_map=False, i
has_turtle = "forward" in commands or "turn" in commands or "color" in commands
has_pressed = "if_pressed" in commands or "if_pressed_else" in commands or "assign_button" in commands
has_music = "play" in commands
has_sleep = "sleep" in commands

roles_of_variables = determine_roles(lookup_table, input_string, level, lang)

parse_result = ParseResult(python, source_map, has_turtle, has_pressed,
has_clear, has_music, commands, roles_of_variables)
has_clear, has_music, has_sleep, commands, roles_of_variables)

if populate_source_map:
source_map.set_python_output(python)
Expand Down
6 changes: 3 additions & 3 deletions static/js/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,7 @@ export function runPythonProgram(this: any, code: string, sourceMap: any, hasTur
$('#turtlecanvas').show();
}

if (hasSleep) {
if (hasSleep && theLevel < 7) {
function executeWithDelay(index: number) {
return new Promise((resolve, reject) => {
if (index >= hasSleep.length) {
Expand Down Expand Up @@ -1076,8 +1076,8 @@ export function runPythonProgram(this: any, code: string, sourceMap: any, hasTur
// Also on a level < 7 (as we don't support loops yet), a timeout is redundant -> just set one for 5 minutes
return (3000000);
}
// Set a time-out of either 20 seconds when having a sleep and 5 seconds when not
return ((hasSleep) ? 20000 : 5000);
// Set a time-out of either 30 seconds when having a sleep and 10 seconds when not
return ((hasSleep) ? 30000 : 10000);
}) ()
});

Expand Down
4 changes: 2 additions & 2 deletions static/js/appbundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -105479,7 +105479,7 @@ def note_with_error(value, err):
code_prefix += music_prefix;
$("#turtlecanvas").show();
}
if (hasSleep) {
if (hasSleep && theLevel < 7) {
let executeWithDelay = function(index3) {
return new Promise((resolve2, reject) => {
if (index3 >= hasSleep.length) {
Expand Down Expand Up @@ -105545,7 +105545,7 @@ def note_with_error(value, err):
if (level3 < 7) {
return 3e6;
}
return hasSleep ? 2e4 : 5e3;
return hasSleep ? 3e4 : 1e4;
}()
});
const currentProgram = Number(sessionStorage.getItem("currentProgram") || 0) + 1;
Expand Down
4 changes: 2 additions & 2 deletions static/js/appbundle.js.map

Large diffs are not rendered by default.

Loading