Skip to content

multiple improvements : memory, functionality #18

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

Merged
merged 6 commits into from
Mar 6, 2020
Merged

multiple improvements : memory, functionality #18

merged 6 commits into from
Mar 6, 2020

Conversation

Marius-450
Copy link
Contributor

multiple improvements : memory, functionality + consistency with documentation

See this repo for the full changelog.

  • memory improvements : smaller background bitmap scaled to fullscreen and added a scale parameter to scale the "canvas".
  • colors : +7 colors (for same memory consumption), background color management.
  • mode is now implemented and consistent with documentation.
  • Images : added set_bgpic() to display a bitmap as background, using OnDiskBitmap. Added changeturtle() to change the turtle shape via OnDiskBitmap or by passing a tilegrid object.
  • Implemented : pensize(), speed(), reset() and some helpers like distance() or towards()
  • Stamping of the turtle.
  • Some details like fixing rounding errors in circle(), or improving dot()

The only problematic change i saw while testing all the example is the default heading to 0. I changed 4 examples to work as intended, by only setting heading to 90 once. Everything else worked out of the box, just a little slower due to the default speed value.

I removed the debug logger, and what seemed irrelevant or useless : event-based functions, undo buffer ... any comment is welcome about those.

On a 240x240 display, a turtle object use around 30500 Bytes with this version, versus 37500 Bytes previously. Using scale=2 the object use "only" 8900 Bytes of memory.

@ladyada
Copy link
Member

ladyada commented Mar 6, 2020

thanks! if you can lint it, then we will test and merge :)

@Marius-450
Copy link
Contributor Author

@ladyada : done ! all checks have passed.
I didn't find a lot of testers for the alpha version, but I tested a lot myself. I hope there is not a lot of bug...

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

This is great! I would just say to try to make it as much like the CPython library version https://docs.python.org/3/library/turtle.html as possible. I made one suggestion in that regard, though there may be others. I'm not that familiar with the original API, so for instance I see that changeturtle() is not in the original API. But maybe there is no equivalent for that.

dhalbert
dhalbert previously approved these changes Mar 6, 2020
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

This looks good to me! I tried some functions but not all, and it works fine.
@Marius-450 Maybe we will make this 2.0.0, since there are so many changes?
@ladyada any further comments?

@Marius-450
Copy link
Contributor Author

I have no objection making this 2.0.0. do I need to change something for that ?

@dhalbert
Copy link
Contributor

dhalbert commented Mar 6, 2020

No, we'll make a release, no problem. But I just tried setting .pencolor and .bgcolor, and I'm still getting a white pen and a black background. Am I doing something wrong? (This is on a PyPortal)

>>> t.pencolor = Color.RED
>>> t.right(90)
>>> t.forward(100)
>>> t.pencolor = Color.YELLOW
>>> t.right(90)
>>> t.forward(100)

@dhalbert dhalbert self-requested a review March 6, 2020 19:31
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

will wait for comments re color changing

@dhalbert dhalbert dismissed their stale review March 6, 2020 19:46

color changing q

@Marius-450
Copy link
Contributor Author

Marius-450 commented Mar 6, 2020

try t.pencolor(Color.RED) or t._pencolor = 2 

@dhalbert
Copy link
Contributor

dhalbert commented Mar 6, 2020

try t.pencolor(Color.RED) or t._pencolor = 2

OK, thanks, that's it, I'm too quick to try a property, and all I did was smash the method. :)

@dhalbert dhalbert merged commit be1e671 into adafruit:master Mar 6, 2020
@Marius-450 Marius-450 deleted the marius_improvements branch March 6, 2020 19:54
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Mar 7, 2020
Updating https://github.com/adafruit/Adafruit_CircuitPython_Pypixelbuf to 1.0.3 from 1.0.2:
  > build.yml: move pylint, black, and Sphinx installs to each repo; add description to 'actions-ci/install.sh'
  > Merge pull request adafruit/Adafruit_CircuitPython_Pypixelbuf#6 from jepler/no-re

Updating https://github.com/adafruit/Adafruit_CircuitPython_turtle to 2.0.0 from 1.1.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_turtle#18 from Marius-450/marius_improvements
  > build.yml: move pylint, black, and Sphinx installs to each repo; add description to 'actions-ci/install.sh'

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_Wiznet5k
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants