From 49c5c2f7fb01eb600afead74e9230f698efab6ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Posp=C3=AD=C5=A1il?= Date: Fri, 14 Aug 2026 23:48:59 +0200 Subject: [PATCH] An unidentified unique on a base with two of one name is still a question MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADDED: a capture of a Stormblood on the Topaz Flask, and a test that the base rolls into two candidates rather than one, that each identified copy resolves to the record for the base it is on, and that the search is sent for that base. CHANGED: the bundle slicer refuses a key naming more than one record instead of quietly taking the last, so a unique that drops on two bases is asked for as `"UNIQUE::Stormblood::Topaz Flask"` and the two game classes that both print "Maps" are told apart by id. CHANGED: the test bundle carries both flask bases, both Stormblood records and Vessel of Vinktar; `strip_magic_affixes`' "not in this bundle" case moved to a base the slice still lacks. - The defect itself was in the data build, which keyed a unique on its name alone and dropped the second base — fixed there. Nothing in the app changed; this is the cover that would have caught it. Co-Authored-By: Claude Opus 5 --- docs/testing.md | 5 +- scripts/slice-test-bundle.py | 60 ++++++++++++++---- tests/data/bundle/en-items-base.index.bin | Bin 112 -> 136 bytes tests/data/bundle/en-items-name.index.bin | Bin 424 -> 464 bytes tests/data/bundle/en-items-ref.index.bin | Bin 424 -> 464 bytes tests/data/bundle/en-items.ndjson | 5 ++ .../items/unique-flask-stormblood-topaz.txt | 30 +++++++++ tests/item_pricing_test.cpp | 49 +++++++++++++- 8 files changed, 135 insertions(+), 14 deletions(-) create mode 100644 tests/data/items/unique-flask-stormblood-topaz.txt diff --git a/docs/testing.md b/docs/testing.md index 402b6d9..32e8c67 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -68,7 +68,10 @@ lists at the top of that script, never writing a record by hand. It copies every rebuilds the indices from the offsets it just wrote, which is the point: the `.index.bin` files address the ndjson by byte offset, so one extra byte per line silently shifts every record out from under every lookup and fails as null lookups rather than as a diff. Keep the ndjson **LF and -byte-exact**; `.gitattributes` pins that down and those entries must stay. +byte-exact**; `.gitattributes` pins that down and those entries must stay. A key naming more than +one record is refused rather than resolved, so a unique that drops on two bases is asked for by +both — `"UNIQUE::Stormblood::Topaz Flask"` — and an item class two game classes print the name of +by its id, `"Maps::MapKey"`. The slice has **no `(Local)` stat record**, so the local/global disambiguation in `item/resolve` is not covered offline — it is verified against an installed bundle by hand. Adding one such record (and diff --git a/scripts/slice-test-bundle.py b/scripts/slice-test-bundle.py index 91f3a12..39405b3 100755 --- a/scripts/slice-test-bundle.py +++ b/scripts/slice-test-bundle.py @@ -248,6 +248,16 @@ # `en-items-base.index.bin`, so the pair is also what covers that index at all. "ITEM::Goathide Gloves", "UNIQUE::Hrimburn", + # A unique that drops on **two bases** under one name, and the second unique of one of + # them. Both halves matter: the name addresses two records, so the base is what tells them + # apart, and the Topaz Flask is what an unidentified one is read through — a bundle + # carrying only the Sapphire record leaves that base with a single candidate, which is + # taken as the name rather than asked about. + "ITEM::Topaz Flask", + "ITEM::Sapphire Flask", + "UNIQUE::Stormblood::Topaz Flask", + "UNIQUE::Stormblood::Sapphire Flask", + "UNIQUE::Vessel of Vinktar", # A second card, because the capture that proves a card resolves at all is a real one. "DIVINATION_CARD::The Blazing Fire", # An essence, for the other half of that: both are traded in bulk on the in-game exchange, @@ -364,8 +374,10 @@ "HeistContractNoGangCut1", ] +# "Maps" is qualified because two game classes print that name — the map itself and the +# stand-in row trade lists every map under. ITEM_CLASSES = ["Rings", "Boots", "Gloves", "Body Armours", "Stackable Currency", - "Divination Cards", "Jewels", "Utility Flasks", "Maps", "Skill Gems", + "Divination Cards", "Jewels", "Utility Flasks", "Maps::MapKey", "Skill Gems", "Support Gems", "Chart", "Misc Map Items", "Contracts", "Blueprints", "Sanctum Research", "Expedition Logbooks", "Helmets", "Amulets", "Belts", "Sceptres", "Heist Gear"] @@ -389,12 +401,36 @@ def read_ndjson(path: Path) -> list[tuple[bytes, dict]]: def pick(records: list[tuple[bytes, dict]], wanted: list[str], key) -> list[tuple[bytes, dict]]: - """The wanted records, in the order listed above — the fixture's line order is ours.""" - by_key = {key(r): (line, r) for line, r in records} - missing = [w for w in wanted if w not in by_key] - if missing: + """The wanted records, in the order listed above — the fixture's line order is ours. + + `key` returns every name a record answers to, because one name can address two records: a + unique is its name *and* its base, and Stormblood drops on both the Sapphire and the Topaz + Flask. A wanted key that names more than one record is refused rather than resolved — + taking either would put a record in the fixture that nobody asked for. + """ + by_key: dict[str, list[tuple[bytes, dict]]] = {} + for line, r in records: + for k in key(r): + by_key.setdefault(k, []).append((line, r)) + if missing := [w for w in wanted if w not in by_key]: sys.exit(f"not in the source bundle: {', '.join(missing)}") - return [by_key[w] for w in wanted] + if several := [w for w in wanted if len(by_key[w]) > 1]: + sys.exit("names more than one record, so qualify it with the base or the id " + f"(\"UNIQUE::Stormblood::Topaz Flask\"): {', '.join(several)}") + return [by_key[w][0] for w in wanted] + + +def item_keys(r: dict) -> list[str]: + """`"{namespace}::{name}"`, plus `"::{base}"` again for a unique — see `pick`.""" + key = f"{r['namespace']}::{r['name']}" + if base := r.get("unique", {}).get("base"): + return [key, f"{key}::{base}"] + return [key] + + +def class_keys(r: dict) -> list[str]: + """The class name, plus `"::{id}"` — two game classes can print one name ("Maps").""" + return [r["itemClass"], f"{r['itemClass']}::{r['id']}"] def write_ndjson(path: Path, chosen: list[tuple[bytes, dict]]) -> list[int]: @@ -422,15 +458,14 @@ def main() -> int: src, out = args.source, args.out out.mkdir(parents=True, exist_ok=True) - stats = pick(read_ndjson(src / f"{LANG}-stats.ndjson"), STATS, lambda r: r["ref"]) + stats = pick(read_ndjson(src / f"{LANG}-stats.ndjson"), STATS, lambda r: [r["ref"]]) offsets = write_ndjson(out / f"{LANG}-stats.ndjson", stats) write_index(out / f"{LANG}-stats-ref.index.bin", [(r["ref"], off) for (_, r), off in zip(stats, offsets)]) write_index(out / f"{LANG}-stats-matcher.index.bin", [(m["string"], off) for (_, r), off in zip(stats, offsets) for m in r["matchers"]]) - items = pick(read_ndjson(src / f"{LANG}-items.ndjson"), ITEMS, - lambda r: f"{r['namespace']}::{r['name']}") + items = pick(read_ndjson(src / f"{LANG}-items.ndjson"), ITEMS, item_keys) offsets = write_ndjson(out / f"{LANG}-items.ndjson", items) write_index(out / f"{LANG}-items-name.index.bin", [(f"{r['namespace']}::{r['name']}", off) for (_, r), off in zip(items, offsets)]) @@ -440,13 +475,14 @@ def main() -> int: [(f"UNIQUE::{r['unique']['base']}", off) for (_, r), off in zip(items, offsets) if r["namespace"] == "UNIQUE" and r.get("unique", {}).get("base")]) - uniques = pick(read_ndjson(src / f"{LANG}-unique-mods.ndjson"), UNIQUE_MODS, lambda r: r["name"]) + uniques = pick(read_ndjson(src / f"{LANG}-unique-mods.ndjson"), UNIQUE_MODS, + lambda r: [r["name"]]) offsets = write_ndjson(out / f"{LANG}-unique-mods.ndjson", uniques) write_index(out / f"{LANG}-unique-mods-name.index.bin", [(f"UNIQUE::{r['name']}", off) for (_, r), off in zip(uniques, offsets)]) pools = pick(read_ndjson(src / f"{LANG}-mod-pools.ndjson"), MOD_POOLS, - lambda r: r["mods"][0]) + lambda r: [r["mods"][0]]) offsets = write_ndjson(out / f"{LANG}-mod-pools.ndjson", pools) # One key per wording, qualified by domain: a map and a chart share wordings and are # separate pools, so the domain is part of what is being asked for. @@ -454,7 +490,7 @@ def main() -> int: [(f"{r['domain']}::{s['ref']}", off) for (_, r), off in zip(pools, offsets) for s in r["stats"]]) - classes = pick(read_ndjson(src / "item-classes.ndjson"), ITEM_CLASSES, lambda r: r["itemClass"]) + classes = pick(read_ndjson(src / "item-classes.ndjson"), ITEM_CLASSES, class_keys) write_ndjson(out / "item-classes.ndjson", classes) # Not the source's manifest: the fixture is not a release, and nothing may mistake it for diff --git a/tests/data/bundle/en-items-base.index.bin b/tests/data/bundle/en-items-base.index.bin index 38137755dbc38ed9fda8a73ebf95df9898bdc9a8..4cb8c0ba0cd1f177b913f9cc68e9e89c470e1221 100644 GIT binary patch delta 96 zcmXTuV3eD>{GvR&5(9(wdtZNVK?a6^V5PK4ObiSh8oVvCY7@oev?i!9Fg%{Zwt4|4 y14Gkt%k@Gs3=BHLfA_D|WMH^@aMkVY0ucI<;6y($o@wd~49Dv(y$O|OU;qG3iyr3y delta 90 zcmeBRED)Qz{G$8@IR=J+V5PK4ObiSh8oVvXlqSl_*;pzvFg%{Zwt4|414Gkt%k{e@ q7#MVf|LzY{V_-1U@Ovr63!x|T0o4_CHcJ1_l7Q#2cyr diff --git a/tests/data/bundle/en-items-name.index.bin b/tests/data/bundle/en-items-name.index.bin index ec99677fe6bbad2bc7190faf011081d4db532d13..e971c15ecbd37345c34a1ce15ce360f1221246eb 100644 GIT binary patch delta 428 zcmZ3%e1X~RtknUQ?E(x8Alh4yf#JpXS1kM-3=9ue#RViLbMndjPEau+XYnLaaSPWt{MY_m&DgZDP9JK`>TGZyp~{K@J;cr zPd&xWz+kRYpFSUG(AGYNtT{3a46A4Riga3=9)K9ltpdXu807adOs{jB1 delta 388 zcmcb>yn@;6#rIb%{2UAn4_3yr{S#+kkel*}?W7O`!;#HrI9IbVFsxja#&c7KfuZE- zRzVvs1_s65ed3mi6W!$MMMM}F{wwe+tMfB3L^S``vR7eXXj@-x@ST-`Avvtt;xv%2 zBJE+lfRlmYTEi(11|WD)S>dZI$iVQfzCZArC`*`wE5|z`8~o63>&waFWdyQc<$MP#i~GakFm1s z3&&r(F1|&jQr;sPNlv2?hoL&!>Ly diff --git a/tests/data/bundle/en-items-ref.index.bin b/tests/data/bundle/en-items-ref.index.bin index ec99677fe6bbad2bc7190faf011081d4db532d13..e971c15ecbd37345c34a1ce15ce360f1221246eb 100644 GIT binary patch delta 428 zcmZ3%e1X~RtknUQ?E(x8Alh4yf#JpXS1kM-3=9ue#RViLbMndjPEau+XYnLaaSPWt{MY_m&DgZDP9JK`>TGZyp~{K@J;cr zPd&xWz+kRYpFSUG(AGYNtT{3a46A4Riga3=9)K9ltpdXu807adOs{jB1 delta 388 zcmcb>yn@;6#rIb%{2UAn4_3yr{S#+kkel*}?W7O`!;#HrI9IbVFsxja#&c7KfuZE- zRzVvs1_s65ed3mi6W!$MMMM}F{wwe+tMfB3L^S``vR7eXXj@-x@ST-`Avvtt;xv%2 zBJE+lfRlmYTEi(11|WD)S>dZI$iVQfzCZArC`*`wE5|z`8~o63>&waFWdyQc<$MP#i~GakFm1s z3&&r(F1|&jQr;sPNlv2?hoL&!>Ly diff --git a/tests/data/bundle/en-items.ndjson b/tests/data/bundle/en-items.ndjson index 054db4f..2bcf301 100644 --- a/tests/data/bundle/en-items.ndjson +++ b/tests/data/bundle/en-items.ndjson @@ -18,6 +18,11 @@ {"art":"Art/2DItems/Armours/Gloves/Hrimsorrow.png","name":"Hrimsorrow","namespace":"UNIQUE","refName":"Hrimsorrow","unique":{"base":"Goathide Gloves"}} {"armour":{"ev":[32,42]},"craftable":{"category":"Gloves"},"domain":1,"dropLevel":9,"h":2,"metadataId":"Metadata/Items/Armours/Gloves/GlovesDex2","name":"Goathide Gloves","namespace":"ITEM","refName":"Goathide Gloves","w":2} {"art":"Art/2DItems/Armours/Gloves/Hrimsorrow.png","name":"Hrimburn","namespace":"UNIQUE","refName":"Hrimburn","unique":{"base":"Goathide Gloves"}} +{"craftable":{"category":"Utility Flasks"},"domain":2,"dropLevel":18,"h":2,"metadataId":"Metadata/Items/Flasks/FlaskUtility4","name":"Topaz Flask","namespace":"ITEM","refName":"Topaz Flask","w":1} +{"craftable":{"category":"Utility Flasks"},"domain":2,"dropLevel":18,"h":2,"metadataId":"Metadata/Items/Flasks/FlaskUtility3","name":"Sapphire Flask","namespace":"ITEM","refName":"Sapphire Flask","w":1} +{"art":"Art/2DItems/Flasks/StormbloodSapphire.png","name":"Stormblood","namespace":"UNIQUE","refName":"Stormblood","unique":{"base":"Topaz Flask"}} +{"art":"Art/2DItems/Flasks/StormbloodSapphire.png","name":"Stormblood","namespace":"UNIQUE","refName":"Stormblood","unique":{"base":"Sapphire Flask"}} +{"art":"Art/2DItems/Flasks/VinktarFlask.png","name":"Vessel of Vinktar","namespace":"UNIQUE","refName":"Vessel of Vinktar","unique":{"base":"Topaz Flask"}} {"craftable":{"category":"Divination Cards"},"domain":43,"dropLevel":62,"exchange":true,"h":1,"metadataId":"Metadata/Items/DivinationCards/DivinationCardTheBlazingFire","name":"The Blazing Fire","namespace":"DIVINATION_CARD","refName":"The Blazing Fire","w":1} {"craftable":{"category":"Stackable Currency"},"domain":43,"dropLevel":26,"exchange":true,"h":1,"metadataId":"Metadata/Items/Currency/CurrencyEssenceHatred3","name":"Weeping Essence of Hatred","namespace":"ITEM","refName":"Weeping Essence of Hatred","w":1} {"craftable":{"category":"Support Gems"},"domain":43,"dropLevel":38,"h":1,"metadataId":"Metadata/Items/Gems/SupportGemAdditionalLevel","name":"Empower Support","namespace":"GEM","refName":"Empower Support","w":1} diff --git a/tests/data/items/unique-flask-stormblood-topaz.txt b/tests/data/items/unique-flask-stormblood-topaz.txt new file mode 100644 index 0000000..5d31ed9 --- /dev/null +++ b/tests/data/items/unique-flask-stormblood-topaz.txt @@ -0,0 +1,30 @@ +Item Class: Utility Flasks +Rarity: Unique +Stormblood +Topaz Flask +-------- +Lasts 4 (augmented) Seconds +Consumes 20 of 50 Charges on use +Currently has 0 Charges ++5% to maximum Lightning Resistance +(Maximum Resistances cannot be raised above 90%) ++40% to Lightning Resistance +-------- +Requirements: +Level: 36 +-------- +Item Level: 85 +-------- +{ Unique Modifier — Critical } +31(20-40)% chance to gain a Flask Charge when you deal a Critical Strike +{ Unique Modifier — Elemental, Lightning, Critical, Ailment } +All Damage from Critical Strikes can apply Lightning Ailments during effect +(Lightning Ailments are Shocked and Sapped. Critical Strikes inherently have 100% chance to inflict Shock) +{ Unique Modifier } +50% reduced Duration +-------- +On that fateful day, beset by tempest, Captain Brinehook Vex +came face to face with the blustering Karui god of thunder, +Valako himself. A wager was had, and a wager was won. +-------- +Right click to drink. Can only hold charges while in belt. Refills as you kill monsters. diff --git a/tests/item_pricing_test.cpp b/tests/item_pricing_test.cpp index b915c4d..aadfb70 100644 --- a/tests/item_pricing_test.cpp +++ b/tests/item_pricing_test.cpp @@ -1086,6 +1086,52 @@ TEST_CASE("an unidentified unique on a base with several is a question, not a se } } +TEST_CASE("a unique that drops on two bases is two candidates, and two records") { + auto gd = fixture(); + // Stormblood drops on both the Sapphire and the Topaz Flask under one name. The bundle + // used to carry one record per name, so the Topaz Flask answered with Vessel of Vinktar + // alone — one candidate, which is taken as the name rather than asked about, and an + // unidentified Topaz Flask was priced as somebody else's unique. + const Item it = resolved(*gd, R"(Item Class: Utility Flasks +Rarity: Unique +Topaz Flask +-------- +Lasts 4.00 Seconds +Consumes 20 of 50 Charges on use +Currently has 0 Charges +-------- +Item Level: 85 +-------- +Unidentified +)"); + REQUIRE(it.unique_candidates.size() == 2); + CHECK(it.needs_unique_choice()); + CHECK(build_plan(*gd, it, derive(gd.get(), it)).name.empty()); + + SUBCASE("the identified one resolves to the record for the base it is on") { + // Both records answer to "Stormblood", so the base is what tells them apart — and the + // search is sent for this flask rather than for the cold one of the same name. + const Item id = resolved(*gd, capture("unique-flask-stormblood-topaz.txt")); + REQUIRE(id.unique_entry != nullptr); + CHECK(id.unique_entry->unique_base == "Topaz Flask"); + const SearchPlan p = build_plan(*gd, id, derive(gd.get(), id)); + CHECK(p.name == "Stormblood"); + CHECK(p.type == "Topaz Flask"); + } + + SUBCASE("and the other base's copy is the other record") { + const Item other = resolved(*gd, R"(Item Class: Utility Flasks +Rarity: Unique +Stormblood +Sapphire Flask +-------- +Item Level: 85 +)"); + REQUIRE(other.unique_entry != nullptr); + CHECK(other.unique_entry->unique_base == "Sapphire Flask"); + } +} + TEST_CASE("an identified unique is not read off its base") { auto gd = fixture(); // The candidate list is only ever about the gap an unidentified item leaves: an identified @@ -1113,7 +1159,8 @@ TEST_CASE("a magic item's base is found under its affixes") { CHECK(strip_magic_affixes(*gd, "Surgeon's Two-Stone Ring of the Cheetah", "Rings") == "Two-Stone Ring"); // Nothing in the bundle matches, and inventing a base is worse than admitting it. - CHECK(strip_magic_affixes(*gd, "Surgeon's Sapphire Flask of Heat", "Utility Flasks").empty()); + CHECK(strip_magic_affixes(*gd, "Surgeon's Quicksilver Flask of Heat", "Utility Flasks") + .empty()); const Item it = resolved(*gd, R"(Item Class: Rings Rarity: Magic