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.

Sign up for GitHub

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

Jump to bottom

🪲 Fix Key Pressed adventure in level 9 #5725

Merged
merged 4 commits into from
Sep 3, 2024
Merged

🪲 Fix Key Pressed adventure in level 9 #5725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
563ba16
Fix variable scope in if_pressed command; fix variables not recognize…
boryanagoncharenko Aug 28, 2024
3c5fac6
Address comments #5679
boryanagoncharenko Aug 29, 2024
bb21a9c
Merge branch 'main' into key_press_5679
boryanagoncharenko Aug 29, 2024
c46355d
Merge branch 'main' into key_press_5679
jpelay Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 35 additions & 13 deletions hedy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2218,10 +2218,10 @@ def if_pressed_else(self, meta, args):
self.process_arg_for_data_access(args[0], meta.line)
key = args[0]

if_code = args[1]
if_code = self.if_pressed_code_prepend_global_vars(args[1])
if_function_name = self.make_function_name(key)

else_code = args[2]
else_code = self.if_pressed_code_prepend_global_vars(args[2])
else_function_name = self.make_function_name('else')

return (
Expand All @@ -2233,6 +2233,24 @@ def if_pressed_else(self, meta, args):
self.make_extension_call()
)

def if_pressed_code_prepend_global_vars(self, lines):
""" Combines if_pressed arguments into a code snippet, which will be placed inside a function. To allow
assigning and reassigning variables, all lookup entries, except for funcs and list access, are marked as
global and prepended to the snippet. For example:
points = 5
def if_pressed():
global points <-- without this line, we get an error
points = points + 1
"""
lines = lines if isinstance(lines, list) else [lines]
variables = [e.name for e in self.lookup if '(' not in e.name and '[' not in e.name]
if variables and lines:
variables = sorted(list(set(variables)))
global_vars = f"global {', '.join(variables)}"
lines.insert(0, global_vars)

return '\n'.join(lines)


@v_args(meta=True)
@hedy_transpiler(level=6)
Expand Down Expand Up @@ -2542,11 +2560,9 @@ def ifs(self, meta, args):
def if_pressed(self, meta, args):
self.process_arg_for_data_access(args[0], meta.line)
args = [a for a in args if a != ""] # filter out in|dedent tokens

key = args[0]

if_code = '\n'.join([x for x in args[1:]])
if_code = ConvertToPython.indent(if_code)
if_code = self.if_pressed_code_prepend_global_vars(args[1:])
if_function_name = self.make_function_name(key)

return (
Expand All @@ -2555,10 +2571,9 @@ def if_pressed(self, meta, args):
self.make_function(if_function_name, if_code) + '\n'
)

def if_pressed_elses(self, met, args):
def if_pressed_elses(self, meta, args):
args = [a for a in args if a != ""] # filter out in|dedent tokens
else_code = '\n'.join([x for x in args[0:]])
else_code = ConvertToPython.indent(else_code)
else_code = self.if_pressed_code_prepend_global_vars(args)
else_function_name = self.make_function_name('else')

return (
Expand Down Expand Up @@ -3034,6 +3049,16 @@ def returns(self, meta, args):
except ValueError:
return Value(f'''{args_str}''')""")

def add_if_key_mapping(self, key, function_name):
return textwrap.dedent(f"""\
global {function_name}
if_pressed_mapping['{key}'] = '{function_name}'""")

def add_else_key_mapping(self, function_name):
return textwrap.dedent(f"""\
global {function_name}
if_pressed_mapping['else'] = '{function_name}'""")


@v_args(meta=True)
@hedy_transpiler(level=13)
Expand Down Expand Up @@ -3087,8 +3112,7 @@ def not_equal(self, meta, args):
class ConvertToPython_15(ConvertToPython_14):
def while_loop(self, meta, args):
args = [a for a in args if a != ""] # filter out in|dedent tokens
all_lines = [ConvertToPython.indent(x) for x in args[1:]]
body = "\n".join(all_lines)
body = self.indent("\n".join(args[1:]))
body = add_sleep_to_command(body, True, self.is_debug, location="after")
exception = self.make_index_error_check_if_list([args[0]])
return exception + "while " + args[0] + ":" + self.add_debug_breakpoint() + "\n" + body
Expand Down Expand Up @@ -3156,11 +3180,9 @@ def elifs(self, meta, args):
def if_pressed_elifs(self, meta, args):
self.process_arg_for_data_access(args[0], meta.line)
args = [a for a in args if a != ""] # filter out in|dedent tokens

key = args[0]

elif_code = '\n'.join([x for x in args[1:]])
elif_code = self.indent(elif_code)
elif_code = self.if_pressed_code_prepend_global_vars(args[1:])
elif_function_name = self.make_function_name(key)

return (
Expand Down
10 changes: 9 additions & 1 deletion static/vendor/skulpt-stdlib-extensions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,15 @@ var $builtinmodule = function (name) {
// If mapped key is a variable (not char), we retrieve variable value and use that
// otherwise if char, use that.
const charOrVar = value[0].v;
let mapLetterKey = Object.hasOwn(Sk.globals, charOrVar) ? Sk.globals[charOrVar].v : charOrVar;

let mapLetterKey = charOrVar;
Copy link
Member

Choose a reason for hiding this comment

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

I think this solution is great and works, my only question would be if someone assigns a multileter string, and not a single letter, and then uses it in the key press. Like this:

x is asdf
if x is pressed
    print 'great'
else
    print 'not great'

Then it will always be false!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right. We allow the left-hand-side of the if-pressed command to be a variable. And this variable could be assigned anything: x is 21 + 1 or x is hello world. At the same time we do not allow for multiple keys to be pressed at the same time.

What do you think is the best thing to do here? We could either silently take the first letter of the string which might be confusing. Alternatively, we could issue a warning at runtime (not sure if we have the mechanism for that?) that lets the user know that it is a multi-letter string so it will not work.

Since this issue was not introduced by the PR, I created a separate issue to discuss and track the matter: #5727

Copy link
Member

Choose a reason for hiding this comment

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

I think the best way would be show an alert or an error warning the user that probably they don't want to be using a multicharacter string in a keypress! It can probably be done by checking the value of the variable and then calling the modal.alert function I think.

if (Object.hasOwn(Sk.globals, charOrVar)) {
if (Sk.globals[charOrVar].hasOwnProperty('v')) {
mapLetterKey = Sk.globals[charOrVar].v;
} else {
mapLetterKey = Sk.globals[charOrVar].$d.entries['data'][1].v;
}
}

if (event.key === `${mapLetterKey}`){
pressed_mapped_key = true;
Expand Down
48 changes: 47 additions & 1 deletion tests/test_level/test_level_05.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,7 @@ def if_pressed_else_():

self.multi_level_tester(code=code, expected=expected, max_level=7)

def test_if_pressed_x_is_variable(self):
def test_if_pressed_x_is_var(self):
code = textwrap.dedent("""\
x is a
if x is pressed print 'it is a letter key' else print 'it is another letter key'
Expand All @@ -1137,14 +1137,60 @@ def test_if_pressed_x_is_variable(self):
if_pressed_mapping['x'] = 'if_pressed_x_'
if_pressed_mapping['else'] = 'if_pressed_else_'
def if_pressed_x_():
global x
print(f'it is a letter key')
def if_pressed_else_():
global x
print(f'it is another letter key')
extensions.if_pressed(if_pressed_mapping)
print(f'{x}')""")

self.single_level_tester(code=code, expected=expected)

def test_if_pressed_x_is_var_and_var_reassignment(self):
code = textwrap.dedent("""\
x is a
if x is pressed x is great else x is not great
print x""")

expected = self.dedent("""\
x = 'a'
if_pressed_mapping = {"else": "if_pressed_default_else"}
if_pressed_mapping['x'] = 'if_pressed_x_'
if_pressed_mapping['else'] = 'if_pressed_else_'
def if_pressed_x_():
global x
x = 'great'
def if_pressed_else_():
global x
x = 'not great'
extensions.if_pressed(if_pressed_mapping)
print(f'{x}')""")

self.single_level_tester(code=code, expected=expected)

def test_if_pressed_x_is_var_and_new_var_assignment(self):
code = textwrap.dedent("""\
x is a
if x is pressed m is great else m is not great
print m""")

expected = self.dedent("""\
x = 'a'
if_pressed_mapping = {"else": "if_pressed_default_else"}
if_pressed_mapping['x'] = 'if_pressed_x_'
if_pressed_mapping['else'] = 'if_pressed_else_'
def if_pressed_x_():
global m, x
m = 'great'
def if_pressed_else_():
global m, x
m = 'not great'
extensions.if_pressed(if_pressed_mapping)
print(f'{m}')""")

self.single_level_tester(code=code, expected=expected)

def test_double_if_pressed(self):
code = textwrap.dedent("""\
if x is pressed print 'first key' else print 'something else'
Expand Down
48 changes: 47 additions & 1 deletion tests/test_level/test_level_06.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2654,7 +2654,7 @@ def test_if_ar_number_not_list_with_latin_numbers(self):
#
# if pressed tests
#
def test_if_pressed_x_is_variable(self):
def test_if_pressed_x_is_var(self):
code = textwrap.dedent("""\
x is a
if x is pressed print 'it is a letter key' else print 'it is another letter key'
Expand All @@ -2666,10 +2666,56 @@ def test_if_pressed_x_is_variable(self):
if_pressed_mapping['x'] = 'if_pressed_x_'
if_pressed_mapping['else'] = 'if_pressed_else_'
def if_pressed_x_():
global x
print(f'it is a letter key')
def if_pressed_else_():
global x
print(f'it is another letter key')
extensions.if_pressed(if_pressed_mapping)
print(f'{x}')""")

self.multi_level_tester(code=code, expected=expected, max_level=7)

def test_if_pressed_x_is_var_and_var_reassignment(self):
code = textwrap.dedent("""\
x is a
if x is pressed x is great else x is not great
print x""")

expected = self.dedent("""\
x = Value('a')
if_pressed_mapping = {"else": "if_pressed_default_else"}
if_pressed_mapping['x'] = 'if_pressed_x_'
if_pressed_mapping['else'] = 'if_pressed_else_'
def if_pressed_x_():
global x
x = Value('great')
def if_pressed_else_():
global x
x = Value('not great')
extensions.if_pressed(if_pressed_mapping)
print(f'{x}')""")

self.multi_level_tester(code=code, expected=expected, max_level=7)

def test_if_pressed_x_is_var_and_new_var_assignment(self):
code = textwrap.dedent("""\
x is a
if x is pressed m is great else m is not great
print m""")

expected = self.dedent("""\
x = Value('a')
if_pressed_mapping = {"else": "if_pressed_default_else"}
if_pressed_mapping['x'] = 'if_pressed_x_'
if_pressed_mapping['else'] = 'if_pressed_else_'
def if_pressed_x_():
global m, x
m = Value('great')
def if_pressed_else_():
global m, x
m = Value('not great')
extensions.if_pressed(if_pressed_mapping)
print(f'{m}')""")

self.multi_level_tester(code=code, expected=expected, max_level=7)
Loading
Loading