mechanic icon indicating copy to clipboard operation
mechanic copied to clipboard

Adding decision document around new animation api

Open runemadsen opened this issue 2 years ago • 7 comments

You can read a nicer layout by visiting the markdown file directly.

runemadsen avatar Sep 27 '22 13:09 runemadsen

Deploy Preview for dsi-logo-maker ready!

Name Link
Latest commit de8afea0d5a75120f9fd183e3c7e4e45c631ba59
Latest deploy log https://app.netlify.com/sites/dsi-logo-maker/deploys/6343dda2ac917b0008edb90c
Deploy Preview https://deploy-preview-156--dsi-logo-maker.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify[bot] avatar Sep 27 '22 13:09 netlify[bot]

Great! After thinking about it, I'm not a big fan of the customAnimation mode name. I'm wondering if we can rename it to something that makes more sense, but I don't really have a good idea yet. static and animation still seem good to me.

runemadsen avatar Sep 28 '22 07:09 runemadsen

Great! After thinking about it, I'm not a big fan of the customAnimation mode name. I'm wondering if we can rename it to something that makes more sense, but I don't really have a good idea yet. static and animation still seem good to me.

Since they are strings anyways, I was thinking on going with "custom-animation" really, but yeah maybe it's not great. I'm honestly fine with it. Maybe a separate setting only applicable for "animated", that ahs values:"frame-based" and "event-based", so we start this animation basis language in the code too.

fdoflorenzano avatar Sep 28 '22 15:09 fdoflorenzano

Thanks @fdoflorenzano! I think both of those could work. If we keep the single mode setting, then perhaps static, animation and animation-custom?

runemadsen avatar Sep 29 '22 11:09 runemadsen

I updated #152 with a draft implementation showing the new engine behavior for the two new animation modes.

A few observations:

  • Rewriting the frame and done methods as discussed above removes the need for treating them mutually exclusive in an if-else-block.
    • you only need to wrap done in an if clause, but can call frame however you want, as done will make sure to exit the function after the next call of frame
    • this logic is needed to correctly capture the last frame
  • Refactoring settings from animated?: booelan to mode?: 'static' | 'animation' | 'animation-custom' is a breaking change to existing design functions
    • how do we want to communicate this? Is it docs only? Or should we add a checkDeprecations method to the mechanic core, that can run on initialization and check for any deprecated settings being present?
  • Additionally I just realized this should probably change the way static functions are handled too. I think static functions can always be thought of as pure, so I'd argue for them being treated similar to animation functions. But I think we can even go one step further here and say, that they don't even need to call done but can just return whenever they are done drawing.

lucasdinonolte avatar Sep 30 '22 11:09 lucasdinonolte

@fdoflorenzano I have edited the doc and I think it's ready for you to take a look :)

runemadsen avatar Oct 10 '22 08:10 runemadsen

@runemadsen I like it! A couple of thoughts came to mind:

  1. Is there an animated setting or similar in the settings of a design function? I didn't see any in the examples. Maybe having an example for the static case will help this feel complete.
  2. For an animation DF, if I understood correctly, the api let's the user not use the drawLoophelper, right? Maybe adding an example of that too would help exemplify the flexibility aspect of the setup.

fdoflorenzano avatar Oct 11 '22 20:10 fdoflorenzano