Web · Wiki · Activities · Blog · Lists · Chat · Meeting · Bugs · Git · Translate · Archive · People · Donate

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:

Version 1
  • Version 1
  • dbc9dae
  • 75b09fa
  • change interface of Turtle.set_xy to set_xy(x, y, share, pendown)

Showing 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.

→ State changed from Open to Merged

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

How to apply this merge request to your repository