nerves-keyboard / xebow

Firmware for the Keybow written in Elixir
40 stars 10 forks source link

Simon! #73

Open doughsay opened 4 years ago

doughsay commented 4 years ago

What's your high score!?

doughsay commented 4 years ago

The question I'm mulling over is if Simon should be an animation, or if it should be an application.

We don't currently have any way to achieve this game other than as an animation? There's no support in Engine for arbitrary other applications to draw stuff to the paintables. The ONLY way for us to paint right now is to adhere to the animation behaviour.

I'm open to discussion about how to change this, but I had to work within the confines of what's possible right now.

Also trying to weigh if we want to add another animation before tackling the tech debt of cleaning up the interfaces around animations, config, engine, etc. It's one more file to refactor that has to retain its functionality.

The cost of the future refactor aside, I did this on purpose. The point is to show more possible ways to use the animation behaviour so that a refactor doesn't remove possibilities. This is exactly what happened last time and what caused this entire disrupt in the first place. Because I only originally had 60fps constant math-based animation examples to begin with, the last refactor made it so ONLY that kind animation was possible.

I'm trying to come up with as many outlandish animation ideas I can NOW so that a refactor doesn't destroy my ability to do so again in the future.

Next up: WPM heatmap! :stuck_out_tongue_closed_eyes: (kinda kidding, not actually working on this... lol)

amclain commented 4 years ago

The question I'm mulling over is if Simon should be an animation, or if it should be an application.

We don't currently have any way to achieve this game other than as an animation? There's no support in Engine for arbitrary other applications to draw stuff to the paintables. The ONLY way for us to paint right now is to adhere to the animation behaviour.

I'm open to discussion about how to change this, but I had to work within the confines of what's possible right now.

I think we do support this currently. If the application creates animations for what it wants to do, which is a solid colored pixel for Simon, it can hand those animations to the engine as it wants to play them. Here's some pseudo-code which may help illustrate what I'm trying to describe:

defmodule Simon do
  use GenServer

  # Pseudo-code - Boilerplate omitted...

  def init(_) do
    state = start_game()
    {:ok, state}
  end

  defp start_game do
    %State{
      sequence: [] # Sequence of LEDs that light up this round
      # ...
    }
    |> add_to_sequence()
    |> play_sequence()
    |> start_timer()
  end

  defp add_to_sequence(state) do
    new_sequence_element = %{
      location: random_location(),
      color: random_color()
    }

    new_sequence = state.sequence ++ [new_sequence_element]

    %State{state | sequence: new_sequence}
  end

  defp play_sequence(state) do
    state.sequence
    |> Enum.each(fn element ->
      animation = Animation.Implementation.Point.new(
        location: element.location,
        color: element.color,
        duration_in_ms: 500
      )

      Engine.play_animation(animation, async: false)
    end)
  end
end
doughsay commented 4 years ago

Here's some pseudo-code which may help illustrate what I'm trying to describe:

Engine.play_animation(animation, async: false)

We no longer support async: false. And I renamed it from play_animation to set_animation exactly for this reason: it's not "play this animation please and get back to me when it's done", it's "set this as the current animation".

I get your point though. It's just super clunky no? I'd rather Engine expose a painting interface directly, so instead of having to define and play an animation, you just say: "hey engine, paint these pixels these colors".

amclain commented 4 years ago

We no longer support async: false.

Really what I'm getting at in the pseudo-code is that the app has a way to know when to switch to the next animation. There are a handful of options for how we could do that.

It's just super clunky no?

Could you expand on this? I'm not quite following.

I saw this pseudo-code as pushing Simon "the game" into the application layer and away from modules like Animation and Engine. I think this is simplifying the architecture and making each module easier to reason about. It's separating the game (user sees and replays a pattern) from the animation (display a color on a pixel).

Simon "the animation" may appear lighter weight on the surface, but it's limited (or should be limited) in what it can do. It should never overreach the boundary of being an animation, because its render function is in the engine's hot path. It should never do non-animation things, like understand 1 or 2 player mode, play sound effects (if the Keybow had sound), or track/push stats to a leaderboard. As an animation, all it should do is describe the colors of pixels over time. The Simon animation as it's implemented is a great novelty that's fun to play; I think the temptation will be to expand out Animation and Engine when building more games (or other apps), which I think is an anti-pattern.

I'd rather Engine expose a painting interface directly, so instead of having to define and play an animation, you just say: "hey engine, paint these pixels these colors".

I don't have a good feeling about that; it would result in the engine exposing two different paradigms for how to paint pixels. Engine already has Animation as an input, and since anyone can implement an animation and output what they want in its render function, that's the painting interface. The boundaries of responsibility here are that animations paint pixels; the engine is responsible for which animation is being rendered, when it's rendered, and passing the output to devices that can display those pixels.

If we need a low-level pixel painting interface that's available in the application layer, I recommend we create an Animation that exposes more of the low-level details in its interface. For example, here's pseudo-code to illustrate the concept of using an animation that lets you paint on whatever pixels you want to:

Animation.Implementation.Paint.new(pixels: %{
  {0, 0} => "red",
  {0, 1} => "yellow",
  {1, 0} => "orange",
  {1, 1} => "green"
})

I think the more we can keep each module focused on a specific responsibility, the easier the system will be to reason about, as well as maintain.

doughsay commented 4 years ago

Yeah I think all that makes sense. But my original point still stands: the point of this experiment was to show another way to use the animation behaviour, with the specific intent of making sure that any future refactors maintain the ability to build such an animation. Just because we shouldn't use an animation to implement Simon, doesn't mean we shouldn't have the ability to. Does that make sense?

amclain commented 4 years ago

Just because we shouldn't use an animation to implement Simon, doesn't mean we shouldn't have the ability to.

image

doughsay commented 4 years ago

Added invalid tag; we're going to re-work this as an "application" instead of an animation.