You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A few months (or years?) ago, I bookmarked this project since I found it interesting.
Today, I had the chance to test it and check its code.
It's a cool project! :D
I really like the simple/straightforward syntax and API.
I have a few "remarks" though:
You are using lodash and wondered if you could get rid of it and use newer JS APIs.
Yeah, most of them can be replaced with ES6. The code already uses newer APIs (like Object.assign) but sometimes uses Lodash (_.assign) :p
Anyways, consider using lodash-es instead. It is treeshakeable, meaning the build will be a bit smaller.
In the API docs, the "Emitting state for debug purposes" section talks about receiving the state as JSON,
but the example code uses game.stateListener, which returns an object representation of the game state.
The state emitter for stateListener uses a shallow copy (using the spread operator).
This can be confusing for the programmer if they expect a snapshot of the state.
The game engine actually mutates the game state, for example this.state.graph.nodeHistory.unshift(...).
The user may accidentally mutate the data.
Consider:
Using structuredClone (supported in modern browsers and Node, and can be polyfilled).
Using Lodash's cloneDeep.
Using a silly solution like JSON stringify + parse.
Mentioning this as a caveat.
Refactoring. In Game, the bodies of Actions.RECEIVE_INPUT and Actions.SET_VARIABLES are the exact same.
Maybe combine them with an "or"(?).
Commenting the code.
There is a comment/question for Game.addObserver: "What's the difference between this and emitState?".
If I understand the code correctly,
emitState is a private method that emits the current (complete) state to a stateListener and a jsonStateListener (if they exist) whenever the state changes (more like "after handling dispatch calls").
while addObserver is a public method to register multiple listeners. They are called whenever a property they are interested in is "changed" (they will be notified if prop X is 10 but we reassign it to 10 using RECEIVE_INPUT/receiveInput).
Feature request(?): It would be awesome to have onPassageStart (or Play) and onPassageEnd (or Completed) methods.
They can be used for animations and stuff.
That is all. Again, cool project!
The text was updated successfully, but these errors were encountered:
A few months (or years?) ago, I bookmarked this project since I found it interesting.
Today, I had the chance to test it and check its code.
It's a cool project! :D
I really like the simple/straightforward syntax and API.
I have a few "remarks" though:
You are using
lodash
and wondered if you could get rid of it and use newer JS APIs.Yeah, most of them can be replaced with ES6. The code already uses newer APIs (like
Object.assign
) but sometimes uses Lodash (_.assign
) :pAnyways, consider using
lodash-es
instead. It is treeshakeable, meaning the build will be a bit smaller.In the API docs, the "Emitting state for debug purposes" section talks about receiving the state as JSON,
but the example code uses
game.stateListener
, which returns an object representation of the game state.The state emitter for
stateListener
uses a shallow copy (using the spread operator).This can be confusing for the programmer if they expect a snapshot of the state.
The game engine actually mutates the game state, for example
this.state.graph.nodeHistory.unshift(...)
.The user may accidentally mutate the data.
Consider:
structuredClone
(supported in modern browsers and Node, and can be polyfilled).cloneDeep
.Refactoring. In
Game
, the bodies ofActions.RECEIVE_INPUT
andActions.SET_VARIABLES
are the exact same.Maybe combine them with an "or"(?).
Commenting the code.
There is a comment/question for
Game.addObserver
: "What's the difference between this and emitState?".If I understand the code correctly,
emitState
is a private method that emits the current (complete) state to astateListener
and ajsonStateListener
(if they exist) whenever the state changes (more like "after handling dispatch calls").addObserver
is a public method to register multiple listeners. They are called whenever a property they are interested in is "changed" (they will be notified if prop X is 10 but we reassign it to 10 usingRECEIVE_INPUT
/receiveInput
).Feature request(?): It would be awesome to have
onPassageStart
(orPlay
) andonPassageEnd
(orCompleted
) methods.They can be used for animations and stuff.
That is all. Again, cool project!
The text was updated successfully, but these errors were encountered: