Skip to content
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

Move animation management out of Engine #95

Merged
merged 8 commits into from
Aug 1, 2020

Conversation

vanvoljg
Copy link
Member

Here, animation loading, state, and management are pulled out of RGBMatrix.Engine and into the Xebow application. This separates a few concerns to where the Engine only has to deal with rendering, RGBMatrix.Animation handles functions related to animations themselves, and Xebow handles creating the list of animations, initializing animations, switching animations, and persisting config modifications in memory.

This should make it easier to introduce other application controlling logic such as animation activation/deactivation, configuration persistence between reboots, and eventually runtime discovery/installation of animations (which are currently hard-coded into the animation module).

This PR also fixes the black frame on live view creation, and removes the 0-ms render_in that was returned by all animations on calling new/2. Animations which need to delay the render of their first frame could set the first frame to black, have an internal state that determines how long to wait, and check that state in its render/2 callback functions.

addresses part of #49, but leaves open a bunch of questions aboutruntime discovery of animations, enabling/disabling, etc.
closes #80
closes #83

@vanvoljg vanvoljg added the refactor Refactoring code or tech debt repayment label Jul 29, 2020
@vanvoljg vanvoljg requested review from doughsay and amclain July 29, 2020 18:28
@vanvoljg vanvoljg self-assigned this Jul 29, 2020
@vanvoljg vanvoljg force-pushed the refactor/application-managed-animations branch from dc998b1 to d07dd37 Compare July 29, 2020 19:11
Jesse Van Volkinburg added 5 commits July 29, 2020 12:13
This moves responsibility for tracking animations and switching to next/previous animations out of the various paintables and into a single location
All animations have an initial render delay of 0ms for their initial frame.
@vanvoljg vanvoljg force-pushed the refactor/application-managed-animations branch from d07dd37 to 7f5d16c Compare July 29, 2020 19:13
lib/rgb_matrix/engine.ex Outdated Show resolved Hide resolved
lib/rgb_matrix/engine.ex Outdated Show resolved Hide resolved
lib/rgb_matrix/engine.ex Outdated Show resolved Hide resolved
lib/xebow.ex Outdated Show resolved Hide resolved
lib/xebow.ex Outdated Show resolved Hide resolved
Jesse Van Volkinburg added 2 commits July 29, 2020 22:20
Completely removes config functions from Engine
Changes state into a struct and improves the config update function
@vanvoljg vanvoljg requested a review from amclain July 30, 2020 05:22
Copy link
Member

@amclain amclain left a comment

Choose a reason for hiding this comment

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

I'll take another look at this PR tomorrow. Just a couple more comments until then.

lib/xebow.ex Outdated Show resolved Hide resolved
lib/xebow.ex Outdated Show resolved Hide resolved
Allows instant access to a given index without modifying or traversing lists
@vanvoljg vanvoljg closed this Jul 30, 2020
@vanvoljg vanvoljg deleted the refactor/application-managed-animations branch July 30, 2020 22:24
@vanvoljg vanvoljg restored the refactor/application-managed-animations branch July 30, 2020 22:25
@doughsay
Copy link
Member

wait, why was this closed, I haven't had a chance to look yet!

@amclain
Copy link
Member

amclain commented Jul 30, 2020

I was able to have a chat with @vanvoljg offline. This PR was on track to merge and didn't have any blockers, so I think we can reopen it.

@amclain amclain reopened this Jul 30, 2020
Copy link
Member

@doughsay doughsay left a comment

Choose a reason for hiding this comment

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

Great stuff! 🎉

@vanvoljg vanvoljg merged commit 04f1645 into master Aug 1, 2020
@vanvoljg vanvoljg deleted the refactor/application-managed-animations branch August 1, 2020 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring code or tech debt repayment
Projects
None yet
3 participants