diff --git a/docs/jni_interface.md b/docs/jni_interface.md index f4fd8cd3..0bcc464e 100644 --- a/docs/jni_interface.md +++ b/docs/jni_interface.md @@ -77,9 +77,11 @@ and ctypes can all bind to: The shim covers the modern API's full surface: context lifecycle (default and config-based creation, destroy), `dds_c_solve_board`, `dds_c_calc_dd_table` and -its `_pbn` twin, `dds_c_calc_par`, transposition-table configure/resize/clear, -both resets, and the logging passthroughs. The Java bindings here use a subset; -the .NET binding uses all of it ([dotnet_interface.md](dotnet_interface.md)). +its `_pbn` twin, `dds_c_calc_par`, `dds_c_convert_from_pbn`, +transposition-table configure/resize/clear, both resets, and the logging +passthroughs. The Java bindings here use a subset; the .NET binding uses all of +it except `dds_c_convert_from_pbn` +([dotnet_interface.md](dotnet_interface.md)). The flat legacy C API from `dll.h` (`SolveBoard`, `CalcDDtable`, `GetDDSInfo`, `ErrorMessage`, …) is exported unchanged and is also callable from FFM. @@ -93,8 +95,8 @@ non-hermetic early-access binaries, and the generated output is just plain exposes: - Public `MemoryLayout` constants matching the C structs: `DEAL`, - `FUTURE_TRICKS`, `DD_TABLE_DEAL`, `DD_TABLE_RESULTS`, `PAR_RESULTS`, - `DDS_INFO`. + `FUTURE_TRICKS`, `DD_TABLE_DEAL`, `DD_TABLE_DEAL_PBN`, `DD_TABLE_RESULTS`, + `PAR_RESULTS`, `DDS_INFO`. - Typed downcall wrappers for the shim functions plus `getDdsInfo`. These return an `int` [`DdsStatus`](../jni/java/org/dds/ffm/DdsStatus.java) `RETURN_*` code (`RETURN_NO_FAULT == 1` on success); compare against the named diff --git a/jni/exported_symbols.lds b/jni/exported_symbols.lds index 085f763f..4807a323 100644 --- a/jni/exported_symbols.lds +++ b/jni/exported_symbols.lds @@ -42,9 +42,9 @@ _SolveBoardPBN _dds_c_calc_dd_table _dds_c_calc_dd_table_pbn _dds_c_calc_par -_dds_c_calc_par_pbn _dds_c_clear_tt _dds_c_configure_tt +_dds_c_convert_from_pbn _dds_c_convert_to_dealer_text_format _dds_c_convert_to_sides_text_format _dds_c_create_solvercontext @@ -63,4 +63,3 @@ _dds_c_resize_tt _dds_c_sides_par _dds_c_sides_par_bin _dds_c_solve_board -_dds_c_solve_board_pbn diff --git a/jni/java/org/dds/ffm/Dds.java b/jni/java/org/dds/ffm/Dds.java index 9f6dc636..b01e9456 100644 --- a/jni/java/org/dds/ffm/Dds.java +++ b/jni/java/org/dds/ffm/Dds.java @@ -78,6 +78,11 @@ public class Dds implements AutoCloseable { MemoryLayout.sequenceLayout(16, JAVA_INT).withName("cards")) .withName("DdTableDeal"); + /** struct DdTableDealPBN — cards is a NUL-terminated PBN deal string. */ + public static final MemoryLayout DD_TABLE_DEAL_PBN = MemoryLayout.structLayout( + MemoryLayout.sequenceLayout(80, JAVA_BYTE).withName("cards")) + .withName("DdTableDealPBN"); + /** struct DdTableResults — res_table[DDS_STRAINS][DDS_HANDS] = 5x4. */ public static final MemoryLayout DD_TABLE_RESULTS = MemoryLayout.structLayout( MemoryLayout.sequenceLayout(20, JAVA_INT).withName("resTable")) @@ -114,6 +119,8 @@ public class Dds implements AutoCloseable { private final MethodHandle solveBoard; private final MethodHandle calcDdTable; private final MethodHandle calcPar; + private final MethodHandle calcDdTablePbn; + private final MethodHandle convertFromPbn; private Dds(Arena arena, SymbolLookup lookup) { this.arena = arena; @@ -130,6 +137,10 @@ private Dds(Arena arena, SymbolLookup lookup) { FunctionDescriptor.of(JAVA_INT, ADDRESS, ADDRESS, ADDRESS)); this.calcPar = handle(linker, lookup, "dds_c_calc_par", FunctionDescriptor.of(JAVA_INT, ADDRESS, ADDRESS, JAVA_INT, ADDRESS, ADDRESS)); + this.calcDdTablePbn = handle(linker, lookup, "dds_c_calc_dd_table_pbn", + FunctionDescriptor.of(JAVA_INT, ADDRESS, ADDRESS, ADDRESS)); + this.convertFromPbn = handle(linker, lookup, "dds_c_convert_from_pbn", + FunctionDescriptor.of(JAVA_INT, ADDRESS, ADDRESS)); } private static MethodHandle handle(Linker linker, SymbolLookup lookup, String name, @@ -281,6 +292,50 @@ public int calcPar(MemorySegment ctx, MemorySegment deal, int vulnerable, } } + /** Compute the double dummy table for a PBN-format deal. Returns a {@link DdsStatus} {@code RETURN_*} code. */ + public int calcDdTablePbn(MemorySegment ctx, MemorySegment dealPbn, MemorySegment results) { + try { + return (int) calcDdTablePbn.invoke(ctx, dealPbn, results); + } catch (Throwable t) { + throw rethrow(t); + } + } + + /** + * Parse a NUL-terminated PBN deal string into a binary holdings block: 16 + * consecutive ints, row-major {@code [hand][suit]}, as laid out by + * {@link #DEAL}'s {@code remainCards} and {@link #DD_TABLE_DEAL}'s + * {@code cards}. Point {@code cards} at either field (or a slice of it) and + * then use the binary entry points; this is the general PBN path, which is + * why solve and par have no PBN variant here. The terminator must occur + * within the first 80 bytes of {@code pbnDeal} (79 content bytes plus + * terminator fits exactly). Needs no solver context. + * Returns a {@link DdsStatus} {@code RETURN_*} code. + * + *

{@code RETURN_NO_FAULT} means the string parsed, not that it describes + * a legal deal: an unrecognized rank is skipped rather than refused, so a + * typo'd rank yields a short hand and still converts — except a compass + * letter (N/E/S/W, either case) found after the first hand, which IS + * refused with {@code RETURN_PBN_FAULT} rather than skipped. The solve and + * table calls validate the deal and return {@code RETURN_CARD_COUNT} or + * {@code RETURN_DUPLICATE_CARDS}, so check their status too. + * + *

On {@code RETURN_PBN_FAULT} (a rejected string that reached the native + * parser) {@code cards} is not preserved — it is zeroed before parsing + * begins and may hold a partial parse. Convert into scratch storage if the + * destination must survive a bad string. This does not apply to a failure + * that never reaches the parser — a {@code RETURN_UNKNOWN_FAULT} from a + * {@code NULL} segment, or an exception thrown by this call before the + * native invocation completes — where {@code cards} is left untouched. + */ + public int convertFromPbn(MemorySegment pbnDeal, MemorySegment cards) { + try { + return (int) convertFromPbn.invoke(pbnDeal, cards); + } catch (Throwable t) { + throw rethrow(t); + } + } + @Override public void close() { arena.close(); diff --git a/jni/java/org/dds/ffm/DdsSmokeTest.java b/jni/java/org/dds/ffm/DdsSmokeTest.java index 072b19a5..6cf7bb35 100644 --- a/jni/java/org/dds/ffm/DdsSmokeTest.java +++ b/jni/java/org/dds/ffm/DdsSmokeTest.java @@ -63,6 +63,18 @@ public final class DdsSmokeTest { // parScore is char[2][16] (NS entry then EW); we read only the NS entry. private static final long PAR_SCORE_NS_LEN = Dds.PAR_RESULTS.select(PathElement.groupElement("parScore")).byteSize() / 2; + private static final long DEAL_REMAIN_LEN = + Dds.DEAL.select(PathElement.groupElement("remainCards")).byteSize(); + private static final long DTDP_CARDS = + Dds.DD_TABLE_DEAL_PBN.byteOffset(PathElement.groupElement("cards")); + private static final long DTDP_CARDS_LEN = + Dds.DD_TABLE_DEAL_PBN.select(PathElement.groupElement("cards")).byteSize(); + + // Same reference board as checkSolveKnownDeal/checkCalcDdTable, in PBN + // format. Matches kReferencePbn in library/tests/dds_c_api_test.cpp so the + // JVM and C++ bindings agree on one fixture. + private static final String REFERENCE_PBN = + "N:AKQJT98765432... .AKQJT98765432.. ..AKQJT98765432. ...AKQJT98765432"; public static void main(String[] args) throws Exception { Path library = locateLibrary(); @@ -74,6 +86,9 @@ public static void main(String[] args) throws Exception { checkSolveRejectsInvalidDeal(dds, arena); checkCalcDdTable(dds, arena); checkCalcPar(dds, arena); + checkCalcDdTablePbn(dds, arena); + checkConvertFromPbn(dds, arena); + checkConvertFromPbnRejectsMalformedString(dds, arena); } System.out.println("DDS FFM smoke test passed."); } @@ -200,6 +215,68 @@ private static void checkCalcPar(Dds dds, Arena arena) { } } + private static void checkCalcDdTablePbn(Dds dds, Arena arena) { + MemorySegment tableDealPbn = arena.allocate(Dds.DD_TABLE_DEAL_PBN); + tableDealPbn.fill((byte) 0); + writeCString(tableDealPbn, DTDP_CARDS, DTDP_CARDS_LEN, REFERENCE_PBN); + + MemorySegment ctx = dds.createSolverContext(); + try { + MemorySegment results = arena.allocate(Dds.DD_TABLE_RESULTS); + int rc = dds.calcDdTablePbn(ctx, tableDealPbn, results); + check(rc == RETURN_NO_FAULT, "dds_c_calc_dd_table_pbn returned " + rc); + + for (int i = 0; i < EXPECTED_DD_TABLE.length; i++) { + int got = results.get(JAVA_INT, DTR_RES_TABLE + (long) i * Integer.BYTES); + check(got == EXPECTED_DD_TABLE[i], + "pbn resTable[" + i + "] expected " + EXPECTED_DD_TABLE[i] + ", got " + got); + } + System.out.println("calc_dd_table_pbn: 5x4 table matches expected."); + } finally { + dds.destroySolverContext(ctx); + } + } + + private static void checkConvertFromPbn(Dds dds, Arena arena) { + // The general PBN path for bindings, and the replacement for the + // withdrawn dds_c_solve_board_pbn: parse the PBN string straight into a + // binary Deal's remainCards, set the already-binary fields, then use the + // binary solve. Must reproduce checkSolveKnownDeal's 13 tricks. + MemorySegment deal = arena.allocate(Dds.DEAL); + deal.fill((byte) 0); + deal.set(JAVA_INT, DEAL_TRUMP, 0); // trump = spades + deal.set(JAVA_INT, DEAL_FIRST, 0); // first = North + + MemorySegment pbn = arena.allocateFrom(REFERENCE_PBN); + int convertRc = dds.convertFromPbn(pbn, deal.asSlice(DEAL_REMAIN, DEAL_REMAIN_LEN)); + check(convertRc == RETURN_NO_FAULT, "dds_c_convert_from_pbn returned " + convertRc); + + MemorySegment ctx = dds.createSolverContext(); + check(!ctx.equals(MemorySegment.NULL), "createSolverContext returned NULL"); + try { + MemorySegment fut = arena.allocate(Dds.FUTURE_TRICKS); + int rc = dds.solveBoard(ctx, deal, -1, 1, 1, fut); + check(rc == RETURN_NO_FAULT, "solve_board on converted deal returned " + rc); + + int topScore = fut.get(JAVA_INT, FT_SCORE); + System.out.println("convert_from_pbn + solve_board: score[0]=" + topScore); + check(topScore == 13, "expected 13 tricks, got " + topScore); + } finally { + dds.destroySolverContext(ctx); + } + } + + private static void checkConvertFromPbnRejectsMalformedString(Dds dds, Arena arena) { + MemorySegment cards = arena.allocate(Dds.DD_TABLE_DEAL); + MemorySegment pbn = arena.allocateFrom("xx"); + int rc = dds.convertFromPbn(pbn, cards); + System.out.println("convert_from_pbn(malformed): rc=" + rc + " (" + DdsStatus.name(rc) + ")"); + // Pin the mapped code, not just "not success": turning the parser's bare + // 0 into RETURN_PBN_FAULT is the shim's only added behavior here. + check(rc == DdsStatus.RETURN_PBN_FAULT, + "expected RETURN_PBN_FAULT, got " + DdsStatus.name(rc)); + } + private static void setRemain(MemorySegment deal, int hand, int suit, int holding) { // remainCards[hand][suit], row-major with DDS_SUITS = 4 columns. setHolding(deal, DEAL_REMAIN, hand, suit, holding); @@ -210,6 +287,20 @@ private static void setHolding(MemorySegment struct, long base, int hand, int su struct.set(JAVA_INT, base + (long) (hand * 4 + suit) * Integer.BYTES, holding); } + private static void writeCString(MemorySegment struct, long offset, long fieldLength, String value) { + // Write value as ASCII into a fixed-size char[] field, zero-padding the + // remainder so it stays NUL-terminated for readCString/native use. + byte[] bytes = value.getBytes(java.nio.charset.StandardCharsets.US_ASCII); + if (bytes.length >= fieldLength) { + throw new IllegalArgumentException( + "value does not fit in " + fieldLength + "-byte field (with NUL terminator): " + value); + } + for (long i = 0; i < fieldLength; i++) { + byte b = i < bytes.length ? bytes[(int) i] : 0; + struct.set(JAVA_BYTE, offset + i, b); + } + } + private static String readCString(MemorySegment struct, long offset, long maxLength) { // Read a fixed-size char[] field as a NUL-terminated string. Bound the // scan to the field length (clamped to the segment) so a missing diff --git a/jni/version_script.lds b/jni/version_script.lds index 7c747c87..b1e36137 100644 --- a/jni/version_script.lds +++ b/jni/version_script.lds @@ -44,9 +44,9 @@ dds_c_calc_dd_table; dds_c_calc_dd_table_pbn; dds_c_calc_par; - dds_c_calc_par_pbn; dds_c_clear_tt; dds_c_configure_tt; + dds_c_convert_from_pbn; dds_c_convert_to_dealer_text_format; dds_c_convert_to_sides_text_format; dds_c_create_solvercontext; @@ -65,7 +65,6 @@ dds_c_sides_par; dds_c_sides_par_bin; dds_c_solve_board; - dds_c_solve_board_pbn; local: *; }; diff --git a/library/src/api/dds_api.hpp b/library/src/api/dds_api.hpp index b7ca197b..8751a3f8 100644 --- a/library/src/api/dds_api.hpp +++ b/library/src/api/dds_api.hpp @@ -46,13 +46,6 @@ extern "C" { int mode, FutureTricks* futp) -> int; - EXTERN_C DLLEXPORT auto dds_solve_board_pbn(DDS_SOLVER_CTX ctx, - const DealPBN& dlpbn, - int target, - int solutions, - int mode, - FutureTricks* futp) -> int; - EXTERN_C DLLEXPORT auto dds_calc_dd_table( DDS_SOLVER_CTX ctx, const DdTableDeal& table_deal, @@ -71,12 +64,5 @@ extern "C" { DdTableResults* table_results, ParResults* par_results) -> int; - EXTERN_C DLLEXPORT auto dds_calc_par_pbn( - DDS_SOLVER_CTX ctx, - const DdTableDealPBN& table_deal_pbn, - int vulnerable, - DdTableResults* table_results, - ParResults* par_results) -> int; - -} +} diff --git a/library/src/api/dds_c_api.cpp b/library/src/api/dds_c_api.cpp index 75e51d5a..be7cdc22 100644 --- a/library/src/api/dds_c_api.cpp +++ b/library/src/api/dds_c_api.cpp @@ -19,6 +19,7 @@ #include #include #include /* legacy Par/SidesPar/DealerPar/.../GetDDSInfo/ErrorMessage */ +#include /* convert_from_pbn */ /* This shim is the boundary between the C++ implementation and pure-C FFI consumers (JVM/FFM, .NET, ctypes). Two things must never cross it: @@ -69,22 +70,6 @@ DLLEXPORT int dds_c_solve_board(DDS_C_SOLVER_CTX ctx, } } -DLLEXPORT int dds_c_solve_board_pbn(DDS_C_SOLVER_CTX ctx, - const struct DealPBN* dlpbn, - int target, int solutions, int mode, - struct FutureTricks* futp) -{ - if (ctx == nullptr || dlpbn == nullptr || futp == nullptr) - return RETURN_UNKNOWN_FAULT; - - try { - return dds_solve_board_pbn(static_cast(ctx), - *dlpbn, target, solutions, mode, futp); - } catch (...) { - return RETURN_UNKNOWN_FAULT; - } -} - DLLEXPORT int dds_c_calc_dd_table(DDS_C_SOLVER_CTX ctx, const struct DdTableDeal* deal, struct DdTableResults* results) @@ -116,23 +101,6 @@ DLLEXPORT int dds_c_calc_par(DDS_C_SOLVER_CTX ctx, } } -DLLEXPORT int dds_c_calc_par_pbn(DDS_C_SOLVER_CTX ctx, - const struct DdTableDealPBN* deal, - int vulnerable, - struct DdTableResults* results, - struct ParResults* par) -{ - if (ctx == nullptr || deal == nullptr || results == nullptr || par == nullptr) - return RETURN_UNKNOWN_FAULT; - - try { - return dds_calc_par_pbn(static_cast(ctx), - *deal, vulnerable, results, par); - } catch (...) { - return RETURN_UNKNOWN_FAULT; - } -} - DLLEXPORT DDS_C_SOLVER_CTX dds_c_create_solvercontext(int tt_kind, int def_mb, int max_mb) { @@ -162,6 +130,25 @@ DLLEXPORT int dds_c_calc_dd_table_pbn(DDS_C_SOLVER_CTX ctx, } } +DLLEXPORT int dds_c_convert_from_pbn(const char* pbn_deal, + unsigned int cards[DDS_HANDS][DDS_SUITS]) +{ + if (pbn_deal == nullptr || cards == nullptr) + return RETURN_UNKNOWN_FAULT; + + try { + /* convert_from_pbn reports success as RETURN_NO_FAULT but failure as a + bare 0, which is not a RETURN_* code; map it to RETURN_PBN_FAULT so + every status this shim returns is one. */ + if (convert_from_pbn(pbn_deal, cards) != RETURN_NO_FAULT) + return RETURN_PBN_FAULT; + + return RETURN_NO_FAULT; + } catch (...) { + return RETURN_UNKNOWN_FAULT; + } +} + DLLEXPORT void dds_c_configure_tt(DDS_C_SOLVER_CTX ctx, int tt_kind, int def_mb, int max_mb) { diff --git a/library/src/api/dds_c_api.h b/library/src/api/dds_c_api.h index d6bb8729..4fbb4ec1 100644 --- a/library/src/api/dds_c_api.h +++ b/library/src/api/dds_c_api.h @@ -43,12 +43,6 @@ DLLEXPORT int dds_c_solve_board(DDS_C_SOLVER_CTX ctx, int target, int solutions, int mode, struct FutureTricks* futp); -/* Solve a single board in PBN format. Returns a RETURN_* status code. */ -DLLEXPORT int dds_c_solve_board_pbn(DDS_C_SOLVER_CTX ctx, - const struct DealPBN* dlpbn, - int target, int solutions, int mode, - struct FutureTricks* futp); - /* Compute the double dummy table for a deal. */ DLLEXPORT int dds_c_calc_dd_table(DDS_C_SOLVER_CTX ctx, const struct DdTableDeal* deal, @@ -61,14 +55,6 @@ DLLEXPORT int dds_c_calc_par(DDS_C_SOLVER_CTX ctx, struct DdTableResults* results, struct ParResults* par); -/* Compute the par result for a PBN-format deal (computes the DD table - internally). */ -DLLEXPORT int dds_c_calc_par_pbn(DDS_C_SOLVER_CTX ctx, - const struct DdTableDealPBN* deal, - int vulnerable, - struct DdTableResults* results, - struct ParResults* par); - /* Creation with explicit transposition-table configuration. The C++ SolverConfig is decomposed into scalars rather than mirrored as a struct: passing a struct by value is exactly the ABI question this shim exists to avoid, and a mirror @@ -83,6 +69,41 @@ DLLEXPORT int dds_c_calc_dd_table_pbn(DDS_C_SOLVER_CTX ctx, const struct DdTableDealPBN* deal, struct DdTableResults* results); +/* Parse a PBN deal string into the binary holdings block that both + struct Deal.remainCards and struct DdTableDeal.cards use: DDS_HANDS x + DDS_SUITS unsigned ints, row-major ([hand][suit]), so a caller can convert + straight into either struct field and then use the binary entry points. Only + the first hand carries a compass letter (N/E/S/W); the other three follow + clockwise. The terminator must occur within the first 80 bytes (a string of + 79 content bytes plus terminator fits exactly); no more than 80 bytes are + read. Needs no SolverContext. + + This is the general PBN entry point for bindings: it is why the solve and par + calls have no *_pbn twin on this shim. + + Returns RETURN_UNKNOWN_FAULT if either pointer is NULL -- checked before + pbn_deal is read at all, so on this path cards is untouched, not cleared -- + RETURN_PBN_FAULT if a non-NULL string cannot be parsed, else RETURN_NO_FAULT. + Two limits matter once both pointers are non-NULL: + + - RETURN_NO_FAULT means "parsed", not "describes a legal deal". A rank + character that is out of range or otherwise unrecognized is skipped + rather than refused, so a typo'd rank yields a short hand and still + converts -- except a compass letter (N/E/S/W, either case) found after + the first hand, which IS refused with RETURN_PBN_FAULT rather than + skipped. Deal validation lives in the entry points that consume the + block: they return RETURN_CARD_COUNT or RETURN_DUPLICATE_CARDS. Check + that status too -- a successful conversion is not a validated deal. + (Partial deals are legitimate, e.g. mid-trick, so this function cannot + count cards itself.) + + - On RETURN_PBN_FAULT the output is NOT preserved: all DDS_HANDS x + DDS_SUITS entries are zeroed before parsing begins, so a rejected string + leaves the block zeroed or holding a partial parse. Convert into scratch + storage if the destination must survive a bad string. */ +DLLEXPORT int dds_c_convert_from_pbn(const char* pbn_deal, + unsigned int cards[DDS_HANDS][DDS_SUITS]); + /* Transposition-table configuration. */ DLLEXPORT void dds_c_configure_tt(DDS_C_SOLVER_CTX ctx, int tt_kind, int def_mb, int max_mb); diff --git a/library/src/dds_api.cpp b/library/src/dds_api.cpp index 75274072..0eac8a59 100644 --- a/library/src/dds_api.cpp +++ b/library/src/dds_api.cpp @@ -75,11 +75,6 @@ DLLEXPORT auto dds_solve_board(DDS_SOLVER_CTX ctx, const Deal& dl, int target, i futp); } -DLLEXPORT auto dds_solve_board_pbn(DDS_SOLVER_CTX ctx, const DealPBN& dlpbn, int target, int solutions, int mode, FutureTricks* futp) -> int -{ - return solve_board_pbn(*ctx, dlpbn, target, solutions, mode, futp); -} - DLLEXPORT auto dds_calc_dd_table(DDS_SOLVER_CTX ctx, const DdTableDeal& table_deal, DdTableResults* table_results) -> int { return calc_dd_table(*ctx, table_deal, table_results); @@ -101,13 +96,3 @@ DLLEXPORT auto dds_calc_par( return calc_par(*ctx, table_deal, vulnerable, table_results, par_results); } -DLLEXPORT auto dds_calc_par_pbn( - DDS_SOLVER_CTX ctx, - const DdTableDealPBN& table_deal_pbn, - int vulnerable, - DdTableResults* table_results, - ParResults* par_results) -> int -{ - return calc_par_pbn(*ctx, table_deal_pbn, vulnerable, table_results, par_results); -} - diff --git a/library/tests/dds_c_api_test.cpp b/library/tests/dds_c_api_test.cpp index f46f8b7d..e7f016eb 100644 --- a/library/tests/dds_c_api_test.cpp +++ b/library/tests/dds_c_api_test.cpp @@ -20,7 +20,7 @@ #include #include -#include // full definitions for DealPBN, ParResults{Dealer,Master}, ParTextResults, DDSInfo +#include // full definitions for ParResults{Dealer,Master}, ParTextResults, DDSInfo namespace { @@ -68,15 +68,6 @@ struct DdTableDeal MakeReferenceTableDeal() return deal; } -struct DealPBN MakeReferenceDealPbn() -{ - struct DealPBN dlpbn = {}; - dlpbn.trump = 0; // spades - dlpbn.first = 0; // North leads - std::snprintf(dlpbn.remainCards, sizeof dlpbn.remainCards, "%s", kReferencePbn); - return dlpbn; -} - // Solve the reference board on ctx and return the trick count. int SolveReference(DDS_C_SOLVER_CTX ctx) { @@ -96,7 +87,6 @@ int SolveReference(DDS_C_SOLVER_CTX ctx) TEST(DdsCApiNullHandle, IntReturningEntryPointsFailFast) { const struct Deal dl = MakeReferenceDeal(); - const struct DealPBN dlpbn = MakeReferenceDealPbn(); const struct DdTableDeal table_deal = MakeReferenceTableDeal(); struct DdTableDealPBN pbn_deal = {}; struct FutureTricks fut = {}; @@ -104,11 +94,9 @@ TEST(DdsCApiNullHandle, IntReturningEntryPointsFailFast) struct ParResults par = {}; EXPECT_EQ(dds_c_solve_board(nullptr, &dl, -1, 1, 1, &fut), RETURN_UNKNOWN_FAULT); - EXPECT_EQ(dds_c_solve_board_pbn(nullptr, &dlpbn, -1, 1, 1, &fut), RETURN_UNKNOWN_FAULT); EXPECT_EQ(dds_c_calc_dd_table(nullptr, &table_deal, &results), RETURN_UNKNOWN_FAULT); EXPECT_EQ(dds_c_calc_dd_table_pbn(nullptr, &pbn_deal, &results), RETURN_UNKNOWN_FAULT); EXPECT_EQ(dds_c_calc_par(nullptr, &table_deal, 0, &results, &par), RETURN_UNKNOWN_FAULT); - EXPECT_EQ(dds_c_calc_par_pbn(nullptr, &pbn_deal, 0, &results, &par), RETURN_UNKNOWN_FAULT); } // Context-free utilities: must reject null data pointers. @@ -167,15 +155,11 @@ TEST(DdsCApiNullArgument, PointerArgumentsAreValidated) struct DdTableDealPBN pbn_deal = {}; const struct DdTableDeal table_deal = MakeReferenceTableDeal(); struct FutureTricks fut = {}; - struct ParResults par = {}; EXPECT_EQ(dds_c_solve_board(ctx, nullptr, -1, 1, 1, &fut), RETURN_UNKNOWN_FAULT); - EXPECT_EQ(dds_c_solve_board_pbn(ctx, nullptr, -1, 1, 1, &fut), RETURN_UNKNOWN_FAULT); EXPECT_EQ(dds_c_calc_dd_table_pbn(ctx, nullptr, &results), RETURN_UNKNOWN_FAULT); EXPECT_EQ(dds_c_calc_dd_table_pbn(ctx, &pbn_deal, nullptr), RETURN_UNKNOWN_FAULT); EXPECT_EQ(dds_c_calc_par(ctx, &table_deal, 0, &results, nullptr), RETURN_UNKNOWN_FAULT); - EXPECT_EQ(dds_c_calc_par_pbn(ctx, nullptr, 0, &results, &par), RETURN_UNKNOWN_FAULT); - EXPECT_EQ(dds_c_calc_par_pbn(ctx, &pbn_deal, 0, &results, nullptr), RETURN_UNKNOWN_FAULT); // A null message must be ignored rather than passed through to strlen. dds_c_log_append(ctx, nullptr); @@ -358,39 +342,192 @@ TEST(DdsCApiPar, ProducesNonEmptyScore) dds_c_destroy_solvercontext(ctx); } -TEST(DdsCApiSolveBoard, PbnMatchesBinary) +// --------------------------------------------------------------------------- +// PBN conversion. dds_c_convert_from_pbn exposes the parser itself, so a +// binding can turn a PBN string into the binary holdings block shared by +// struct Deal.remainCards and struct DdTableDeal.cards and then use the binary +// entry points. It needs no SolverContext. +// --------------------------------------------------------------------------- + +TEST(DdsCApiConvertFromPbn, ProducesTheBinaryReferenceHoldings) +{ + const struct DdTableDeal expected = MakeReferenceTableDeal(); + unsigned int cards[DDS_HANDS][DDS_SUITS] = {}; + + ASSERT_EQ(dds_c_convert_from_pbn(kReferencePbn, cards), RETURN_NO_FAULT); + + for (int hand = 0; hand < DDS_HANDS; ++hand) + for (int suit = 0; suit < DDS_SUITS; ++suit) + EXPECT_EQ(cards[hand][suit], expected.cards[hand][suit]) + << "cards[" << hand << "][" << suit << "]"; +} + +TEST(DdsCApiConvertFromPbn, RejectsNullArguments) +{ + unsigned int cards[DDS_HANDS][DDS_SUITS] = {}; + + EXPECT_EQ(dds_c_convert_from_pbn(nullptr, cards), RETURN_UNKNOWN_FAULT); + EXPECT_EQ(dds_c_convert_from_pbn(kReferencePbn, nullptr), RETURN_UNKNOWN_FAULT); +} + +// A malformed string must surface as RETURN_PBN_FAULT, not as the parser's raw +// 0: every other entry point here returns a RETURN_* code. +TEST(DdsCApiConvertFromPbn, RejectsMalformedStringWithPbnFault) +{ + unsigned int cards[DDS_HANDS][DDS_SUITS] = {}; + + EXPECT_EQ(dds_c_convert_from_pbn("xx", cards), RETURN_PBN_FAULT); +} + +// Success means "parsed", not "legal deal". The parser skips characters it does +// not recognize, so a bad rank silently yields a short hand; the deal is then +// refused by the binary entry point on card count, which is where deal +// validation lives (see library/tests/deal_input_validation_test.cpp, where the +// same layering is pinned for the legacy CalcDDtablePBN path). A caller must +// therefore check the solve/table status, not just the conversion status. +TEST(DdsCApiConvertFromPbn, UnrecognizedRankParsesButDealIsRefusedOnCardCount) +{ + // The reference deal with North's '2' of spades replaced by 'X'. + const char* const pbn_with_bad_rank = + "N:AKQJT9876543X... .AKQJT98765432.. ..AKQJT98765432. ...AKQJT98765432"; + + struct DdTableDeal deal = {}; + EXPECT_EQ(dds_c_convert_from_pbn(pbn_with_bad_rank, deal.cards), RETURN_NO_FAULT); + + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + struct DdTableResults results = {}; + EXPECT_EQ(dds_c_calc_dd_table(ctx, &deal, &results), RETURN_CARD_COUNT); + + dds_c_destroy_solvercontext(ctx); +} + +// Not every unrecognized character is skipped the same way: a compass letter +// (N/E/S/W, either case) after the first hand is explicitly refused, unlike an +// unrecognized rank (see UnrecognizedRankParsesButDealIsRefusedOnCardCount). +TEST(DdsCApiConvertFromPbn, RejectsCompassLetterAfterFirstHand) +{ + // The reference deal with the '.' between South's spades and hearts + // replaced by 'N'. + const char* const pbn_with_stray_compass_letter = + "N:AKQJT98765432... .AKQJT98765432.. N.AKQJT98765432. ...AKQJT98765432"; + unsigned int cards[DDS_HANDS][DDS_SUITS] = {}; + + EXPECT_EQ(dds_c_convert_from_pbn(pbn_with_stray_compass_letter, cards), + RETURN_PBN_FAULT); +} + +// The clobbering guarantee only applies once the string has actually reached +// the parser: a null pbn_deal is rejected by the shim's own guard before +// convert_from_pbn runs, so an existing cards block is left completely alone +// rather than zeroed. +TEST(DdsCApiConvertFromPbn, NullPbnDealDoesNotTouchTheOutputBlock) +{ + unsigned int cards[DDS_HANDS][DDS_SUITS]; + for (auto& hand : cards) + for (auto& suit : hand) + suit = 0xFFFFFFFFU; + + ASSERT_EQ(dds_c_convert_from_pbn(nullptr, cards), RETURN_UNKNOWN_FAULT); + + for (int hand = 0; hand < DDS_HANDS; ++hand) + for (int suit = 0; suit < DDS_SUITS; ++suit) + EXPECT_EQ(cards[hand][suit], 0xFFFFFFFFU) + << "cards[" << hand << "][" << suit << "] after a null pbn_deal"; +} + +// The 80-byte scan window is inclusive: a string whose NUL terminator falls at +// byte 79 (80 bytes total, matching sizeof(DealPBN::remainCards)) is valid -- +// see library/tests/pbn_test.cpp's AcceptsInputThatFitsRemainCardsBuffer / +// RejectsInputExactlyAtRemainCardsBufferLimit for the exact boundary. Trailing +// junk characters are silently skipped like any other unrecognized character, +// so padding the reference deal out to that length must still parse and solve +// identically. +TEST(DdsCApiConvertFromPbn, AcceptsStringWithTerminatorAtByte79) +{ + std::string pbn = kReferencePbn; + ASSERT_LT(pbn.size(), 79U); + pbn.append(79U - pbn.size(), 'z'); + ASSERT_EQ(pbn.size(), 79U); + + struct Deal dl = {}; + dl.trump = 0; + dl.first = 0; + ASSERT_EQ(dds_c_convert_from_pbn(pbn.c_str(), dl.remainCards), RETURN_NO_FAULT); + + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + struct FutureTricks fut = {}; + ASSERT_EQ(dds_c_solve_board(ctx, &dl, -1, 1, 1, &fut), RETURN_NO_FAULT); + EXPECT_EQ(fut.score[0], kExpectedTricks); + + dds_c_destroy_solvercontext(ctx); +} + +// On failure the output is not preserved: the parser zeroes all 16 entries +// before it validates anything, so converting in place into a live deal +// destroys the previous holdings. Pinned so the header's warning stays true. +// (This applies once the string reaches the parser -- see +// NullPbnDealDoesNotTouchTheOutputBlock for the null-input case, which does +// not.) +TEST(DdsCApiConvertFromPbn, FailureClobbersTheOutputBlock) +{ + unsigned int cards[DDS_HANDS][DDS_SUITS]; + for (auto& hand : cards) + for (auto& suit : hand) + suit = 0xFFFFFFFFU; + + ASSERT_EQ(dds_c_convert_from_pbn("xx", cards), RETURN_PBN_FAULT); + + for (int hand = 0; hand < DDS_HANDS; ++hand) + for (int suit = 0; suit < DDS_SUITS; ++suit) + EXPECT_EQ(cards[hand][suit], 0U) + << "cards[" << hand << "][" << suit << "] after a failed parse"; +} + +// The replacement path for the withdrawn dds_c_solve_board_pbn: convert, fill +// the binary Deal's remaining (already-binary) fields, then solve. +TEST(DdsCApiConvertFromPbn, ConvertedDealSolvesLikeTheBinaryFixture) { + struct Deal dl = {}; + dl.trump = 0; // spades + dl.first = 0; // North leads + ASSERT_EQ(dds_c_convert_from_pbn(kReferencePbn, dl.remainCards), RETURN_NO_FAULT); + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); ASSERT_NE(ctx, nullptr); - const struct DealPBN dlpbn = MakeReferenceDealPbn(); struct FutureTricks fut = {}; - ASSERT_EQ(dds_c_solve_board_pbn(ctx, &dlpbn, -1, 1, 1, &fut), RETURN_NO_FAULT); + ASSERT_EQ(dds_c_solve_board(ctx, &dl, -1, 1, 1, &fut), RETURN_NO_FAULT); EXPECT_EQ(fut.score[0], kExpectedTricks); dds_c_destroy_solvercontext(ctx); } -TEST(DdsCApiPar, PbnMatchesBinary) +// Cross-check the converter against the one surviving PBN twin: feeding the +// converted holdings to the binary table call must match the twin exactly. +TEST(DdsCApiConvertFromPbn, ConvertedTableDealMatchesThePbnTwin) { DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); ASSERT_NE(ctx, nullptr); - const struct DdTableDeal binary_deal = MakeReferenceTableDeal(); - struct DdTableResults binary_results = {}; - struct ParResults binary_par = {}; - ASSERT_EQ(dds_c_calc_par(ctx, &binary_deal, 0, &binary_results, &binary_par), - RETURN_NO_FAULT); + struct DdTableDeal converted = {}; + ASSERT_EQ(dds_c_convert_from_pbn(kReferencePbn, converted.cards), RETURN_NO_FAULT); + struct DdTableResults converted_results = {}; + ASSERT_EQ(dds_c_calc_dd_table(ctx, &converted, &converted_results), RETURN_NO_FAULT); struct DdTableDealPBN pbn_deal = {}; std::snprintf(pbn_deal.cards, sizeof pbn_deal.cards, "%s", kReferencePbn); - struct DdTableResults pbn_results = {}; - struct ParResults pbn_par = {}; - ASSERT_EQ(dds_c_calc_par_pbn(ctx, &pbn_deal, 0, &pbn_results, &pbn_par), - RETURN_NO_FAULT); + struct DdTableResults twin_results = {}; + ASSERT_EQ(dds_c_calc_dd_table_pbn(ctx, &pbn_deal, &twin_results), RETURN_NO_FAULT); - EXPECT_STREQ(pbn_par.par_score[0], binary_par.par_score[0]); - EXPECT_STREQ(pbn_par.par_score[1], binary_par.par_score[1]); + for (int strain = 0; strain < DDS_STRAINS; ++strain) + for (int hand = 0; hand < DDS_HANDS; ++hand) + EXPECT_EQ(converted_results.res_table[strain][hand], + twin_results.res_table[strain][hand]) + << "res_table[" << strain << "][" << hand << "]"; dds_c_destroy_solvercontext(ctx); } diff --git a/specs/dds-public-api.md b/specs/dds-public-api.md index ba4e8a56..b8f61124 100644 --- a/specs/dds-public-api.md +++ b/specs/dds-public-api.md @@ -1,7 +1,7 @@ --- capability: dds-public-api owners: [api] -last-updated: 2026-07-19 +last-updated: 2026-09-26 --- # DDS Public API @@ -52,8 +52,9 @@ capability defines what crosses the boundary and promises to stay stable. opaque `void*` handle (`DDS_C_SOLVER_CTX`); no C++ types cross the boundary. It forwards to layer 2 and now covers that layer's full surface: context lifecycle (including config-based creation), `dds_c_solve_board`, - `dds_c_calc_dd_table` and its `_pbn` twin, `dds_c_calc_par`, TT - configure/resize/clear, both resets, and the logging passthroughs. + `dds_c_calc_dd_table` and its `_pbn` twin, `dds_c_calc_par`, + `dds_c_convert_from_pbn`, TT configure/resize/clear, both resets, and the + logging passthroughs. `SolverConfig` is decomposed into scalar arguments and `TTKind` crosses as an `int`, so no struct is passed by value. - **Bindings pick different layers.** The shim header @@ -80,13 +81,32 @@ capability defines what crosses the boundary and promises to stay stable. `SetMaxThreads`/`SetResources`/`FreeMemory` instead. - **Integer status returns.** Solver entry points return `RETURN_*` status codes (success is positive/`RETURN_NO_FAULT`); `ErrorMessage` maps a code to text. -- **PBN and binary variants are paired on the legacy layer.** Most flat `dll.h` - entry points have a `*PBN` twin; both compute identical results from the same - deal. On the modern C++ layer only `dds_calc_dd_table` has one - (`dds_calc_dd_table_pbn`) — `dds_solve_board` and `dds_calc_par` do not. The - shim mirrors the modern layer exactly: `dds_c_calc_dd_table_pbn` is its only - PBN twin. Agreement between each pair is asserted by - `//library/tests:dds_c_api_test` and by the .NET smoke tests. +- **PBN is paired by twins on the legacy layer, but reached through a converter + on the exported modern surface.** Most flat `dll.h` entry points have a `*PBN` + twin; both compute identical results from the same deal, and that layer is + frozen, so its twins stay. The exported modern surface deliberately does *not* + grow one twin per entry point: `dds_c_convert_from_pbn` parses a PBN deal + string into the binary holdings block that both `Deal.remainCards` and + `DdTableDeal.cards` use, so a binding converts once and then uses the binary + entry points. Every other field of a `*PBN` struct (`trump`, `first`, + `currentTrick*`) is already binary and needs no conversion, which is what makes + one converter enough to replace a twin per function. + `dds_c_calc_dd_table_pbn` is the sole surviving twin: it shipped in v3.1.0 and + is bound by .NET, so it stays for compatibility. `dds_c_solve_board_pbn` and + `dds_c_calc_par_pbn` (and their `dds_*` layer-2 counterparts) were withdrawn + in favour of the converter before any tagged release carried them. Note that + `DDS_VERSION` / `DDS_FFM_VERSION` are not bumped between tags, so pre-release + `develop` builds cannot be told apart by version: only a tagged release is a + meaningful ABI reference point. The C++ `solve_board_pbn` / `calc_par_pbn` + overloads survive as non-exported static-link conveniences for in-tree C++ + callers (today only `configure_tt_api_test`; `calc_par_pbn` has no caller at + all). Neither is `DLLEXPORT`, so neither appears in any export list on any + platform and neither carries ABI cost — the batch and analyse paths call + `convert_from_pbn` directly rather than going through them. + `//library/tests:dds_c_api_test` asserts converter/twin agreement and the + converter's contract, including that a successful parse is not a validated + deal and that a failed parse does not preserve the output block; + `//jni:dds_ffm_smoke_test` covers the same path end to end. - **The pinned binding export set is `dll.h` + `dds_c_api.h`.** On Linux/macOS the JNI shared library exports are constrained by `jni/version_script.lds` / `exported_symbols.lds` and checked by the export-set test. That is the *stable