From cd2db6346e766e3d0d240fd0ebabc94b61cb2f1e Mon Sep 17 00:00:00 2001 From: JoshHeaps Date: Sat, 5 Sep 2026 22:26:16 -0600 Subject: [PATCH] Stop a chip colour taking the table's money with it, quietly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a chip colour was one click of an unlabelled 24px button, and it was never a change to a legend. computeContribution() prices a seat by running the legend over that seat's chip counts, so a colour that leaves takes every chip counted in it off the table. Measured in a browser on a three-handed table: deleting the $500 row took $22,500 of $26,874 with it. No confirmation, nothing said afterwards, and no way back — a colour added again is a new colour with a new id, while the counts stay keyed to the one that left. The counts were left behind too, so the saved file carried money nothing could price and nothing could show. Now: a colour nobody has counted still goes on one click, and one that is holding money asks first, naming the amount, the chips and the seats, and saying that Undo will put it back while adding the colour again will not. Conditional on purpose — a confirmation that fires every time is one nobody reads by the third night, and four others already compete for that attention here. Removal goes through runUndoable, and the counts go with the colour so the file matches the screen. The number in that warning is the difference between pricing every seat with the colour and without it, not chips times value. The obvious arithmetic is wrong both ways: a seat with a typed amount is never priced from its counts, and a seat whose chips all leave falls through to the table's standing ante rather than to zero. A randomised check over 2,000 tables asserts the warning equals what the table then does, and it found a third case — fourteen $1 chips against a $200 ante, where removing the colour puts the table UP $186. The wording says so in that direction too. The × also had no accessible name, so uiAudit.js gained controlNames(): every control needs a name containing a letter or a digit, resolved the way the accessibility tree resolves it. A symbol is not a name. Taking the new aria-label off again makes it report all five buttons. Audit unchanged at 2 failing configurations and 3 offenders, both pre-existing. 805 unit tests pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015bnp7xKUPFFSpwkYNFaPgb --- .../wwwroot/js/casino/holdem/main.js | 48 +++++ .../wwwroot/js/casino/holdem/render.js | 23 ++- .../wwwroot/js/casino/holdem/state.js | 78 ++++++++ tests/holdem/README.md | 47 +++++ tests/holdem/chipLegend.test.js | 183 ++++++++++++++++++ tests/holdem/uiAudit.js | 65 +++++++ 6 files changed, 439 insertions(+), 5 deletions(-) create mode 100644 tests/holdem/chipLegend.test.js diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js index cc0a4d6..2e09cfb 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/main.js @@ -86,6 +86,54 @@ document.addEventListener("DOMContentLoaded", () => { () => HoldemState.removeFromWaitingList(state, id)); }, onGiveButton: index => seatMenuHooks.makeDealer(index), + + // Removing a chip colour, which is only sometimes a small thing. + // + // A colour nobody has counted is a row in a legend, and taking it out should + // cost a click and nothing else. A colour seats are holding is money: the + // legend is what prices their chips, so the row leaving takes their chips off + // the table with it — and it cannot be put back, because a colour added again + // is a new one and the counts belong to the old. + // + // So the friction is on the second case only. A confirmation that fires every + // time is one nobody reads by the third night, and the one that matters here + // would go the same way as the four before it that didn't. + removeChipColor: async id => { + const usage = HoldemState.chipColorUsage(state, id); + if (!usage.chip) return; + const name = Number(usage.chip.value) > 0 + ? `${Money.format(usage.chip.value)} chip` + : "chip colour"; + + if (usage.total !== 0) { + const seatCount = `${usage.seats.length} ${usage.seats.length === 1 ? "seat" : "seats"}`; + const held = usage.seats.map(seat => `${seat.name} ${seat.count}`).join(", "); + const ok = await Dialog.confirm({ + // Both directions are named rather than only the loss. Going up is + // the odd one — seats falling back to the table's standing ante once + // their chips are gone — and a dealer who is told the total will move + // and then finds it moved the other way has been told something false. + title: usage.total > 0 + ? `Remove the ${name} and ${Money.format(usage.total)} with it?` + : `Remove the ${name} and put the table up ${Money.format(-usage.total)}?`, + message: + `${usage.chips} of them are counted onto ${seatCount} — ${held} — and the table ` + + `${usage.total > 0 ? "loses" : "gains"} ${Money.format(Math.abs(usage.total))} ` + + `when the colour goes. Undo will put it back; adding the colour again will not.`, + confirmLabel: "Remove it", + danger: usage.total > 0, + }); + if (!ok) return; + } + + runUndoable( + usage.total > 0 + ? `${name} removed — ${Money.format(usage.total)} off the table.` + : usage.total < 0 + ? `${name} removed — table up ${Money.format(-usage.total)}.` + : `${name} removed.`, + () => HoldemState.removeChipColor(state, id)); + }, }; const seatMenuHooks = { diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js index cf00d9c..488f02a 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/render.js @@ -2006,25 +2006,38 @@ const Render = { const row = template.content.firstElementChild.cloneNode(true); const colorInput = row.querySelector(".chipColorInput"); const valueInput = row.querySelector(".chipValueInput"); + const removeBtn = row.querySelector(".removeChipColorBtn"); colorInput.value = chip.color; valueInput.value = chip.value; + // "×" is the whole of this button, so without a name it is announced as a + // button and nothing else — and there are five of them in a column, all + // identical, each doing something different and none of them small. The + // value is what tells them apart, and it changes as it is typed, so the + // name is kept up with it rather than set once. + const nameRemove = () => { + const worth = Number(valueInput.value) || 0; + removeBtn.setAttribute("aria-label", worth > 0 + ? `Remove the ${Money.format(worth)} chip` + : "Remove this chip colour"); + }; + nameRemove(); + colorInput.addEventListener("input", () => { chip.color = colorInput.value; this.hooks.save(this.state); }); valueInput.addEventListener("input", () => { chip.value = Number(valueInput.value) || 0; + nameRemove(); this.hooks.save(this.state); this.renderSeats(); this.renderSeatEditor(); this.updatePotBadge(); }); - row.querySelector(".removeChipColorBtn").addEventListener("click", () => { - this.state.chipLegend = this.state.chipLegend.filter(c => c.id !== chip.id); - this.hooks.save(this.state); - this.renderAll(); - }); + // Through the hook rather than done here, because what this costs depends on + // what the seats are holding — see removeChipColor in main.js. + removeBtn.addEventListener("click", () => this.hooks.removeChipColor(chip.id)); list.appendChild(row); }); diff --git a/JoshHeaps.Net/wwwroot/js/casino/holdem/state.js b/JoshHeaps.Net/wwwroot/js/casino/holdem/state.js index bfbf74d..c658db9 100644 --- a/JoshHeaps.Net/wwwroot/js/casino/holdem/state.js +++ b/JoshHeaps.Net/wwwroot/js/casino/holdem/state.js @@ -1139,6 +1139,84 @@ const HoldemState = { return Math.max(0, Money.round(Betting.stackOf(state, player))); }, + // What a chip colour is holding up. + // + // Removing one is not a change to a legend. computeContribution() prices a seat by + // running the legend over that seat's chip counts, so a colour that leaves the + // legend takes every chip counted in it off the table. Measured on a three-handed + // table counting real chips: deleting the $500 row took $22,500 of $26,874 with it, + // on one click of a 24px button, with nothing said and nothing shown. + // + // It does not come back, either. A colour added again is a new colour with a new + // id, while the counts stay keyed to the one that left — so "put it back" restores + // the row and none of the money. + // + // Hence this: the cost is answerable before the click rather than discovered at the + // end of the night, when the box is short and nobody knows why. + // The number here is the *difference*, asked by pricing every seat twice — once with + // the colour and once without — rather than by multiplying the chips by their value. + // + // Those two are not the same, and the multiplication is the wrong one, because + // computeContribution() does not always reach the chips. A seat with a typed amount + // is priced from that and its chip counts are ignored, so a colour it holds is worth + // nothing to it; and a seat whose chips all leave falls through to the table's base + // pot — the standing ante — rather than to zero, so it loses less than its chips + // were worth. + // Both would put a number in the confirmation that the table then failed to match, + // which is worse than no number at all — this is a warning whose whole job is to be + // the truth. + chipColorUsage(state, chipId) { + const chip = (state.chipLegend || []).find(entry => entry.id === chipId); + if (!chip) return { chip: null, total: 0, chips: 0, seats: [] }; + + const legend = state.chipLegend || []; + const without = legend.filter(entry => entry.id !== chipId); + + const seats = state.seats + .map((player, index) => { + const before = this.computeContribution(player, legend, state.basePotAmount); + const after = this.computeContribution(player, without, state.basePotAmount); + return { + index, + name: this.displayName(player, index), + count: Number((player.chipCounts || {})[chipId]) || 0, + amount: Money.round(before - after), + }; + }) + // Signed, and not filtered to losses, because removing a colour can put the + // table *up*. A seat whose chips are worth less than the table's standing + // ante falls through to the ante once they are gone: found by the randomised + // check at 14 chips of a $1 colour against a $200 base pot, where deleting it + // moved the table by +186. Rare, but a warning that said "nothing changes" + // there would be as wrong as one that named the wrong number. + .filter(row => row.amount !== 0); + + return { + chip, + seats, + chips: seats.reduce((sum, row) => sum + row.count, 0), + total: Money.round(seats.reduce((sum, row) => sum + row.amount, 0)), + }; + }, + + // Take a colour out, and take its counts with it. + // + // Leaving them behind is what the old code did — the seats kept counts against an + // id no legend entry answers to any more, so the file carried money that nothing + // could price and nothing could show. Clearing them makes the saved table say what + // the screen says. The usage is measured before anything moves, so a caller can + // report what it cost. + removeChipColor(state, chipId) { + const usage = this.chipColorUsage(state, chipId); + if (!usage.chip) return usage; + + state.chipLegend = (state.chipLegend || []).filter(entry => entry.id !== chipId); + state.seats.forEach(player => { + if (player.chipCounts && chipId in player.chipCounts) delete player.chipCounts[chipId]; + }); + return usage; + }, + // Seats playing with chips the box has no cash against. // // Two kinds, and they are not the same problem. A seat holding the table's starting diff --git a/tests/holdem/README.md b/tests/holdem/README.md index 7762d9b..d0e48d5 100644 --- a/tests/holdem/README.md +++ b/tests/holdem/README.md @@ -26,6 +26,7 @@ other in the page, in the load order from `TexasHoldem.cshtml`. | `feltShape.test.js` | What shape the felt is at each phone size, resolved through the media queries in source order (see below) | | `potBreakdown.test.js` | How each pot says who can win it, and the nesting property that wording rests on (see below) | | `sidebarPanels.test.js` | What the sidebar's panels are called and what lives in each (see below) | +| `chipLegend.test.js` | Removing a chip colour, and whether the warning's number is the one the table loses (see below) | | `invariants.test.js` | 1000 randomised hands per property (see below) | ## The layout audit @@ -245,6 +246,52 @@ The panels are split on the opening tag rather than parsed, which is sound only sidebar panel never contains another one; Table Setup's sub-panels carry a different class. That assumption is worth knowing about if one ever does. +## A warning is only worth having if its number is right + +`chipLegend.test.js` covers removing a chip colour, which was one click of an unlabelled +24px button and was never a change to a legend. `computeContribution()` prices a seat by +running the legend over that seat's chip counts, so a colour that leaves takes every chip +counted in it off the table. Measured in a browser on a three-handed table: deleting the +$500 row took **$22,500 of $26,874** with it, with no confirmation, nothing said +afterwards, and no way back — a colour added again is a new colour with a new id, while +the counts stay keyed to the one that left. + +Most of the file is about the number in the warning rather than the removal, because a +warning that overstates is worse than none. The obvious way to work it out — chips times +value — is wrong in **both** directions: + +- a seat with a typed amount is priced from that and its counts are never read, so a + colour it holds is worth nothing to it; +- a seat whose chips all leave falls through to the table's standing ante rather than to + zero, so it loses less than its chips were worth. + +So the figure is the difference between pricing every seat with the colour and without +it, and a randomised check across 2,000 tables asserts that what the warning claims is +exactly what the table then does. That check found a third case nobody would have written +by hand: fourteen $1 chips against a $200 ante, where removing the colour puts the table +**up** $186. The wording says so in that direction too. + +The friction is conditional on purpose. A colour nobody has counted deletes on one click; +only one that is holding money asks. A confirmation that fires every time is one nobody +reads by the third night, and this is a tool where four other confirmations already +compete for that attention. + +## Every control says what it is + +`uiAudit.js` gained `controlNames()`, which is the general form of the bug above: the × +had no accessible name, so it was announced as a button and nothing else — five of them +in a column, identical, each deleting a different colour. + +It treats a symbol as no name even though `×` is technically text. The rule is that a +name has to contain a letter or a digit, which is the difference between "×" and "Undo". +Names are resolved the way the accessibility tree does it — `aria-label`, then +`aria-labelledby`, then a field's wrapping or pointing `