diff --git a/doc/dll-description.md b/doc/dll-description.md index 8e09f1ea1..bce60b33e 100644 --- a/doc/dll-description.md +++ b/doc/dll-description.md @@ -407,7 +407,7 @@ Common encodings are as follows HoldingA value of 16388 = 16384 + 4 is the encoding for the holding “A2” (ace and deuce).
The two lowest bits are always zero.   -PBNExample:
W:T5.K4.652.A98542 K6.QJT976.QT7.Q6 432.A.AKJ93.JT73 AQJ987.8532.84.K +PBNExample:
W:T5.K4.652.A98542 K6.QJT976.QT7.Q6 432.A.AKJ93.JT73 AQJ987.8532.84.K
Only the first hand has a compass letter (N/E/S/W); the other three hands follow clockwise and must not include additional directions. diff --git a/docs/python_interface.md b/docs/python_interface.md index cc2401b2c..f7041486f 100644 --- a/docs/python_interface.md +++ b/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) diff --git a/library/src/api/PBN.h b/library/src/api/PBN.h index ad07351a5..994c84a4e 100644 --- a/library/src/api/PBN.h +++ b/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. diff --git a/library/src/api/dll.h b/library/src/api/dll.h index f6073a359..02cef5f8e 100644 --- a/library/src/api/dll.h +++ b/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 { diff --git a/library/src/pbn.cpp b/library/src/pbn.cpp index eb3e40a4a..4ec56fec7 100644 --- a/library/src/pbn.cpp +++ b/library/src/pbn.cpp @@ -9,26 +9,33 @@ #include "pbn.hpp" #include +#include + +constexpr int PbnBufferSize = static_cast(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((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) diff --git a/library/src/pbn.hpp b/library/src/pbn.hpp index ad07351a5..994c84a4e 100644 --- a/library/src/pbn.hpp +++ b/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. diff --git a/library/tests/BUILD.bazel b/library/tests/BUILD.bazel index cf8c37293..ff30e5092 100644 --- a/library/tests/BUILD.bazel +++ b/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 = [ diff --git a/library/tests/README.md b/library/tests/README.md index 7ec764fb4..98e4b4605 100644 --- a/library/tests/README.md +++ b/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. diff --git a/library/tests/pbn_test.cpp b/library/tests/pbn_test.cpp new file mode 100644 index 000000000..afe843701 --- /dev/null +++ b/library/tests/pbn_test.cpp @@ -0,0 +1,150 @@ +/// @file pbn_test.cpp +/// @brief Unit tests for convert_from_pbn deal-string parsing. + +#include + +#include + +#include +#include + +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::length(kNorthFirst), kBufSize); + EXPECT_EQ(convert(kNorthFirst), RETURN_NO_FAULT); +} diff --git a/python/src/bindings.cpp b/python/src/bindings.cpp index 0d103285c..12892a55b 100644 --- a/python/src/bindings.cpp +++ b/python/src/bindings.cpp @@ -195,7 +195,8 @@ auto register_solve_bindings(py::module_& module) -> void py::arg("context") = py::none(), "Solve a single bridge deal from PBN (Portable Bridge Notation) format.\n\n" "Args:\n" - " remain_cards (str): Remaining cards in PBN format (e.g., 'N:AK.234.456.789T...').\n" + " remain_cards (str): Remaining cards in PBN format (e.g., 'N:AK.234.456.789T...'). " + "Only the first hand may have a compass letter; later hands follow clockwise.\n" " trump (int, optional): Trump suit (0=♠, 1=♥, 2=♦, 3=♣, 4=NT). Default: 4\n" " first (int, optional): Seat that plays first (0=N, 1=E, 2=S, 3=W). Default: 0\n" " current_trick_suit (tuple, optional): Suits in current trick (3-tuple of ints, 0-3). Default: (0, 0, 0)\n" @@ -264,7 +265,8 @@ auto register_solve_bindings(py::module_& module) -> void "Solve multiple bridge deals in PBN format.\n\n" "Args:\n" " boards (list): List of board dicts, each with:\n" - " remain_cards (str): Remaining cards in PBN format (e.g., 'N:AK.234.456.789T...').\n" + " remain_cards (str): Remaining cards in PBN format (e.g., 'N:AK.234.456.789T...'). " + "Only the first hand may have a compass letter; later hands follow clockwise.\n" " trump (int, optional): Trump suit (0=♠, 1=♥, 2=♦, 3=♣, 4=NT). Default: 4\n" " first (int, optional): Seat that plays first (0=N, 1=E, 2=S, 3=W). Default: 0\n" " current_trick_suit (tuple, optional): Suits in current trick. Default: (0, 0, 0)\n" diff --git a/python/tests/README.md b/python/tests/README.md index aace6eb3f..41f71436e 100644 --- a/python/tests/README.md +++ b/python/tests/README.md @@ -76,6 +76,8 @@ pytest python/tests/test_solve_board.py::TestSolveBoard::test_solve_board_basic - **TestPBNConversions**: PBN string parsing validation - Valid PBN format acceptance - Invalid seat designations + - Extra compass letters on later hands rejected + - Raises an error (validation fails; does not continue) - Truncated/empty string handling - **TestTrumpFilterValidation**: Trump filter parameter bounds diff --git a/python/tests/test_type_conversions.py b/python/tests/test_type_conversions.py index 8d13b64fe..95ccb5c19 100644 --- a/python/tests/test_type_conversions.py +++ b/python/tests/test_type_conversions.py @@ -198,6 +198,21 @@ def test_pbn_truncated(self) -> None: pbn = "N:AK.234.456.789TJQ W:QJ.AKQJ" # Incomplete assert_raises((ValueError, RuntimeError), solve_board_pbn, pbn) + def test_pbn_extra_seat_letters_rejected(self) -> None: + """Later hands may not include compass letters; only the first hand may.""" + pbn = ( + "N:QJ6.K652.J85.T98 E:873.J97.AT764.Q4 " + "S:K5.T83.KQ9.A7652 W:AT942.AQ4.32.KJ3" + ) + assert_raises((ValueError, RuntimeError), solve_board_pbn, pbn) + + def test_pbn_single_extra_seat_letter_rejected(self) -> None: + pbn = ( + "N:QJ6.K652.J85.T98 W:873.J97.AT764.Q4 " + "K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3" + ) + assert_raises((ValueError, RuntimeError), solve_board_pbn, pbn) + class TestTrumpFilterValidation(unittest.TestCase): """Tests for trump_filter validation in calc_all_tables_pbn."""