unsub and subscribe to new seat in one api call

[merowin]

This is an attempt to fix the bug mentioned in https://forums.online-go.com/t/game-setup-parallel-fractional-go-game-4/53321/39

I've experienced this bug on the live site too, but could not reproduce locally. So I'm not sure if this will indeed fix the bug.

In any case, I believe this is an improvement because it reduces the number of api calls when a player selects a seat. Previously two calls were triggered, one to unsubscribe and one to subscribe to the new seat. The above mentioned error could happen if the unsubscribe call is slower than the subscribe call.

[benjaminpjones]

I think you're on the right track, but improving the way we unsubscribe from other seats (game/{seat}) implies that the bug is caused by the user changing seats. I don't think that can happen in our Fractional game since each user has a unique seat. If I'm interpreting the network traffic correctly, the problem is that we never unsubscribe from the game as an observer (seat == null).

I do think the atomic sub/unsub is good though. Also, we might consider adding a check on the client that the message is associated with the right seat so that the client can be resilient to server issues (perhaps the server never receives a certain message)

[merowin]

I think you're on the right track, but improving the way we unsubscribe from other seats (game/{seat}) implies that the bug is caused by the user changing seats. I don't think that can happen in our Fractional game since each user has a unique seat. If I'm interpreting the network traffic correctly, the problem is that we never unsubscribe from the game as an observer (seat == null).

Hm I think the function setPlayingAs is called when a user selects a seat while they already occupy the seat. The "sit down on a seat" api-call unfortunately does not subscribe the user to a socket room, because I did not know how to do that.

My thinking is that, if the unsubscribe-call is slower than the subscribe call, then the subscription created by the subscribe call is immediately destroyed by the unsubscribe-call, if it arrives at the server late. This could explain the bug, but I'm not 100% sure.

It could be caused by the observer state, but in theory state_map can hold both, because its indexed by a combination of round + (seat | null). As long as the states cached in state_map are in sync with the server game state, then its just a question of correctly displaying the state.

[benjaminpjones]

Hm I think the function setPlayingAs is called when a user selects a seat while they already occupy the seat.

Oh I got it! I thought the problem was

1) subscribe(seat 0) 2) unsubscribe(seat 1)

But the problem you're addressing is:

1) subscribe(seat 0) 2) unsubscribe(seat 0)

Thanks for explaining!

---

As an aside, I did look it up, and websocket/socket.io should preserve order: https://socket.io/docs/v4/delivery-guarantees#message-ordering

So, I'm not confident pushing the logic to the server will change the order of the socket.joins. I'm still okay with the change, but just want to be clear that we'd be making the change for readability and network traffic but not actually changing the order.

[merowin]

The bug happens inconsistently, or at least I have not figured out a pattern yet. Since I haven't been able to reproduce the bug locally, my theory is that it's caused by some asynchronous processes. Maybe a network call takes more time than usual, or something like that.

[benjaminpjones]

Okay, having gotten some eyes on GameView, I have some qualms with the way we're doing reactivity (on main).

Namely:

• never unsubscribing from the "unseated" topic (is this the source of our bug?)
• unsubscribing from all seats, when we really should unsubscribe from one (not a bug, but just feels like a smell)
• Too many effects in each watchEffect, meaning effects that should only run once are run multiple times.

See https://github.com/govariantsteam/govariants/pull/297. I'm wondering are these the types of things worth fixing? And if so, would they be compatible with this PR?

Let me know your thoughts!

[merowin]

Let me know your thoughts!

To be honest I don't have much experience with vues reactivity (or any reactive UI framework). I think we should merge your changes and check if they fix the bug! So I've converted this to draft for the time being.