Opened 7 years ago

Closed 7 years ago

Last modified 3 years ago

#2420 closed defect (fixed)

[PATCH] EndGame() does not send pending messages of turn to server

Reported by: Michael Owned by: Michael
Priority: Must Have Milestone: Alpha 16
Component: Network Keywords: network multiplayer
Cc: Patch:

Description (last modified by elexis)

When the game is ended using GameSetup->EndGame(), the network messages for the pending turn are not sended to the server.

Because of that it is not possible to send a Message over the network in the same turn as the game is ended.

Refs #2373

Attachments (2)

sendNetworkMessagesAtGameEnd_v1.patch (2.7 KB ) - added by Michael 7 years ago.
Ends sending the messages for the current turn and flushs them when the game is ended.
sendNetworkMessagesAtGameEnd_v2.patch (2.0 KB ) - added by Michael 7 years ago.
Moved some function calls to destructor. (Thx to Josh)

Download all attachments as: .zip

Change History (9)

comment:1 by Michael, 7 years ago

Owner: set to Michael

by Michael, 7 years ago

Ends sending the messages for the current turn and flushs them when the game is ended.

comment:2 by Michael, 7 years ago

Keywords: patch review added
Summary: EndGame() does not send pending messages of turn to server[PATCH] EndGame() does not send pending messages of turn to server

by Michael, 7 years ago

Moved some function calls to destructor. (Thx to Josh)

comment:3 by JoshuaJB, 7 years ago

Resolution: fixed
Status: newclosed

In 14732:

Send messages in queue before closing connection, patch by boeseRaupe. Fixes #2420

comment:4 by Josh, 7 years ago

Keywords: multiplayer added; MP patch review removed
Milestone: BacklogAlpha 16

Thanks for the patch, I only made some minor grammatical corrections to the comments.

comment:5 by elexis, 5 years ago

Component: Core engineNetwork

(changed component to network)

comment:6 by elexis, 3 years ago

Description: modified (diff)
  1. Bug: This commit forgot that the simulation command may only be flushed with CEndCommandBatchMessage for clients that finished the loading screen and rejoin process, triggering a NetServer FSM error otherwise, see Phab:rP14732 and #4594.
  1. The reported problem was never solved and cannot be solved! I leave it on the milestone for historic reasons, but it should have been an Invalid ticket.

Evidence:

  1. Phab:D1557: According to testing over there, the CEndCommandBatchMessage is received by the NetServer in 1 out of 10 tests with 1 or 2 clients total when ending that client with alt+f4.
  1. Phab:rP15106 removing EndGame call:

Phab:rP14732 was a preparation of having players resign just before leaving the game: #2373. Phab:rP14772 implemented a resign question, but still ended the game instantly after attempting to send the command.

Phab:rP15106 changed that clicking resign after confirming the leave doesn't actually end the game anymore after trying to post the simulation command, but to post the simulation command and then don't do anything (rather than eventually exiting).

  1. Testing for the reported issue specifically Alpha 23. Add leaveGame(false); to resignGame and remove resignGame from leaveGame. Click on exit, confirm to resign and you're most likely out of the game without having sent the defeat message.

Also, at some later point a question was added to ask to leave after having been defeated, which in case of resigning through the exit button is duplicate: #5204 (but I don't know the culprit commit).

Also notice that there's the problem that the defeat will never be confirmed during pause. That can't be solved unless testing if the server confirmed the receive of these commands (which it doesn't until the next turn currently.)

Edit: Bit of related chat in 2014-01-31-QuakeNet-#0ad-dev.log I didn't read.

Last edited 3 years ago by elexis (previous) (diff)

comment:7 by elexis, 3 years ago

In 21837:

Fix network FSM errors when a client closes the game during the loading screen or rejoin synchronization stage, fixes #4594, refs 3199.

Fix comments claiming to ensure reliability when in reality there is only a small likelihood of the message being received.

Broken by rP14732 / refs #2420 which was a preparation for rP14772 / refs #2373, which hence didn't actually work and was circumvented by rP15106 without clarification.
Might have caused #3643.
Differential Revision: https://code.wildfiregames.com/D1557
Reviewed By: Imarok

Note: See TracTickets for help on using tickets.