Please find both attached and on the branch table-picker-bug-13787 @
r5953 a fix for bug #13787.  Note that this will require change to the
implementation of the table picker feature in jpoker.

I plan to merge it into trunk tomorrow night.

Here's the commit log I'll use:

Merge into table-picker-bug-13787 into trunk.  See
table-picker-bug-13787 commit logs from r5949 to r5953 for detailed
implementation notes.  A summary of the work follows:

Closes bug#13787: PacketPokerError() sent on PacketPokerTablePickerFailure()

  * Deprecated empty PacketPokerTable() sent in response to
    PacketPokerTableJoin().  Currently, both empty PacketPokerTable()
    and a PacketPokerError() are sent if PacketPokerTableJoin() fails.

  * PacketPokerTablePicker() now only returns PacketPokerError() if it
    cannot find a table.  It only returns a PacketPokerTable() if a
    table is Join()'ed.

  * Documentation of PacketPokerTableJoin() and PacketPokerTablePicker()
    behavior in pokerpackets.py is improved.


The patch is copyrighted by me and licensed AGPLv3-or-later.

diff --git a/poker-network/ChangeLog b/poker-network/ChangeLog
index b4a2abc..be32339 100644
--- a/poker-network/ChangeLog
+++ b/poker-network/ChangeLog
@@ -1,3 +1,37 @@
+2009-06-27  Bradley M. Kuhn  <[email protected]>
+
+	* tests/test-pokeravatar.py.in
+	(PokerAvatarTestCase.requestsWithWrongSerial): Changed err_type
+	expected for table_picker to PACKET_POKER_ERROR.
+	(PokerAvatarTablePickerTestCase.tablePickerFails): Now expect only
+	a PACKET_POKER_ERROR on failure.
+	(PokerAvatarTablePickerTestCase): Wrote method.
+	(PokerAvatarTablePickerTestCase.restoreTableCanAddPlayer): Wrote
+	method.
+	(PokerAvatarTablePickerTestCase.test11_tablePicker_failureFromTableCannotAddPlayer):
+	Wrote test.
+
+	* pokernetwork/pokeravatar.py
+	(PokerAvatar.performPacketPokerTablePicker): Added
+	deprecatedEmptyTableBehavior and requestorPacketType arguments to
+	performPacketPokerTableJoin() call.  Error conditions that once
+	sent PacketPokerTable() now return PacketPokerError().
+	(PokerAvatar.performPacketPokerTableJoin): Added
+	deprecatedEmptyTableBehavior and requestorPacketType arguments.
+
+	* pokernetwork/pokerpackets.py (PacketPokerTablePicker):
+	Documented the new packet error return for failures.
+
+	* tests/test-pokeravatar.py.in
+	(PokerAvatarTestCase.joinTableForceFail): Added check for new
+	PACKET_POKER_ERROR when TableJoin() fails.  Added note about
+	deprecated behavior.
+
+	* pokernetwork/pokerpackets.py (PacketPokerTableJoin): Added
+	GENERAL_FAILURE error code for PacketPokerTableJoin().  Documented
+	new error response.  Documented deprecation of empty PokerTable()
+	return for errors.
+
 2009-06-26  Loic Dachary <[email protected]>
 
  	* Release 1.7.5
diff --git a/poker-network/pokernetwork/pokeravatar.py b/poker-network/pokernetwork/pokeravatar.py
index 17e7e30..8d4a280 100644
--- a/poker-network/pokernetwork/pokeravatar.py
+++ b/poker-network/pokernetwork/pokeravatar.py
@@ -780,14 +780,23 @@ class PokerAvatar:
     # PacketTablePicker() do.  A secondary benefit is that it makes that
     # giant if statement in handlePacketLogic() above a bit smaller.
     # -------------------------------------------------------------------------
-    def performPacketPokerTableJoin(self, packet, table = None):
+    def performPacketPokerTableJoin(self, packet, table = None,
+                                    deprecatedEmptyTableBehavior = True,
+                                    requestorPacketType = PACKET_POKER_TABLE_JOIN):
         """Perform the operations that must occur when a
         PACKET_POKER_TABLE_JOIN is received."""
         if not table:
             table = self.service.getTable(packet.game_id)
         if table:
             if not table.joinPlayer(self, self.getSerial()):
-                self.sendPacketVerbose(PacketPokerTable())
+                if deprecatedEmptyTableBehavior:
+                    self.sendPacketVerbose(PacketPokerTable())
+                self.sendPacketVerbose(
+                  PacketPokerError(code = PacketPokerTableJoin.GENERAL_FAILURE,
+                                   message = "Unable to join table for unknown reason",
+                                   other_type = requestorPacketType,
+                                   serial     = self.getSerial(),
+                                   game_id    = 0))
         return table
     # -------------------------------------------------------------------------
     def performPacketPokerSeat(self, packet, table, game):
@@ -838,8 +847,14 @@ class PokerAvatar:
     def performPacketPokerTablePicker(self, packet):
         mySerial = self.getSerial()
         if mySerial != packet.serial:
-            self.message("attempt to run table picker for player %d by player %d" % ( packet.serial, mySerial ))
-            self.sendPacketVerbose(PacketPokerTable())
+            errMsg = "attempt to run table picker for player %d by player %d" % ( packet.serial, mySerial )
+            self.message(errMsg)
+            self.sendPacketVerbose(
+                PacketPokerError(code       = PacketPokerTableJoin.GENERAL_FAILURE,
+                                 message    = errMsg,
+                                 other_type = PACKET_POKER_TABLE_PICKER,
+                                 serial     = mySerial,
+                                 game_id    = 0))
         else:
             # Call autorefill() first before checking for a table,
             # since the amount of money we have left will impact the
@@ -851,11 +866,24 @@ class PokerAvatar:
                 self._convertTablePickerArgsToListTableQuery(packet.min_players,
                       packet.currency_serial, packet.variant, packet.betting_structure))
 
-            if not table or not table.game.canAddPlayer(mySerial):
-                # If we cannot find a table or if for some weird reason,
-                # the table we get can't take us just send back an empty
-                # PackerPokerTable packet.
-                self.sendPacketVerbose(PacketPokerTable())
+            if not table:
+                # If we cannot find a table, tell user we were unable to
+                # find a table matching their criteria
+                self.sendPacketVerbose(
+                  PacketPokerError(code       = PacketPokerTableJoin.GENERAL_FAILURE,
+                                   message    = "No table found matching given criteria",
+                                   other_type = PACKET_POKER_TABLE_PICKER,
+                                   serial     = mySerial,
+                                   game_id    = 0))
+            elif not table.game.canAddPlayer(mySerial):
+                # If the table we found just can't take us, tell user we
+                # could not add them.
+                self.sendPacketVerbose(
+                  PacketPokerError(code      = PacketPokerTableJoin.GENERAL_FAILURE,
+                                   message   = "Found matching table, but unable to join it.",
+                                   other_type = PACKET_POKER_TABLE_PICKER,
+                                   serial     = mySerial,
+                                   game_id    = table.game.id))
             else:
                 # Otherwise, we perform the sequence of operations
                 # that is defined by the semantics of this packet in
@@ -867,7 +895,8 @@ class PokerAvatar:
                 #   PacketPokerSit()
                 if self.performPacketPokerTableJoin(
                      PacketPokerTableJoin(serial = mySerial,
-                                          game_id = table.game.id), table):
+                                          game_id = table.game.id), table,
+                       deprecatedEmptyTableBehavior = False):
 
                     # Giving no seat argument at all for the packet should cause
                     # us to get any available seat.
diff --git a/poker-network/pokernetwork/pokerpackets.py b/poker-network/pokernetwork/pokerpackets.py
index c3e4280..88d8f87 100644
--- a/poker-network/pokernetwork/pokerpackets.py
+++ b/poker-network/pokernetwork/pokerpackets.py
@@ -1119,35 +1119,64 @@ class PacketPokerTableJoin(PacketPokerId):
 Semantics: player "serial" wants to become an observer
 of the game "game_id".
 
-The packets sent to the client when successfully joining the
-table are as follows:
-
-PACKET_POKER_TABLE
-PACKET_POKER_BATCH_MODE
-for each player in the game:
-  PACKET_POKER_PLAYER_ARRIVE
-  if the player is playing:
-     PACKET_POKER_PLAYER_CHIPS
-  if the player is sit:
-     PACKET_POKER_SIT
-  PACKET_POKER_SEATS
-  if the game is running:
-     the exact packet sequence that lead to the current
-     state of the game. Varies according to the game.
-PACKET_POKER_STREAM_MODE
-
-If the player cannot join the table for any reason, the packet
-PacketPokerTable with serial 0 will be sent. It won't be filled with
-any meaningfull information.
-
-If the reason that the player was unable to join is specifically that the
-server has reached the maximum number of joined players, in addition to
-the empty PacketPokerTable sent above, the client will also receive a
-PacketPokerError with the code of PacketPokerTableJoin.FULL.
-
-If the player has already joined the table, the packets will be sent
-as if the player just joined. In this case the packet will have no
-side effect.
+There are three possible outcomes for the client in response to a
+PacketPokerTableJoin():
+
+  (0) In the case that the join is completely successful, or if the player
+      had already joined the table, the following packets are sent:
+
+          PACKET_POKER_TABLE
+          PACKET_POKER_BATCH_MODE
+          for each player in the game:
+               PACKET_POKER_PLAYER_ARRIVE
+          if the player is playing:
+                PACKET_POKER_PLAYER_CHIPS
+          if the player is sit:
+                PACKET_POKER_SIT
+          PACKET_POKER_SEATS
+          if the game is running:
+                the exact packet sequence that lead to the current state
+                of the game. Varies according to the game.
+          PACKET_POKER_STREAM_MODE
+
+      Note clearly that if the player had already previously joined the
+      table, the packets above will be sent as if the player just joined.
+      However, in that case, the packet will have no side effect.
+
+
+   (1) If the the player was unable to join the table specifically that
+       the server has reached the maximum number of joined players, two
+       packets will be sent to the client, the second of which is
+       deprecated:
+
+        (a) the following packet (recommended way of testing for failure):
+            PacketPokerError(code      = PacketPokerTableJoin.FULL,
+                            message   = "This server has too many seated players and observers.",
+                           other_type = PACKET_POKER_TABLE_JOIN,
+                           serial     = <player's serial id>,
+                           game_id    = <id of the table>)
+
+        (b) a packet, PACKET_POKER_TABLE, with serial 0 will be sent.  It
+            will contain no meaningful information.  (THIS BEHAVIOR IS
+            DEPRECATED, and is left only for older clients such as
+            poker2d.  New clients should not rely on this behavior.)
+
+  (2) If the player cannot join the table for any reason (other than the
+      table is FULL (as per (1) above), two packets will be sent to the
+      client, one of which is deprecated:
+
+       (a) the following packet (recommended way of testing for failure):
+           PacketPokerError(code      = PacketPokerTableJoin.GENERAL_FAILURE,
+                            message   = <some string of non-zero length, for use
+                                        in displaying to the user>,
+                           other_type = PACKET_POKER_TABLE_JOIN,
+                           serial     = <player's serial id>,
+                           game_id    = 0)
+
+       (b) a packet, PACKET_POKER_TABLE, with serial 0 will be sent.  It
+           will contain no meaningful information.  (THIS BEHAVIOR IS
+           DEPRECATED, and is left only for older clients such as poker2d.
+           New clients should not rely on this behavior.)
 
 Direction: server <= client
 
@@ -1156,6 +1185,7 @@ game_id: integer uniquely identifying a game.
 """
 
     FULL = 1
+    GENERAL_FAILURE = 2
     type = PACKET_POKER_TABLE_JOIN
 
 PacketFactory[PACKET_POKER_TABLE_JOIN] = PacketPokerTableJoin
@@ -4419,27 +4449,28 @@ Semantics: The player "serial" wishes to join a table that matches the
                     Send: PacketPokerBuyIn(amount = best)
                Send: PacketPokerSit()
 
+           In the case of failure, an error packet as follows will be sent
+           to the client:
+             PacketPokerError(code      = PacketPokerTableJoin.GENERAL_FAILURE,
+                              message   = <some string of non-zero length, for use
+                                          in displaying to the user>,
+                             other_type = PACKET_POKER_TABLE_PICKER,
+                             serial     = <player's serial id>,
+                             game_id    = <if failure occured before table matching criteria was identified: 0
+                                           else: game.id for table where join was attempted>)
+
            In this case of success, the client can expect to receive all
            the packets that it would expect to receive in response to any
            of the packets listed in "Send" above.  These include:
+                  PacketPokerTable()        # info about the table joined
                   PacketPokerBuyInLimits()  # still sent despite mention in pseudo-code above
                   PacketPokerPlayerArrive() # for client.serial
                   PacketPokerPlayerChips()  # for client.serial
                   PacketPokerSit()          # for client.serial
                   PacketPokerSeats()
               
-           If a table matching the criteria is *not* available, an empty
-           PacketPokerTable() will be sent (which matches the semantics of
-           certain PacketPokerTableJoin() fails, and that's why we do it
-           here.).
-
-           Thus, the key packet the client should look for after sending a
-           PacketPokerTablePicker is a PacketPokerTable().  That packet
-           will be empty if the picker couldn't find a seat matching the
-           criteria, and will be full of info if it was successful.
-
-           Even if a valid PacketPokerTable() is received, it's possible,
-           although unlikely, that the intervening operations --
+           Note even if a valid PacketPokerTable() is received, it's
+           possible, although unlikely, that the intervening operations --
            PacketPokerSeat(), PacketPokerBuyIn() and PacketPokerSit() --
            might fail.  If one of them fails, the client should expect to
            receive the normal errors it would receive if such an operation
diff --git a/poker-network/tests/test-pokeravatar.py.in b/poker-network/tests/test-pokeravatar.py.in
index 17c0005..2bb9dc9 100644
--- a/poker-network/tests/test-pokeravatar.py.in
+++ b/poker-network/tests/test-pokeravatar.py.in
@@ -1190,7 +1190,7 @@ class PokerAvatarTestCase(PokerAvatarTestCaseBaseClass):
             'table_picker' : { 'output':
                            "%sattempt to run table picker%s" % (messageStart, \
                             forPlayerByPlayer),
-                           'err_type' : PACKET_POKER_TABLE,
+                           'err_type' : PACKET_POKER_ERROR,
                             'packet' :
                                 PacketPokerTablePicker(serial = someoneElseSerial) },
             'rebuy' : { 'output':
@@ -2718,13 +2718,27 @@ class PokerAvatarTestCase(PokerAvatarTestCaseBaseClass):
         avatar.queuePackets()
         avatar.handlePacketLogic(PacketPokerTableJoin(serial = client.getSerial(),
                                                       game_id = gameId))
-        found = False
+        found = 0
         for packet in avatar.resetPacketsQueue():
+            # Note: the behavior below, tested in the "if", of returning
+            # an empty PacketTable() on error is deprecated, per
+            # documentation in pokerpackets.py.  This functionality is
+            # left in and supported for old clients.  The Error packet (in
+            # the elif below) is the proper way to test for an error
+            # return on PacketPokerTableJoin()
             if packet.type == PACKET_POKER_TABLE:
-                found = True
+                found += 1
                 self.assertEquals(packet.id, 0)
                 self.assertEquals(packet.name, "noname")
-        self.assertEquals(found, True)
+            elif packet.type == PACKET_POKER_ERROR:
+                found += 1
+                self.assertEquals(packet.serial, client.getSerial())
+                self.assertEquals(packet.game_id, 0)
+                self.assertEquals(packet.other_type, PACKET_POKER_TABLE_JOIN)
+                self.assertEquals(packet.code, PacketPokerTableJoin.GENERAL_FAILURE)
+                self.failUnless(len(packet.message) > 0,
+                                "some message should be included")
+        self.assertEquals(found, 2)
         return (client, packet)
     # ------------------------------------------------------------------------
     def test55_tableJoinWhenFails(self):
@@ -4041,7 +4055,7 @@ class PokerAvatarTablePickerTestCase(PokerAvatarTestCaseBaseClass):
         return (client, packet)
     # ------------------------------------------------------------------------
     def tablePickerFails(self, (client, packet), id, numPlayers = 0, currencySerial = 0,
-                         variant = '', structure = ''):
+                         variant = '', structure = '', gameId = 0):
         avatar = self.service.avatars[id]
         avatar.queuePackets()
         avatar.handlePacketLogic(PacketPokerTablePicker(serial = client.getSerial(),
@@ -4051,14 +4065,18 @@ class PokerAvatarTablePickerTestCase(PokerAvatarTestCaseBaseClass):
                                                         betting_structure = structure))
         found = 0
         for packet in avatar.resetPacketsQueue():
-            if packet.type == PACKET_POKER_TABLE:
-                found += 1
-                self.assertEquals(packet.id, 0)
-                self.assertEquals(packet.name, "noname")
+            found += 1
+            if packet.type == PACKET_POKER_ERROR:
+                self.assertEquals(packet.serial, client.getSerial())
+                self.assertEquals(packet.game_id, gameId)
+                self.assertEquals(packet.other_type, PACKET_POKER_TABLE_PICKER)
+                self.assertEquals(packet.code, PacketPokerTableJoin.GENERAL_FAILURE)
+                self.failUnless(len(packet.message) > 0,
+                                "some message should be included")
             else:
-                self.fail("Unexpected packet found when only empty PACKET_POKER_TABLE"
+                self.fail("Unexpected packet found when only PACKET_POKER_ERROR"
                           + " was expected: " + packet.__str__())
-        self.assertEquals(found, 1)
+        self.assertEquals(found, 1, "expected only PACKET_POKER_ERROR")
         return (client, packet)
     # -------------------------------------------------------------------------
     def tablePickerSucceeds(self, (client, packet), id, numPlayers, currencySerial,
@@ -4185,6 +4203,21 @@ class PokerAvatarTablePickerTestCase(PokerAvatarTestCaseBaseClass):
             self.assertEquals(minBuyInAmount, amountBoughtIn)
         return (client, packet)
     # ------------------------------------------------------------------------
+    def dummyTableCanAddPlayer(self, (client, packet), tableNumber):
+        table = self.service.getTable(tableNumber)
+        self.savedCanAddPlayer = table.game.canAddPlayer
+        def mockCanAddPlayer(serial):
+            return False
+        table.game.canAddPlayer = mockCanAddPlayer
+
+        return (client, packet)
+    # ------------------------------------------------------------------------
+    def restoreTableCanAddPlayer(self, (client, packet), tableNumber):
+        table = self.service.getTable(tableNumber)
+        table.game.canAddPlayer  = self.savedCanAddPlayer
+
+        return (client, packet)
+    # ------------------------------------------------------------------------
     def startPlayerSeatedAndPlaying(self, playerNumber, tableNumber, tableName,
                                     tableStructure, variant, maxPlayers, currencySerial):
         d = self.client_factory[playerNumber].established_deferred
@@ -4385,6 +4418,23 @@ class PokerAvatarTablePickerTestCase(PokerAvatarTestCaseBaseClass):
                                     "holdem", "NL HE 6-max 100/200")
         playersDeferreds.append(pickerDeferred)
         return defer.DeferredList(playersDeferreds)
+    # ------------------------------------------------------------------------
+    def test11_tablePicker_failureFromTableCannotAddPlayer(self):
+        self.createClients(4)
+
+        playersDeferreds = []
+        for ii in [ 0, 1, 2 ]:
+            playersDeferreds.append(
+                self.startPlayerSeatedAndPlaying(ii, 5, "Stud 8-max 2/4",
+                                             "2-4-limit", "7stud", 8, 2))
+        pickerDeferred = self.preparePlayerForTablePickerSend(3)
+        pickerDeferred.addCallback(self.setMoneyForPlayer, 3, 2, "over_min_under_best", 5)
+        pickerDeferred.addCallback(self.dummyTableCanAddPlayer, 5)
+        pickerDeferred.addCallback(self.tablePickerFails, 3, 3, 0,
+                                    "7stud", "2-4-limit", 5)
+        pickerDeferred.addCallback(self.restoreTableCanAddPlayer, 5)
+        playersDeferreds.append(pickerDeferred)
+        return defer.DeferredList(playersDeferreds)
 # -----------------------------------------------------------------------------
 class ConvertTablePickerArgsToListTablesQueryStringUnitTestCase(unittest.TestCase):
     """This coverage class exists to cover on specific method in avatar,
-- 

   -- bkuhn
_______________________________________________
Pokersource-users mailing list
[email protected]
https://mail.gna.org/listinfo/pokersource-users

Reply via email to