Reject extra compass letters on PBN deals

doc/dll-description.md

@@ -407,7 +407,7 @@ Common encodings are as follows
 <td><a name="Holding"></a>Holding</td><td colspan="2">A value of 16388 = 16384 + 4 is the encoding for the holding &#8220;A2&#8221; (ace and deuce).<br />The two lowest bits are always zero.</td>
 </tr>
 <tr><td colspan="3">&nbsp;</td></tr>
-<td><a name="PBN"></a>PBN</td><td colspan="2">Example:<br />W:T5.K4.652.A98542 K6.QJT976.QT7.Q6 432.A.AKJ93.JT73 AQJ987.8532.84.K</td>
+<td><a name="PBN"></a>PBN</td><td colspan="2">Example:<br />W:T5.K4.652.A98542 K6.QJT976.QT7.Q6 432.A.AKJ93.JT73 AQJ987.8532.84.K<br />Only the first hand has a compass letter (N/E/S/W); the other three hands follow clockwise and must not include additional directions.</td>
 </tr>
 </tbody>
 </table>

docs/python_interface.md

@@ -101,7 +101,7 @@ print(f"Tricks available: {result['score']}")
 Solves a single bridge deal using PBN (Portable Bridge Notation).
 
 **Parameters:**
-- `remain_cards` (str): PBN string (e.g., "N:AK.234.456.789TJQ W:QJ.AKQJ.789.234 E:T9.T9.TJ.AK S:8765.8765.AKQJ32.6")
+- `remain_cards` (str): PBN string (e.g., `"N:AK.234.456.789TJ T9432.T9.TJ2.AKQ 8765.8765.AKQ3.6 QJ.AKQJ.789.2345"`). Only the first hand has a compass letter; the other three follow clockwise with no seat prefixes.
 - `trump` (int, default=4): Trump suit (0-4)
 - `first` (int, default=0): Player to lead
 - `current_trick_suit` (tuple, default=(0,0,0)): Current trick suits
@@ -179,7 +179,7 @@ from dds3 import calc_all_tables_pbn
 
 deals = [
     "N:QJ6.K652.J85.T98 873.J97.AT764.Q4 K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3",
-    "N:AK.234.456.789TJQ W:QJ.AKQJ.789.234 E:T9.T9.TJ.AK S:8765.8765.AKQJ32.6",
+    "N:AK.234.456.789TJ T9432.T9.TJ2.AKQ 8765.8765.AKQ3.6 QJ.AKQJ.789.2345",
 ]
 
 result = calc_all_tables_pbn(deals, mode=0)
@@ -312,13 +312,16 @@ remain_cards = [
 ```
 
 ### PBN Format
-Portable Bridge Notation format: `"N:AK.234.456.789TJQ W:QJ.AKQJ.789.234 E:T9.T9.TJ.AK S:8765.8765.AKQJ32.6"`
+Portable Bridge Notation deal string. Only the first hand has a compass letter (`N`/`E`/`S`/`W`); the other three hands follow clockwise and must not include additional directions.
 
-Format: `[Seat]:[Spades].[Hearts].[Diamonds].[Clubs]`
-- Seats: N (North), E (East), S (South), W (West)
+Example: `"N:AK.234.456.789TJ T9432.T9.TJ2.AKQ 8765.8765.AKQ3.6 QJ.AKQJ.789.2345"`
+
+Format: `[Seat]:[Spades].[Hearts].[Diamonds].[Clubs] [next hand clockwise] ...`
+- First-hand seat: N (North), E (East), S (South), or W (West)
 - Cards: 2-9, T (10), J, Q, K, A (highest)
 - Dots separate suits
 - Omitted cards belong to other players
+- Extra compass letters on later hands are rejected
 
 ## Validation and Error Handling
 
@@ -328,7 +331,7 @@ The Python interface validates all inputs:
 - Rank values: 0 or 2-14 for trick cards (`0` means unset)
 - Card bitmasks: 0..0x7FFC
 - Array dimensions: 4x4 for card arrays, 5x4 for results
-- PBN format: Must be valid PBN notation
+- PBN format: Must be valid PBN notation (first-hand compass letter only)
 
 ### Exception Handling
 - `ValueError`: Invalid input parameters (bounds, format)

library/src/api/PBN.h

@@ -16,6 +16,7 @@
  * @brief Convert a PBN (Portable Bridge Notation) Deal string to DDS card array.
  *
  * Parses a PBN-format Deal string and fills the DDS card array with the remaining cards for each hand and suit.
+ * Only the first hand may have a compass letter (N/E/S/W); the other three hands follow clockwise and must not include additional directions.
  *
  * @param dealBuff PBN-format Deal string.
  * @param remainCards Output array for remaining cards per hand and suit.

library/src/api/dll.h

@@ -196,7 +196,7 @@ struct Deal
  * @param first The hand to play first
  * @param currentTrickSuit Suits of cards played in the current trick
  * @param currentTrickRank Ranks of cards played in the current trick
- * @param remainCards PBN string describing remaining cards
+ * @param remainCards PBN string describing remaining cards. Only the first hand may have a compass letter (N/E/S/W); later hands follow clockwise with no extra directions.
  */
 struct DealPBN
 {

library/src/pbn.cpp

@@ -9,26 +9,33 @@
 
 #include "pbn.hpp"
 #include <api/dds.h>
+#include <api/dll.h>
+
+constexpr int PbnBufferSize = static_cast<int>(sizeof(DealPBN::remainCards));
 
 auto is_card(const char cardChar) -> int;
+auto is_compass_letter(const char c) -> bool;
 
 
 auto convert_from_pbn(
   char const * dealBuff,
   unsigned int remainCards[DDS_HANDS][DDS_SUITS]) -> int
 {
+  if (remainCards == nullptr)
+    return 0;
+
   for (int h = 0; h < DDS_HANDS; h++)
     for (int s = 0; s < DDS_SUITS; s++)
       remainCards[h][s] = 0;
 
+  if (dealBuff == nullptr)
+    return 0;
+
   int bp = 0;
-  while (((dealBuff[bp] != 'W') && (dealBuff[bp] != 'N') &&
-          (dealBuff[bp] != 'E') && (dealBuff[bp] != 'S') &&
-          (dealBuff[bp] != 'w') && (dealBuff[bp] != 'n') &&
-          (dealBuff[bp] != 'e') && (dealBuff[bp] != 's')) && (bp < 3))
+  while ((bp < 3) && (dealBuff[bp] != '\0') && !is_compass_letter(dealBuff[bp]))
     bp++;
 
-  if (bp >= 3)
+  if ((bp >= 3) || (dealBuff[bp] == '\0') || (dealBuff[bp + 1] != ':'))
     return 0;
 
   int first;
@@ -48,11 +55,14 @@ auto convert_from_pbn(
   int suitInHand = 0;
   int card, hand;
 
-  while ((bp < 80) && (dealBuff[bp] != '\0'))
+  while ((bp < PbnBufferSize) && (dealBuff[bp] != '\0'))
   {
     card = is_card(dealBuff[bp]);
     if (card)
     {
+      if (hand_rel_first >= DDS_HANDS || suitInHand >= DDS_SUITS)
+        return 0;
+
       switch (first)
       {
         case 0:
@@ -81,23 +91,60 @@ auto convert_from_pbn(
             hand = hand_rel_first - 1;
       }
 
+      if (hand < 0 || hand >= DDS_HANDS)
+        return 0;
+
       remainCards[hand][suitInHand] |=
         static_cast<unsigned>((bit_map_rank[card] << 2));
 
     }
     else if (dealBuff[bp] == '.')
+    {
+      if (suitInHand >= DDS_SUITS - 1)
+        return 0;
       suitInHand++;
+    }
     else if (dealBuff[bp] == ' ')
     {
+      if (hand_rel_first >= DDS_HANDS - 1)
+        return 0;
       hand_rel_first++;
       suitInHand = 0;
     }
+    else if (is_compass_letter(dealBuff[bp]))
+      return 0;
     bp++;
   }
+
+  if (bp >= PbnBufferSize)
+    return 0;
+
+  if (hand_rel_first != DDS_HANDS - 1)
+    return 0;
+
   return RETURN_NO_FAULT;
 }
 
 
+auto is_compass_letter(const char c) -> bool
+{
+  switch (c)
+  {
+    case 'N':
+    case 'n':
+    case 'E':
+    case 'e':
+    case 'S':
+    case 's':
+    case 'W':
+    case 'w':
+      return true;
+    default:
+      return false;
+  }
+}
+
+
 auto is_card(const char cardChar) -> int
 {
   switch (cardChar)

library/src/pbn.hpp

@@ -16,6 +16,7 @@
  * @brief Convert a PBN (Portable Bridge Notation) Deal string to DDS card array.
  *
  * Parses a PBN-format Deal string and fills the DDS card array with the remaining cards for each hand and suit.
+ * Only the first hand may have a compass letter (N/E/S/W); the other three hands follow clockwise and must not include additional directions.
  *
  * @param dealBuff PBN-format Deal string.
  * @param remainCards Output array for remaining cards per hand and suit.

library/tests/BUILD.bazel

@@ -17,6 +17,7 @@ filegroup(
             "parse_par_test.cpp",  # Uses GoogleTest, compiled separately
             "loop_par_test.cpp",  # Uses GoogleTest, compiled separately
             "dds_c_api_test.cpp",  # Uses GoogleTest, compiled separately
+            "pbn_test.cpp",  # Uses GoogleTest, compiled separately
         ],
     ),
 )
@@ -151,6 +152,20 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "pbn_test",
+    srcs = ["pbn_test.cpp"],
+    size = "small",
+    copts = DDS_CPPOPTS,
+    linkopts = DDS_LINKOPTS,
+    local_defines = DDS_LOCAL_DEFINES,
+    deps = [
+        "//library/src:testable_dds",
+        "//library/src/api:api_definitions",
+        "@googletest//:gtest_main",
+    ],
+)
+
 cc_test(
     name = "parse_par_test",
     srcs = [

library/tests/README.md

@@ -10,7 +10,7 @@ The test cases are located in the `hands` directory. Each `.txt` file in this di
 
 The input files use a specific format with keywords to define the test parameters for each deal. The first line is `NUMBER N`, where `N` is the count of deals in the file. Each deal is then a block of lines in this order:
 
-- **`PBN`**: Deal header with four integer fields — dealer, vulnerability, trump, and leader — followed by a quoted PBN `remainCards` string listing the cards held by each player (North, East, South, West).
+- **`PBN`**: Deal header with four integer fields — dealer, vulnerability, trump, and leader — followed by a quoted PBN `remainCards` string. That string starts with one compass letter (`N`/`E`/`S`/`W`) for the first hand; the other three hands follow clockwise with no further seat letters.
 - **`FUT`**: Expected `SolveBoard` future-tricks result for the deal (card count plus suit/rank/equals/score arrays).
 - **`TABLE`**: Expected double-dummy table: 20 integers, `res_table[strain][hand]` for 5 strains (♠♥♦♣NT) × 4 seats (N/E/S/W).
 - **`PAR`**: Expected par scores and contract strings for NS and EW views.

library/tests/pbn_test.cpp

@@ -0,0 +1,150 @@
+/// @file pbn_test.cpp
+/// @brief Unit tests for convert_from_pbn deal-string parsing.
+
+#include <gtest/gtest.h>
+
+#include <string>
+
+#include <api/PBN.h>
+#include <api/dll.h>
+
+namespace
+{
+
+constexpr char kNorthFirst[] =
+    "N:QJ6.K652.J85.T98 873.J97.AT764.Q4 K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3";
+
+constexpr char kEastFirst[] =
+    "E:QJT5432.T.6.QJ82 .J97543.K7532.94 87.A62.QJT4.AT75 AK96.KQ8.A98.K63";
+
+auto convert(const char* pbn) -> int
+{
+  unsigned int remain[DDS_HANDS][DDS_SUITS]{};
+  return convert_from_pbn(pbn, remain);
+}
+
+}  // namespace
+
+TEST(ConvertFromPbn, AcceptsNorthFirstWithoutLaterDirections)
+{
+  unsigned int remain[DDS_HANDS][DDS_SUITS]{};
+  EXPECT_EQ(convert_from_pbn(kNorthFirst, remain), RETURN_NO_FAULT);
+  EXPECT_NE(remain[0][0], 0u);
+}
+
+TEST(ConvertFromPbn, AcceptsEastFirstWithoutLaterDirections)
+{
+  EXPECT_EQ(convert(kEastFirst), RETURN_NO_FAULT);
+}
+
+TEST(ConvertFromPbn, AcceptsLowercaseFirstHandDirection)
+{
+  EXPECT_EQ(
+      convert(
+          "n:QJ6.K652.J85.T98 873.J97.AT764.Q4 K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3"),
+      RETURN_NO_FAULT);
+}
+
+TEST(ConvertFromPbn, RejectsClockwiseSeatLettersOnLaterHands)
+{
+  EXPECT_EQ(
+      convert(
+          "N:QJ6.K652.J85.T98 E:873.J97.AT764.Q4 S:K5.T83.KQ9.A7652 "
+          "W:AT942.AQ4.32.KJ3"),
+      0);
+}
+
+TEST(ConvertFromPbn, RejectsASingleExtraSeatLetter)
+{
+  EXPECT_EQ(
+      convert(
+          "N:QJ6.K652.J85.T98 W:873.J97.AT764.Q4 K5.T83.KQ9.A7652 "
+          "AT942.AQ4.32.KJ3"),
+      0);
+}
+
+TEST(ConvertFromPbn, RejectsLowercaseExtraSeatLetter)
+{
+  EXPECT_EQ(
+      convert(
+          "N:QJ6.K652.J85.T98 e:873.J97.AT764.Q4 K5.T83.KQ9.A7652 "
+          "AT942.AQ4.32.KJ3"),
+      0);
+}
+
+TEST(ConvertFromPbn, RejectsNullPointerDealBuffer)
+{
+  unsigned int remain[DDS_HANDS][DDS_SUITS]{};
+  EXPECT_EQ(convert_from_pbn(nullptr, remain), 0);
+}
+
+TEST(ConvertFromPbn, ClearsOutputOnNullDealBuffer)
+{
+  unsigned int remain[DDS_HANDS][DDS_SUITS]{};
+  remain[0][0] = 0xFFFF;
+  EXPECT_EQ(convert_from_pbn(nullptr, remain), 0);
+  EXPECT_EQ(remain[0][0], 0u);
+}
+
+TEST(ConvertFromPbn, ClearsOutputOnInvalidDeal)
+{
+  unsigned int remain[DDS_HANDS][DDS_SUITS]{};
+  remain[0][0] = 0xFFFF;
+  EXPECT_EQ(convert_from_pbn("xx", remain), 0);
+  EXPECT_EQ(remain[0][0], 0u);
+}
+
+TEST(ConvertFromPbn, RejectsNullOutputBuffer)
+{
+  EXPECT_EQ(convert_from_pbn(kNorthFirst, nullptr), 0);
+}
+
+TEST(ConvertFromPbn, RejectsEmptyAndMissingSeatPrefixInputs)
+{
+  EXPECT_EQ(convert(""), 0);
+  EXPECT_EQ(convert("N"), 0);
+  EXPECT_EQ(convert("xx"), 0);
+}
+
+TEST(ConvertFromPbn, RejectsTooManySuitsInHand)
+{
+  EXPECT_EQ(convert("N:AK.K.K.K.A"), 0);
+}
+
+TEST(ConvertFromPbn, RejectsTooManyHands)
+{
+  unsigned int remain[DDS_HANDS][DDS_SUITS]{};
+  EXPECT_EQ(convert_from_pbn("N:AK.K.K.K    A", remain), 0);
+}
+
+TEST(ConvertFromPbn, RejectsTruncatedDealWithFewerThanFourHands)
+{
+  EXPECT_EQ(convert("N:AK.QJ.T9.876"), 0);         // 1 hand
+  EXPECT_EQ(convert("N:AK.QJ.T9.876 AK.QJ.T9.876"), 0); // 2 hands
+  EXPECT_EQ(convert("N:AK.QJ.T9.876 AK.QJ.T9.876 AK.QJ.T9.876"), 0); // 3 hands
+}
+
+TEST(ConvertFromPbn, RejectsInputLongerThanRemainCardsBuffer)
+{
+  constexpr auto kBufSize = sizeof(DealPBN::remainCards);
+  std::string pbn = "N:";
+  pbn.append(kBufSize, 'A');
+  ASSERT_GT(pbn.size(), kBufSize);
+  EXPECT_EQ(convert(pbn.c_str()), 0);
+}
+
+TEST(ConvertFromPbn, RejectsInputExactlyAtRemainCardsBufferLimit)
+{
+  constexpr auto kBufSize = sizeof(DealPBN::remainCards);
+  std::string pbn = "N:";
+  pbn.append(kBufSize - 2, 'A');
+  ASSERT_EQ(pbn.size(), kBufSize);
+  EXPECT_EQ(convert(pbn.c_str()), 0);
+}
+
+TEST(ConvertFromPbn, AcceptsInputThatFitsRemainCardsBuffer)
+{
+  constexpr auto kBufSize = sizeof(DealPBN::remainCards);
+  ASSERT_LT(std::char_traits<char>::length(kNorthFirst), kBufSize);
+  EXPECT_EQ(convert(kNorthFirst), RETURN_NO_FAULT);
+}