Refactor: Make observers control the lifecycle of capture and playback

[snendev]

Issue

#34 - Refactor to allow users to trigger capture/playback as-needed

This is an extension of #36 using observers.

Changes

The solution here is very similar to #36 except that, because Observers are used to trigger the initialization of playback and startup, the systems that previously initialized the behavior are now observer systems. This means they don't have to be ordered, which I suppose is nice.

In the future I'd like to explore how reasonable it would be to spawn entities that handle the playback/capture rather than spawning resources, e.g. in case users wanted to record multiple windows simultaneously or store captures in the world for later playback/editing.

Questions

Which approach is preferred?

TODOs

• Still would like help auditing the test suite and either adding or suggesting whatever new tests are appropriate.

[snendev]

Whoops! Somehow hit a button on my keyboard that submitted this much before I intended to. Will update the body shortly.

[snendev]

don't know why my setup hates this repo so much. apologies for the noise. seems like it's all set now 👍

[alice-i-cecile]

So, I like the event-based toggling, but I don't understand why we're using observers here? They're intended to be used for targeting a specific entity, but in this case it appears to just be targeted globally. A simple EventReader/EventWriter will be simpler and clearer.

[snendev]

hey @alice-i-cecile, that's completely fine -- in that case I would suggest closing this and preferring #36 -- that PR works just fine for an event-based approach. after the conversation in #36, I wanted to post what work I had on-going in case it was useful for anyone.

the reasons I thought to try this observer-based approach were:

• I don't see sending events or triggering observers as more- or less-simple than one another from a user-API standpoint -- arguably the use of commands often implies one less system parameter for a user system
• the only thing the event-consuming system does is apply commands (inserting resources), so it somewhat makes sense to offer users a way to apply all those commands at once anyway (in case they want to do that themselves for some reason)
• I am not sure that a user will ever expect to read the events in other places than the one that applies commands, so it isn't particularly useful to keep the events in the Event queue that I can think of
• there is the possibility to extend this into a model where these observers do spawn, update, and despawn Entities to allow more complicated storage/retrievable of input sequences if desired (which I was originally very curious to explore and might return to for a slightly different layer of "input recording" at some point soon)

all that said, like I said, this was mostly the product of a rabbit-hole, I just thought it worth sharing for your (and hopefully @JeanMertz's) consideration

[JeanMertz]

I actually had the same idea of allowing multiple recordings at the same time (e.g. targeting different windows or input devices, or only recording interactions with specific entities), at which point recording or playback could become entity based instead of globally.

I haven’t worked with observers before, but their “instant” nature seemed preferable for starting and stopping recordings at the exact time.

[alice-i-cecile]

Okay, I'm happy to pursue this approach then :) Is this otherwise ready to be merged?

[snendev]

I'm sufficiently satisfied with the implementation for the time being, and I don't intend to add anything else unless asked to. I'm not sure that implies "ready to be merged" though 😅 -- I'd feel more comfortable saying so after a reviewer does.

[alice-i-cecile]

Okay, I'm happy with the approach and the code quality is good. It needs a bit of care with docs: an example would be helpful, and we definitely need to explain that these are triggered as observers.

The other thing to do is to add a note in RELEASES.md :)

[snendev]

Thanks a bunch for taking the time @alice-i-cecile! I'll be able to finish this up after the weekend.

[snendev]

Okay, back again. I finished up the examples -- turns out I hadn't completely wired up PlaybackWindow, so that's my bad.

After the last updates, all the examples should be fixed up to run as intended (I think). I've included some doc updates and the additional wiring to actually override serialized window entities when provided. (I'd have broken up the commits but I had a little trouble with the environment while traveling -- my apologies if I end up having to throw up a few commits after this one lol.)

To be honest though, the code in the CursorMoved portion of the send_playback_events function makes me think the approach I'm trying -- expecting users to manually opt-in to the window override, and then only handling that override for keyboard, mouse, and mousewheel events -- is a little smelly. Let me know if you have any thoughts about it.

[snendev]

Also, congrats on 0.15! I'll follow this up with an update for that when I'm home.

[snendev]

realized I forgot the note in RELEASES -- adding something now

[alice-i-cecile]

Great! I'll ship this out with the new Bevy version bump this week.

[snendev]

@alice-i-cecile actually, I got most of the way through a 0.15 PR while working on this yesterday -- just had to finish up some work in the gamepad example which requires the most changes (specifically from the Text and Gamepad reworks).

I'll put that up now and see if I can wrap it up as well -- there are one or two details I wasn't sure how to resolve best yet, but hopefully it will still save you some work.