Skip to content

gh-65276: Fixed Turtle library circle method bug #104022

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

liam-gersten
Copy link
Contributor

@liam-gersten liam-gersten commented Apr 30, 2023

Behavior:

Having used multiple turtle instances to create various object for an interactive Tic-Tac-Toe game, a user noticed that setting turtle speeds to 0 (instant) lead to "disappearing" objects. The bug was with the circle method, which was called on click. The user had created tiles and lines to display the "board" using multiple turtle instances, and when the circle method was called for speed 0 turtles, the board would display on top of all lines and circle drawn, effectively making them disappear.

The issue:

When setting speed to 0 in particular, the circle method would call the _tracer method to set self._tracing and self._delayvalue to zero, before drawing the circle, and then resetting both of these values via the _tracer method. The issue is in the final call to _tracer that resets the two values. This is because _tracer actually calls screen.tracer, and when resetting with a nonzero first argument, screen.tracer actually calls screen.update() on line 1272. This update method redraws EVERY turtle object that shares the current screen, which is both unnecessary in this case, and also creates the actual bug itself.

Eventually, the screen.update() method calls t._drawturtle() for every turtle, which can (and does in this case) include background turtle objects like the user's board objects. Despite it actually iterating through the turtle objects in the right order, the t._drawturtle() method tries to draw the first object on top. This takes place in multiple screen._drawpoly calls with a top=True argument. One might think top=True would behave like a stack, i.e., the most recent call like this would actually be on top. In reality, it either behaves like a queue, or just negates every call except the first. In the case of the user's code, the first-created turtle object (the board) got the screen._drawpoly(..., top=True) call first, and thus, displayed on top of all other turtle creations.

The fix:

The screen._drawpoly ends with external library code, most likely tkinter, so I won't be attempting to change its behavior. Generally, changing _drawturtle(), update, _update, tracer, or _tracer would be inappropriate for such a bug, as doing such would be a large (potentially bug-creating) alteration for such a minor bug. Because of this, the fix should be with the circle method itself.

Due to how the screen._drawpoly(..., top=True) call behaves in _drawturtle, we only actually want the call to be made for the current turtle that is drawing the circle. Unfortunately, the update method calls _drawturtle for every turtle, so we can't avoid _drawturtle calls for the turtles we don't want to draw. Fortunately, _drawturtle won't actually call screen._drawpoly(..., top=True) when self._shown = False and self._hidden_from_screen = True. These are attributes that we CAN set from the circle method for any and every turtle object on the same screen. Thus, for every other turtle sharing the screen, we save the values of these two attributes, set them so they don't get redrawn, call self._tracer, and then restore the prior attribute values for all other turtle objects.

Resolves #65276

@arhadthedev
Copy link
Member

Azure Pipelines PR check → Ubuntu PR TestsRun patchcheck.py reports an error.

Could you run Tools/patchcheck/patchcheck.py locally, commit changes made by the script and push them here, please?

@hugovk
Copy link
Member

hugovk commented May 1, 2023

Looks like some trailing whitespace that slipped in from #103974.

Tip: set your code editor to trim trailing whitespace on save. Or check it can read the .editorconfig file.

@liam-gersten
Copy link
Contributor Author

I have made the requested changes; please review again.

@hugovk
Copy link
Member

hugovk commented May 3, 2023

Tip: usually there's no need to update the PR branch from main, unless there's something specific needed from there.

We can also save some CI resources and avoid triggering notifications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Turtle Circle Speed 0
3 participants