search

fishfolk / bones

An easy-to-use game engine for making real games.
https://fishfolk.org/development/bones/introduction/
Other
236 stars 20 forks source link

Matchmaker temporarily reports duplicate players in matchmaking if player disconnects + restarts search #476

Closed MaxCWhitehead closed 1 month ago

MaxCWhitehead commented 1 month ago

I noticed that if I close the game during matchmaking, and restart and search for another match with two players, it reports matchmaking update with 2 players (duplicate - old DC'd client + new one), then immediately updates correcting back to 1.

(Note this does not occur if first client stops matchmaking explicitly, only if closes and exits uncleanly.

2024-10-07T02:50:40.007215Z  INFO bones_framework::networking::online: Connected to online matchmaker
2024-10-07T02:50:40.007241Z  INFO bones_framework::networking::online_matchmaking: Resolve: Sending match request request=RequestMatchmaking(MatchInfo { max_players: 2, match_data: [], game_id: "jumpy", player_idx_assignment: Random })
2024-10-07T02:50:40.597232Z  INFO poll_send{me=yzwvngy2727ciuya}:get_send_addrs{node=7glgobrbamikjemk}: iroh_net::magicsock::node_map::node_state: new connection type typ=mixed
2024-10-07T02:50:50.532965Z  INFO bones_framework::networking::online_matchmaking: Online matchmaking updated player count: 2
2024-10-07T02:50:50.679926Z  INFO bones_framework::networking::online_matchmaking: Online matchmaking updated player count: 1

This is a minor issue, but if the game is waiting for players and does some state change after a OnlineMatchmakerResponse::MatchmakingUpdate reports all players needed for search (the first 2 reported for 2 max players), this may cause issues. If game does not do anything when reached expected player count, and waits for explicit OnlineMatchmakerResponse::GameStarting, probably won't get into any trouble here.

I tested with a local matchmaker so am certain I am the only client connecting and there was only 1 at any given time.

MaxCWhitehead commented 1 month ago

cc: @RockasMockas - I imagine this has something to do with not keeping matchmaking client's connections live all the time (if I recall that is how this is setup). I haven't dug into the fix here, not sure how tricky this is or if not too bad.

MaxCWhitehead commented 1 month ago

Worth noting game probably shouldn't do anything unrecoverable when reaching max players anyway - as if it does and a player leaves before matchmaker actually starts game, that is an issue.

RockasMockas commented 1 month ago

Yeah I pretty much designed it so that GameStarting should be the thing that causes the client to move forward.

Alternatively to make this a bit more robust (something I had in mind but didn't have time for) is that we should implement the ability for both client -> matchmaker to ping, or matchmaker -> client to ping. Then in the current connection checking code where we send the MatchmakingUpdate to check if everyone's online, we replace that with a matchmaker -> client ping.

In bones we'd read the ping and just respond with an ack confirmation, and from the developer point of view the ping would never reach them/be bothered to deal with manually.

This is a flow that I think will be good to do for long-term robustness/cleanliness, but in real terms if you just watch for GameStarting right now I don't think you should run into any issues.

MaxCWhitehead commented 1 month ago

ok - sounds good to me, I will probably just close this as pretty minor issue.