Reviewing merge request #7: improve interface of Turtle.set_xy
Old interface:
Turtle.set_xy(pos, share, pendown)
New interface:
Turtle.set_xy(x, y, share, pendown)
Some calls to Turtle.set_xy were already assuming the new interface. Plus, it makes it easier to write Primitive objects for blocks using Turtle.set_xy, since the x and y coordinates do not have to be packed together into a tuple.
Also fix a small bug in the 'bullet list' block of the 'presentations' palette. (The parentheses behind get_active_turtle were missing.)
Commits that would be merged:
- dbc9dae
- 75b09fa
change interface of Turtle.set_xy to set_xy(x, y, share, pendown)
dbc9dae
-75b09fa
Comments
Pushed new version 1
Were there places where we were not calling set_xy with pos? Since we use pos for our sprite positioning and coordinate conversions, maybe it makes sense to continue using it here?
Yes. Here are the places where we were calling it as set_xy(x, y, …):
- plugins/turtle_blocks_extras/turtle_blocks_extras.py, line 1409, in _prim_showlist
link
- tacollaboration.py, line 355, in _setxy
link
- tawindow.py, lin 3902, in load_turtle
link
I think this inconsistency is because the (x, y, …) interface is more intuitive. After all, the ‘setxy’ block also has two docks, one for x and one for y. So, if you don’t explicitly look up the interface, (which is probably what happened in the three cases above), you'd guess it’s (x, y, …).
But if you want to keep the (pos, …) interface, I can make another merge request to use that consistently throughout TA.
Now I remember why this makes such a big difference for the Python export tool: If I need to pack the two arguments of the ‘setxy’ block into a tuple, I need to implement wrappers around ranges of arguments again. This is the only block where I need to wrap a function (the tuple constructor) around a range of arguments. I had implemented wrappers for argument ranges in the primitive-class branch already, but it was always a pain to apply changes to them correctly. That’s why I would like to avoid them in the type-system branch if possible.
Had to resync with a change to the turtle dragging code, but otherwise, success :)
Add a new comment:
Login or create an account to post a comment