Skip to content
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

fix concurrent player check #420

Merged
merged 1 commit into from
May 18, 2024
Merged

fix concurrent player check #420

merged 1 commit into from
May 18, 2024

Conversation

windo
Copy link
Contributor

@windo windo commented May 12, 2024

The player_pool property does not exist on main site (yet?).

@windo
Copy link
Contributor Author

windo commented May 12, 2024

Actually, even with this, I still got:

May 12 16:44:07 ! source-map-support.js:495 
May 12 16:44:07 ! source-map-support.js:496 /home/siim/projects/gtp2ogs-release/dist/webpack:/gtp2ogs/src/main.ts:712
            const game_count = Object.keys(this.connected_games).filter((game_id) => {
                                                                 ^
May 12 16:44:07 ! source-map-support.js:499 TypeError: Cannot read properties of null (reading 'player_pool')
    at /home/siim/projects/gtp2ogs-release/dist/webpack:/gtp2ogs/src/main.ts:717:27
    at Array.filter (<anonymous>)
    at Main.checkGamesPerPlayer (/home/siim/projects/gtp2ogs-release/dist/webpack:/gtp2ogs/src/main.ts:712:66)
    at Main.handleNotification (/home/siim/projects/gtp2ogs-release/dist/webpack:/gtp2ogs/src/main.ts:294:30)
    at GobanSocket.<anonymous> (/home/siim/projects/gtp2ogs-release/dist/webpack:/gtp2ogs/src/main.ts:150:58)
    at GobanSocket.emit (/home/siim/projects/gtp2ogs-release/node_modules/eventemitter3/index.js:181:35)
    at WebSocket.<anonymous> (/home/siim/projects/gtp2ogs-release/dist/webpack:/gtp2ogs/node_modules/goban/src/GobanSocket.ts:266:22)
    at callListener (/home/siim/projects/gtp2ogs-release/node_modules/ws/lib/event-target.js:290:14)
    at WebSocket.onMessage (/home/siim/projects/gtp2ogs-release/node_modules/ws/lib/event-target.js:209:9)
    at WebSocket.emit (node:events:517:28)

@raylu
Copy link
Contributor

raylu commented May 12, 2024

oof, https://github.com/online-go/goban/blob/7966731fea970341987103a9e02e38c2bd31255c/src/GoEngine.ts#L120 marks player_pool as optional but tsc doesn't error because

"strictNullChecks": false,

src/main.ts Outdated
Comment on lines 714 to 715
if (state === undefined) {
return false;
}
if (state.player_pool !== undefined) {
return !!state.player_pool[player_id];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May 12 16:44:07 ! source-map-support.js:499 TypeError: Cannot read properties of null (reading 'player_pool')
    at /home/siim/projects/gtp2ogs-release/dist/webpack:/gtp2ogs/src/main.ts:717:27

means that state was null, not undefined

Suggested change
if (state === undefined) {
return false;
}
if (state.player_pool !== undefined) {
return !!state.player_pool[player_id];
if (state?.player_pool !== undefined) {
return !!state.player_pool[player_id];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah! I had added a trace.log for when state was undefined but I guess it looks like that might be the case in pretty normal circumstances. I'll try to run your suggested form for a moment.

@windo
Copy link
Contributor Author

windo commented May 12, 2024

oof, https://github.com/online-go/goban/blob/7966731fea970341987103a9e02e38c2bd31255c/src/GoEngine.ts#L120 marks player_pool as optional but tsc doesn't error because

"strictNullChecks": false,

I'm really not much of a javascript/typescript dev. Is there a neater way to express the count, or is what I've done OK?

The `player_pool` property does not exist on main site (yet?).
@windo
Copy link
Contributor Author

windo commented May 14, 2024

I've been running with this change for a few days now, no more crashes so far.

@roy7 roy7 merged commit bfaafe8 into online-go:devel May 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants