Draft stability improvements

[sprangg]

Drafts can suffer issues, sometimes fatal ones, when a player lets their pick timer run down to 0. These are the three possibilities when a player's draft timer runs out: -The most common result is that the draft continues ok. -Sometimes one or more of the players don't see their next pack. They will have to wait until the pick timer runs out again, after which the draft continues normally, with those players who didn't see the pack getting a random pick since they were unable to choose any card. -Occasionally the whole draft will freeze, with none of the players ever seeing the next pack and the draft never moving on from that.

I haven't been able to reproduce this issue on localhost, but it's easily reproducible by going to an online server, starting a draft where all the players are human (you can use multiple clients for this), and letting the timers run down. There will be missed picks and/or the draft will freeze.

This all means that it's not possible to test a fix for this issue locally. I've however looked for the potential causes of this issue that I find the most likely, and attempted to fix them here. The real test of whether this actually fixes these issues will come when this is merged and beta is updated, but I've tested these changes locally and they work as intended.

Firstly, to ensure players always get their pack, there is now a system where the packs are re-sent to players until their client has confirmed to the server with setBoosterLoaded that the pack has been successfully loaded. This is taken care of by boosterLoadingHandle in DraftImpl.java and the interval is 3 seconds, which I chose because that should be enough time for all the communication between the client and the server, but low enough for the draft to still feel responsive in case the re-sending is needed.

Secondly, the only potential reason I could find for the drafts freezing is the durationless wait in DraftImpl.java. I gave it a duration of 3 seconds, so it's never left hanging forever.

Lastly, there is also a rarer issue where none of the players get their packs after the host starts the tournament, and the tournament needs to be restarted. The boosterLoadingHandle pack-resending might help with this as well.

Fixes #8486

[sprangg]

Note: If the draft freezing issue persists after this is merged, there is another potential cause, and that would be that donePicking in DraftImpl.java gets stuck at false.

I don't want to do anything about that before first testing this PR on an online server though.

[Alex-Vasile]

Note: If the draft freezing issue persists after this is merged, there is another potential cause, and that would be that donePicking in DraftImpl.java gets stuck at false.

I don't want to do anything about that before first testing this PR on an online server though.

Do you have a second computer (or another person to test this with)? Given the current release schedule of the beta, if this breaks drafting, it would be 2-3 weeks before any potential fix for it will go out.

[Alex-Vasile]

I think this is a no-brainer, but looking at the important parts, I can't understand them.

@theelk801

[sprangg]

I've tested this by successfully doing a full draft (up till the deckbuilding stage) on localhost, by joining the local server with multiple clients to make a botless pod.

I've also tested with bots, and they work as well. They make their picks immediately, so it doesn't matter that they don't send the setBoosterLoaded (it's only required if the player is still picking).

[Alex-Vasile]

I've tested this by successfully doing a full draft (up till the deckbuilding stage) on localhost, by joining the local server with multiple clients to make a botless pod.

I've also tested with bots, and they work as well. They make their picks immediately, so it doesn't matter that they don't send the setBoosterLoaded (it's only required if the player is still picking).

Right. But my concern is that testing it locally (everything on the same computer) is not a good representation of how the change will function on the beta server since you were unable to reproduce the bug locally.

[sprangg]

Alright, I managed to host an online xmage server with my laptop and connected to it with 10 clients from my other PC.

As I kind of expected, I was still unable to reproduce the draft-freezing and pick-missing issues. These issues are likely related to higher pings and less stable connections, and my connection to my own house is too good.

Anyway, this PR just aims to re-send the packs to players whose client hasn't confirmed to have received them. From what I can see, the only way that things theoretically could go wrong with this PR is that the same pack would sometimes be sent multiple times to the player. However, the client doesn't have any problem with the same pack being sent to it again, it just blinks and reloads the pack. And I've done my best to make sure that the packs will only get sent once, as long as the players receive them correctly.

Edit: I could perhaps try to get someone from another part of the world to host a server and connect to that one.

[Alex-Vasile]

I will download this as a patch in the next few days and try to wrap mg head around it. My current issue is that there are functions across multiple classes which all have very similar names, so I am not clear on how information flows between them.

As for testing this on your computer. I think what may be happening on the local servers may not be just connections problems, but also slowdown due to the server being close to capacity. Perhaps loading the local server with less ram and playing some AI games in the background would recreate the load on the public servers.

[sprangg]

I'll answer your questions later. Just wanna say that I got help and can now do testing on a server that's on the other side of the world. For now I've just tested that I do get the issues on that server, haven't tested the fixes yet.

[Alex-Vasile]

I'll answer your questions later. Just wanna say that I got help and can now do testing on a server that's on the other side of the world. For now I've just tested that I do get the issues on that server, haven't tested the fixes yet.

Have you gotten a chance to test the fix?

[sprangg]

Alright, I've tested the fix on the online server, and have been unable to reproduce the issues with the fixed version.

On the same server, the normal version has mainly had the issue that some players miss their next pick after the draft timer runs out, but I've managed to cause the draft to freeze a couple times as well.

I think the draft freeze is related to the missed picks, because the missed pick issue always happens first, and the draft freezes immediately after.

Thus, I think this PR has a chance of fixing the freezes as well as the missed picks, because this PR adds a system to make sure players don't miss any picks, by having the server resend the pick to the player until their client has confirmed to have received it.

If the draft freeze issue still happens after this is merged, a similar system most likely needs to be added to make sure that everyone's picks get successfully verified server-side and the draft can move on, i.e. the server-side draft pick timeout method needs to be looped until all the players have successfully gotten a card. Hopefully this kind of system won't be needed though.

[sprangg]

Also, if you're wondering what the heck is going on with all the changes to a billion different files, that's the way that a message is transmitted from the client to the server. I modeled my setBoosterLoaded message after the existing setMarkedCard and sendCardPick messages.

[Alex-Vasile]

Also, if you're wondering what the heck is going on with all the changes to a billion different files, that's the way that a message is transmitted from the client to the server. I modeled my setBoosterLoaded message after the existing setMarkedCard and sendCardPick messages.

I was, and some of that must be unnecessary, but an issue for a different time.

[Alex-Vasile]

Thanks for the work on this!

[sprangg]

Thank you too!

I'll consider adding the logging to DraftImpl at some point. I'd like to add a message there about a pack having been re-sent to a player.