pgzero icon indicating copy to clipboard operation
pgzero copied to clipboard

Add support for simple image animations

Open dowski opened this issue 5 years ago • 1 comments

This is one way of addressing #43.

Sequences of images can be changed one after another to perform simple animations. Animations can be tied to the x or y position of an Actor or directly to the pgzero clock. That allows automatic animation while an Actor moves or while time progresses in the game. They can also be driven manually (e.g. by key presses or any other in-game event).

I added an example that shows various use of the API as well as a couple of unit tests. I think more test coverage is definitely in order if we're going to add this to pgzero, but I wanted some feedback on the current state before going further.

I'm also up for writing documentation for this before merging if it seems like a good fit for the framework.

dowski avatar Feb 16 '20 20:02 dowski

This is pretty good. It explores a lot of concepts.

A few gut feels:

  • I like the idea of an animation as an infinite cycle of frames.
  • I think you mostly nail the feature set this should have.
  • animate.images() feels like the wrong name. It doesn't exactly nail how this relates to actors.
  • I would prefer pause()/play() to start()/stop(). In particular, stop() usually indicates resetting the current frame.
  • A functional API also feels wrong given how overloaded it is. I think maybe we need to expose the ImageAnimation class.
  • I think alt_images is not the right solution; if you're looking for left/right flips then that's a feature that is already in development for Actor (iirc none of the pull requests for this (#92, #116) are quite landable yet because they mess up positioning).
  • Linking directly to actor x and actor y feels a bit too magical. It's nice for me as a game dev but I think that for beginners it's better to be a bit lower level, so they can see how things work and understand the value of abstractions when they create them or come across them. Maybe there's a simple call we can add to the update() function to wire this up, eg. anim.jump(actor.x // 20). Linking animations to the clock is very much expected, though.
  • I guess you chose every because it works for time and coordinates. If we have an API that is specific to time I'd prefer a parameter name that is specific to time, like rate, frame_rate, fps or something. This makes code more readable: every=20 is ambigious about the units for the 20, while fps=20 is not.

Something I'd like to add is sprite sheets, where there are many sprites in one source image. That will be easier to overload if we have a class. Then we can just add class methods for different ways of constructing the sequence. eg. ImageSequence.from_sprite_grid('spritesheet', rows=1, columns=10). (This is needed partly because many of the animations you can find on the web are in this form).

I think with a class we can sort out the overloading of the constructor by creating different methods, like play_once(), play_looping() etc.

lordmauve avatar Feb 17 '20 11:02 lordmauve