-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add falling block animation with <FallingBlock/>
#52
Conversation
Please format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the inline comments. I'll review the semantics more thoroughly later.
src/util/playerUtil.tsx
Outdated
export function dropFloatingCells( | ||
board: BoardCell[][], | ||
): [BoardCell[][], [number, number][], [number, number][]] { | ||
): [BoardCell[][], BoardCell[], BoardCell[]] { | ||
// Returns an array of 3 arrays: | ||
// Array 1: The resulting board with drops. | ||
// Array 1: The resulting board with the only old coords removed. | ||
// Array 2: The array for the coords of the floating cells, post-drop. | ||
// Array 3: Array for the old coords of the floating cells. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should just return an object with field names that describe their use instead of only putting that information in the documentation here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was talking about the return type. It shouldn't be a tuple with comments describing what each index means, it should be an object with self-describing fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean. That's a great change. Updated it again
Lint and Format are still failing in CI, though. |
250bfe7
to
809ae14
Compare
809ae14
to
5a012d6
Compare
3da0a69
to
73cd4cc
Compare
I had forgotten to include |
This is caused by Bun not being able to install `react-scripts`. Discussed in #44
Note: Although slight non-deterministic behavior existed before this PR, it is more apparent now during the fall animations. This should be addressed in Remove non-determinism #51