feat(simultaneous-moves): add option to long form move to ignore stale stateID (close #828) #832

evandroabukamel · 2020-10-20T02:16:16Z

Solves issue #828 .

Adding ignoreStateStateID option to LongFormMove.
Exporting IsLongFormMove function from game.ts.
On master.ts, getting the move and checking if it does not have the ignoreStateStateID option thuthy before triggering invalid stateID error.

Checklist

Use a separate branch in your local repo (not master).
Test coverage is 100% (or you have a story for why it's ok).

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

This reverts commit 62e98e7.

…into nicolodavis/fix-github-install

Sync master from original repository

Merge from base repository

…state. Add ignoreStateState option to LongFormMove. Export IsLongFormMove function from game.ts. On master.ts, get the move and check if it does not have the ignoreStateState option thuthy before triggering invalid stateID error.

Nicolodavis/fix GitHub install

…om:evandroabukamel/boardgame.io into evandroabukamel/process-move-stale-stateid

evandroabukamel · 2020-10-20T02:27:13Z

The file src/server/index.ts has a lower coverage and is causing coveralls to fail. I didn't work on it.

delucis · 2020-10-20T12:48:03Z

The file src/server/index.ts has a lower coverage and is causing coveralls to fail. I didn't work on it.

That’s OK — you don’t need to try to fix the src/server/index.test.ts coverage as part of this PR :-)

src/master/master.ts

src/master/master.test.ts

Improved unit test for simultaneous moves.

evandroabukamel · 2020-10-20T17:50:53Z

I've refactored the code for getMove on master.ts and improved the unit test for simultaneous moves.

…SocketIO onUpdate calls.

delucis

Hi @evandroabukamel — how’s this going? It looks pretty good to me, we probably just need to include a test using an asynchronous storage (like the AsyncInMemory) somehow to make sure the queue protects the master from race conditions as expected.

src/server/transport/socketio.ts

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

…om:evandroabukamel/boardgame.io into evandroabukamel/process-move-stale-stateid

delucis

Hi @evandroabukamel, I have a couple of questions regarding the implementation, before I look into some more of the details. Thanks!

src/server/transport/socketio.ts

src/server/transport/socketio-simultaneous.test.ts

Reducing PQueue interval to 50ms. It needs to be tested in real world,

evandroabukamel · 2020-10-28T20:36:52Z

I'm trying to install this branch on my project and deploy it on Heroku, but I got "run-s not found". Do someone know how can I solve this? As you can see from my last commits I tried a lot o things but nothing solved it. @delucis or @nicolodavis ? Thanks.

I managed to upload a user-scoped package to NPM and it seem to work fine.

delucis · 2020-10-28T22:12:32Z

I'm trying to install this branch on my project and deploy it on Heroku, but I got "run-s not found".

I think the changes in #749 might help, but don’t remember the status there. The run-s command is provided by the npm-run-all package.

Fixing unit tests.

…om:evandroabukamel/boardgame.io into evandroabukamel/process-move-stale-stateid

delucis

Hi @evandroabukamel — sorry for the wait on going through these and thanks for your patience. Glad to hear the tests with your app went smoothly. Hopefully most of these changes are small details and we can get this merged soon 👍

src/server/transport/socketio-simultaneous.test.ts

packages/server.ts

src/master/master.ts

src/master/master.test.ts

src/server/transport/socketio.test.ts

src/server/transport/socketio.ts

delucis · 2020-11-07T22:38:40Z

Merged! Thanks for your persistence @evandroabukamel 🎉

nicolodavis and others added 13 commits June 20, 2020 17:46

fix github install

ed20cf8

Update package.json

62e98e7

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

remove unused script

2948777

Merge branch 'master' into nicolodavis/fix-github-install

e65bcf1

Revert "Update package.json"

8187e01

This reverts commit 62e98e7.

Merge remote-tracking branch 'origin/nicolodavis/fix-github-install' …

773b072

…into nicolodavis/fix-github-install

Merge pull request #1 from boardgameio/master

63ab9f7

Sync master from original repository

Merge pull request #2 from boardgameio/master

410ac5c

Merge from base repository

Merge pull request #3 from boardgameio/nicolodavis/fix-github-install

bc66a8c

Nicolodavis/fix GitHub install

Rename ignoreStaleState option to ignoreStaleStateID.

a692837

Merge branch 'evandroabukamel/process-move-stale-stateid' of github.c…

4a54250

…om:evandroabukamel/boardgame.io into evandroabukamel/process-move-stale-stateid

Undo merge with nicolodavis/fix-github-install.

ba28577

evandroabukamel changed the title ~~feat(simultaneous-moves): add option to long form move to ignore stale stateID~~ feat(simultaneous-moves): add option to long form move to ignore stale stateID (close #828) Oct 20, 2020

delucis requested changes Oct 20, 2020

View reviewed changes

src/master/master.ts Show resolved Hide resolved

src/master/master.test.ts Outdated Show resolved Hide resolved

Refactoring usage of getMove on master.ts.

3572f61

Improved unit test for simultaneous moves.

evandroabukamel force-pushed the evandroabukamel/process-move-stale-stateid branch from 07441a2 to 3572f61 Compare October 20, 2020 17:49

Evandro Abu Kamel added 2 commits October 21, 2020 09:53

Implemented class InMemoryAsync to test simultaneous moves.

900762f

Using PQueue library to create a per-match queue for actions sent to …

fa5ef6c

…SocketIO onUpdate calls.

delucis reviewed Oct 22, 2020

View reviewed changes

src/server/transport/socketio.ts Outdated Show resolved Hide resolved

src/server/transport/socketio.ts Outdated Show resolved Hide resolved

src/server/transport/socketio.ts Outdated Show resolved Hide resolved

evandroabukamel and others added 7 commits October 22, 2020 10:51

Update src/server/transport/socketio.ts

706f107

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

Update src/server/transport/socketio.ts

81b515c

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

Implemented all base methods on InMemoryAsync class.

b55d98c

Refactor createMatchQueue method to getMatchQueue.

9f7a2a5

Fix to install boardgame.io from GitHub.

ebd03ce

Export InMemory in server/db.

d36a839

Fix comment text.

971a5d4

Evandro Abu Kamel and others added 5 commits October 27, 2020 13:06

Merge branch 'evandroabukamel/process-move-stale-stateid' of github.c…

366f735

…om:evandroabukamel/boardgame.io into evandroabukamel/process-move-stale-stateid

Fix unit test for simultaneous moves.

eb12e91

Adjust error messages on Master.

4775d34

Undo changes to fix GitHub install.

aacb4fe

Merge branch 'master' into evandroabukamel/process-move-stale-stateid

8c92b48

delucis reviewed Oct 28, 2020

View reviewed changes

src/server/transport/socketio.ts Outdated Show resolved Hide resolved

src/server/transport/socketio-simultaneous.test.ts Outdated Show resolved Hide resolved

Separating simultaneous moves tests to use sync and async storages.

48df54e

Reducing PQueue interval to 50ms. It needs to be tested in real world,

evandroabukamel force-pushed the evandroabukamel/process-move-stale-stateid branch from 3097c5d to 48df54e Compare October 28, 2020 20:58

evandroabukamel and others added 8 commits October 29, 2020 15:05

Merge branch 'master' into evandroabukamel/process-move-stale-stateid

4493329

Merge branch 'master' into evandroabukamel/process-move-stale-stateid

beb0c8e

Add logging messages for InMemory setState and setMetatada methods.

f085548

Merge branch 'master' into evandroabukamel/process-move-stale-stateid

a50e470

Cleaning code removing PQueue interval, which wasn't needed.

f4fa2c1

Fixing unit tests.

Merge branch 'evandroabukamel/process-move-stale-stateid' of github.c…

baec487

…om:evandroabukamel/boardgame.io into evandroabukamel/process-move-stale-stateid

Removing unnecessary mock for IsSynchronous.

22d4a50

Merge branch 'master' into evandroabukamel/process-move-stale-stateid

fc8cda5

evandroabukamel requested a review from delucis November 6, 2020 02:58

delucis requested changes Nov 7, 2020

View reviewed changes

Evandro Abu Kamel and others added 2 commits November 7, 2020 17:43

Fixing unit tests and doing adjustments from code review.

1697528

Merge branch 'master' into evandroabukamel/process-move-stale-stateid

35871ad

evandroabukamel requested a review from delucis November 7, 2020 20:45

delucis added 4 commits November 7, 2020 23:06

test(socketio): Remove unused import

dfcb767

test: Randomise InMemoryAsync interval

9c00ae0

style(master): Revert style change

0a350c3

docs(Game): Document ignoreStaleStateID move option

2c64877

delucis approved these changes Nov 7, 2020

View reviewed changes

delucis merged commit 6c4e94f into boardgameio:master Nov 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(simultaneous-moves): add option to long form move to ignore stale stateID (close #828) #832

feat(simultaneous-moves): add option to long form move to ignore stale stateID (close #828) #832

evandroabukamel commented Oct 20, 2020 •

edited

Loading

evandroabukamel commented Oct 20, 2020 •

edited

Loading

delucis commented Oct 20, 2020

evandroabukamel commented Oct 20, 2020

delucis left a comment

delucis left a comment

evandroabukamel commented Oct 28, 2020

delucis commented Oct 28, 2020

delucis left a comment

delucis commented Nov 7, 2020

feat(simultaneous-moves): add option to long form move to ignore stale stateID (close #828) #832

feat(simultaneous-moves): add option to long form move to ignore stale stateID (close #828) #832

Conversation

evandroabukamel commented Oct 20, 2020 • edited Loading

Solves issue #828 .

Checklist

evandroabukamel commented Oct 20, 2020 • edited Loading

delucis commented Oct 20, 2020

evandroabukamel commented Oct 20, 2020

delucis left a comment

Choose a reason for hiding this comment

delucis left a comment

Choose a reason for hiding this comment

evandroabukamel commented Oct 28, 2020

delucis commented Oct 28, 2020

delucis left a comment

Choose a reason for hiding this comment

delucis commented Nov 7, 2020

evandroabukamel commented Oct 20, 2020 •

edited

Loading

evandroabukamel commented Oct 20, 2020 •

edited

Loading