search

lordmauve / wasabi2d

Cutting-edge 2D game framework for Python
https://wasabi2d.readthedocs.io/
GNU Lesser General Public License v3.0
154 stars 24 forks source link

Convert initial animation value to tuple to make it immutable #62

Closed encukou closed 3 years ago

encukou commented 3 years ago

This fixes https://github.com/lordmauve/wasabi2d/issues/61 (and hopefully https://github.com/lordmauve/wasabi2d/issues/36), and adds/amends tests. However,test_list_animation asserts that list attributes stay lists when animated. Is that the expected behavior? Should I copy the start value instead of converting to tuple (as per https://github.com/lordmauve/wasabi2d/issues/61#issuecomment-696064082)?

lordmauve commented 3 years ago

I don't think we should support lists. I guess it does take us in a direction where we might need separate codepaths to preserve types across animations of tuples, vector classes of some known type, numpy arrays... I think I'm Ok with special-casing those.

I'm not sure how easily it drops out if we use a copy.copy() though. I could be convinced...

lordmauve commented 3 years ago

I would be happy to merge this if you simply delete the list animation test.

encukou commented 3 years ago

Then I'd like to leave the test in, but adjust it to test for tuples. If it's changed in the future, the test will need to be adjusted again. And also, since all sequences are coerced to tuples, I'll remove the list special-case in tween_attr: https://github.com/lordmauve/wasabi2d/blob/51a1a9a2e71a9bbb73faadacafc9b69a7deb292d/wasabi2d/animation.py#L119-L125 Does that still sound good?

I don't think we should support lists. I guess it does take us in a direction where we might need separate codepaths to preserve types across animations of tuples, vector classes of some known type, numpy arrays... I think I'm Ok with special-casing those.

I think Transformable's property setters are a reasonable precedent for anyone who needs to ensure an attribute has a specific type.

lordmauve commented 3 years ago

I think that sounds good.