newTurn$ stream

[Kooshaba]

• we will probably have a lot of systems that react to a new turn being triggered, so newTurn$ allows us to hook into that without manually listening for clock updates
• changed LocalCurrentStamina -> LocalStamina to better match the contract component
• LocalStamina system hooks into newTurn$ so it doesn't run every second on every single unit in the game

[alvrs]

Why the change to updateComponent? LocalStamina has only one schema key current right?

[Kooshaba]

ah yeah at one point it had more than one but I refactored

[alvrs]

Should we go back to setComponent then? Saves us one call to getComponentValue inside updateComponent

[Kooshaba]

ya did a quick update, forgot

[alvrs]

A ReplaySubject without buffer size stores all previous values (and emits them to new subscribers). I guess a buffer size of 1 is sufficient for our use cases (such that new subscribers get the current turn)

Suggested change

	const newTurn$ = new ReplaySubject<number>();
	const newTurn$ = new ReplaySubject<number>(1);

[alvrs]

(This comment is irrelevant if we implement #48 (comment))

[alvrs]

There is a useful little built-in operator for this: distinctUntilChanged: https://rxjs.dev/api/operators/distinctUntilChanged (with that we don't have to keep track of lastTurn manually)

[Kooshaba]

that's pretty cool

[alvrs]

if we don't have to keep track of lastTurn (see prev comment) we can directly subscribe our output stream here

Suggested change

	.subscribe(tick);
	.subscribe(newTurn$);

[alvrs]

(This comment is irrelevant if we implement #48 (comment))

[alvrs]

This system's purpose is only to start the stream once the gameConfig component is available correct? Since getCurrentTurn returns -1 if the game config is not available, I think a slightly more elegant solution would be to start the pipe immediately and return -1 if no config is available (or we could filter out -1). That way we don't even need to create a new ReplaySubject for newTurn$ but can simply return the stream created by applying operators to clock.time$ (since clock.time$ already is a ReplaySubject we should get the same behavior for subscribers). And by not creating a subscription here by default we can benefit from rxjs's optimization where pipes are only run if there is a subscriber.

[alvrs]

Nit: I think this can be called turn$ since it's already clear by the fact is a stream that it is the "new" turn (or "current" turn if a new subscriber subscribes)

[alvrs]

We could optimize this by having a defineQuery outside of the system, which returns an always up to date set of entities matching the query, so we can iterate over that in the system and don't have to rerun the query every time

[alvrs]

Very nice and clean!

[alvrs]

Doesn't this reintroduce the issue with useEffect remounting when setState is called from within useEffect?

[Kooshaba]

ah yeah i was just doing this so i could properly debug UI, it snuck in