Fixes: adv/dis as a global toggle, auto-number duplicate combatants

- Advantage/Disadvantage are now toggle modes (rollStore.mode) applied to d20
  rolls everywhere (dice page, sheet skills/saves/attacks), preserving modifiers;
  applyRollMode() rewrites the first d20 -> 2d20kh1/kl1
- addCombatant auto-numbers duplicate names (Goblin 1, Goblin 2, …); the first
  duplicate also renames the original. Quantity add relies on this.
- Tests for applyRollMode + duplicate numbering

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/features/combat/EncounterTracker.tsx

@@ -202,7 +202,7 @@ function AddCombatantBar({
     for (let i = 0; i < n; i++) {
       onAdd({
         id: newId(),
-        name: n > 1 ? `${name.trim()} ${i + 1}` : name.trim(),
+        name: name.trim(), // engine auto-numbers duplicates
         kind: 'monster',
         initiative: init,
         initBonus: 0,

src/features/dice/DicePage.tsx

@@ -1,10 +1,11 @@
 import { useEffect, useState } from 'react';
 import { useLiveQuery } from 'dexie-react-hooks';
-import { rollDice, DiceParseError, type RollResult } from '@/lib/dice/notation';
+import { rollDice, applyRollMode, DiceParseError, type RollResult } from '@/lib/dice/notation';
 import { createRng } from '@/lib/rng';
 import { diceRepo } from '@/lib/db/repositories';
 import { useUiStore } from '@/stores/uiStore';
 import { useMacroStore } from '@/stores/macroStore';
+import { useRollStore } from '@/stores/rollStore';
 import { Page, PageHeader } from '@/components/ui/Page';
 import { Button } from '@/components/ui/Button';
 import { Input } from '@/components/ui/Input';
@@ -26,16 +27,21 @@ export function DicePage() {
   const addMacro = useMacroStore((s) => s.add);
   const removeMacro = useMacroStore((s) => s.remove);
 
+  const mode = useRollStore((s) => s.mode);
+  const toggleMode = useRollStore((s) => s.toggleMode);
+
   const roll = async (expression: string, label = '') => {
     try {
-      const result = rollDice(expression, createRng());
+      const finalExpr = applyRollMode(expression, mode);
+      const result = rollDice(finalExpr, createRng());
       setLast(result);
       setRollCount((n) => n + 1);
       setError(null);
+      const modeTag = mode === 'advantage' ? ' (adv)' : mode === 'disadvantage' ? ' (dis)' : '';
       await diceRepo.add({
         ...(campaignId ? { campaignId } : {}),
-        label,
-        expression,
+        label: label + modeTag,
+        expression: finalExpr,
         total: result.total,
         breakdown: result.breakdown,
       });
@@ -81,13 +87,30 @@ export function DicePage() {
                 {d}
               </Button>
             ))}
-            <Button size="sm" variant="secondary" onClick={() => void roll('2d20kh1', 'advantage')}>
+            <Button
+              size="sm"
+              variant={mode === 'advantage' ? 'primary' : 'secondary'}
+              aria-pressed={mode === 'advantage'}
+              onClick={() => toggleMode('advantage')}
+              title="Toggle advantage for d20 rolls"
+            >
               Advantage
             </Button>
-            <Button size="sm" variant="secondary" onClick={() => void roll('2d20kl1', 'disadvantage')}>
+            <Button
+              size="sm"
+              variant={mode === 'disadvantage' ? 'primary' : 'secondary'}
+              aria-pressed={mode === 'disadvantage'}
+              onClick={() => toggleMode('disadvantage')}
+              title="Toggle disadvantage for d20 rolls"
+            >
               Disadvantage
             </Button>
           </div>
+          {mode !== 'normal' && (
+            <p className="mt-2 text-xs text-accent">
+              {mode === 'advantage' ? 'Advantage' : 'Disadvantage'} is on — d20 rolls use 2d20{mode === 'advantage' ? 'kh1' : 'kl1'}.
+            </p>
+          )}
 
           {/* Saved rolls (macros) */}
           <div className="mt-4">

src/lib/combat/engine.test.ts

@@ -89,6 +89,23 @@ describe('mutating mid-combat keeps the turn anchored (C16-C19 regressions)', ()
     expect(e.round).toBe(2);
   });
 
+  it('auto-numbers duplicate names (Goblin -> Goblin 1, Goblin 2, …)', () => {
+    let e = encounter([]);
+    e = addCombatant(e, mk('g1', 10));
+    e = { ...e, combatants: e.combatants.map((c) => ({ ...c, name: 'Goblin' })) }; // first is "Goblin"
+    e = addCombatant(e, { ...mk('g2', 10), name: 'Goblin' });
+    expect(e.combatants.map((c) => c.name).sort()).toEqual(['Goblin 1', 'Goblin 2']);
+    e = addCombatant(e, { ...mk('g3', 10), name: 'Goblin' });
+    expect(e.combatants.find((c) => c.id === 'g3')?.name).toBe('Goblin 3');
+  });
+
+  it('does not number unique names', () => {
+    let e = encounter([]);
+    e = addCombatant(e, { ...mk('a', 10), name: 'Aragorn' });
+    e = addCombatant(e, { ...mk('b', 10), name: 'Legolas' });
+    expect(e.combatants.map((c) => c.name)).toEqual(['Aragorn', 'Legolas']);
+  });
+
   it('adding a combatant mid-combat inserts by initiative without changing whose turn it is', () => {
     let e = startEncounter(encounter([mk('a', 30), mk('b', 10)]));
     e = nextTurn(e); // current = b

src/lib/combat/engine.ts

@@ -106,17 +106,51 @@ export function previousTurn(enc: Encounter): Encounter {
   };
 }
 
+/** Strip a trailing " 3" style number so "Goblin 2" and "Goblin" share a base. */
+function baseName(name: string): string {
+  return name.replace(/\s+\d+$/, '').trim();
+}
+
 /**
- * Insert a combatant. When the encounter is active, the new combatant is placed
- * by initiative and the turn pointer is corrected so it still points at the
- * same combatant whose turn it is.
+ * Disambiguate a new combatant's name against existing ones: the first duplicate
+ * renames the original to "Name 1" and the newcomer becomes "Name 2", etc.
+ * Returns the (possibly renamed) existing list and the new combatant's name.
+ */
+function disambiguate(existing: Combatant[], name: string): { combatants: Combatant[]; name: string } {
+  const base = baseName(name);
+  const sameBase = existing.filter((c) => baseName(c.name) === base);
+  if (sameBase.length === 0) return { combatants: existing, name };
+
+  let combatants = existing;
+  // If there's exactly one and it's unnumbered, number it "1" so the group reads 1,2,…
+  if (sameBase.length === 1 && !/\s+\d+$/.test(sameBase[0]!.name)) {
+    const id = sameBase[0]!.id;
+    combatants = existing.map((c) => (c.id === id ? { ...c, name: `${base} 1` } : c));
+  }
+  const maxNum = Math.max(
+    1,
+    ...sameBase.map((c) => {
+      const m = /\s+(\d+)$/.exec(c.name);
+      return m ? Number(m[1]) : 0;
+    }),
+  );
+  return { combatants, name: `${base} ${maxNum + 1}` };
+}
+
+/**
+ * Insert a combatant. Auto-numbers duplicate names (Goblin 1, Goblin 2, …).
+ * When the encounter is active, the new combatant is placed by initiative and
+ * the turn pointer is corrected so it still points at whose turn it is.
  */
 export function addCombatant(enc: Encounter, combatant: Combatant): Encounter {
+  const { combatants: renamed, name } = disambiguate(enc.combatants, combatant.name);
+  const incoming = name === combatant.name ? combatant : { ...combatant, name };
+
   if (enc.status !== 'active') {
-    return { ...enc, combatants: [...enc.combatants, combatant] };
+    return { ...enc, combatants: [...renamed, incoming] };
   }
   const currentId = currentCombatant(enc)?.id;
-  const combatants = sortByInitiative([...enc.combatants, combatant]);
+  const combatants = sortByInitiative([...renamed, incoming]);
   const turnIndex = Math.max(0, indexOfId(combatants, currentId));
   return { ...enc, combatants, turnIndex };
 }

src/lib/dice/notation.test.ts

@@ -1,7 +1,21 @@
 import { describe, it, expect } from 'vitest';
-import { parseDice, rollDice, DiceParseError } from './notation';
+import { parseDice, rollDice, applyRollMode, DiceParseError } from './notation';
 import { createRng } from '@/lib/rng';
 
+describe('applyRollMode', () => {
+  it('turns a d20 into 2d20kh1 for advantage, keeping modifiers', () => {
+    expect(applyRollMode('1d20+5', 'advantage')).toBe('2d20kh1+5');
+    expect(applyRollMode('d20', 'disadvantage')).toBe('2d20kl1');
+  });
+  it('leaves non-d20 and normal rolls unchanged', () => {
+    expect(applyRollMode('2d6+3', 'advantage')).toBe('2d6+3');
+    expect(applyRollMode('1d20+5', 'normal')).toBe('1d20+5');
+  });
+  it('only transforms the first d20 term', () => {
+    expect(applyRollMode('1d20+1d20', 'advantage')).toBe('2d20kh1+1d20');
+  });
+});
+
 describe('parseDice', () => {
   it('parses a simple die group', () => {
     expect(parseDice('3d6')).toEqual([{ kind: 'dice', sign: 1, count: 3, sides: 6 }]);

src/lib/dice/notation.ts

@@ -48,6 +48,24 @@ export interface RollResult {
 
 export class DiceParseError extends Error {}
 
+export type RollMode = 'normal' | 'advantage' | 'disadvantage';
+
+/**
+ * Apply advantage/disadvantage to a d20 expression by turning the first d20 term
+ * into 2d20kh1 / 2d20kl1. Non-d20 expressions are returned unchanged, so the
+ * toggle only affects d20 checks (and keeps any modifiers, e.g. 1d20+5).
+ */
+export function applyRollMode(expression: string, mode: RollMode): string {
+  if (mode === 'normal') return expression;
+  const keep = mode === 'advantage' ? 'kh1' : 'kl1';
+  let replaced = false;
+  return expression.replace(/(^|[+\-\s])\d*d20\b/i, (m, pre: string) => {
+    if (replaced) return m;
+    replaced = true;
+    return `${pre}2d20${keep}`;
+  });
+}
+
 const MAX_DICE = 1000;
 const MAX_SIDES = 1000;
 

src/lib/useRoll.ts

@@ -1,4 +1,4 @@
-import { rollDice } from '@/lib/dice/notation';
+import { rollDice, applyRollMode } from '@/lib/dice/notation';
 import { createRng } from '@/lib/rng';
 import { degreeOfSuccess, type Degree } from '@/lib/dice/check';
 import { diceRepo } from '@/lib/db/repositories';
@@ -30,9 +30,12 @@ function naturalD20(result: ReturnType<typeof rollDice>): number | undefined {
  * Returns the result + degree. Used by the dice page, sheets, and statblocks.
  */
 export function rollAndShow(opts: RollOptions): { total: number; degree?: Degree } {
-  const result = rollDice(opts.expression, createRng());
+  const mode = useRollStore.getState().mode;
+  const expression = applyRollMode(opts.expression, mode);
+  const result = rollDice(expression, createRng());
   const campaignId = useUiStore.getState().activeCampaignId ?? undefined;
-  const label = opts.label ?? '';
+  const modeTag = mode === 'advantage' ? ' (adv)' : mode === 'disadvantage' ? ' (dis)' : '';
+  const label = (opts.label ?? '') + modeTag;
 
   let degree: Degree | undefined;
   if (opts.dc !== undefined && opts.system) {

src/stores/rollStore.ts

@@ -1,5 +1,5 @@
 import { create } from 'zustand';
-import type { RollResult } from '@/lib/dice/notation';
+import type { RollResult, RollMode } from '@/lib/dice/notation';
 import type { Degree } from '@/lib/dice/check';
 
 export interface TrayRoll {
@@ -13,13 +13,21 @@ export interface TrayRoll {
 
 interface RollState {
   last: TrayRoll | null;
+  /** advantage/disadvantage toggle applied to d20 rolls everywhere */
+  mode: RollMode;
   push: (roll: Omit<TrayRoll, 'seq'>) => void;
   dismiss: () => void;
+  setMode: (mode: RollMode) => void;
+  /** click the same mode again to turn it off */
+  toggleMode: (mode: Exclude<RollMode, 'normal'>) => void;
 }
 
 let seq = 0;
-export const useRollStore = create<RollState>((set) => ({
+export const useRollStore = create<RollState>((set, get) => ({
   last: null,
+  mode: 'normal',
   push: (roll) => set({ last: { ...roll, seq: ++seq } }),
   dismiss: () => set({ last: null }),
+  setMode: (mode) => set({ mode }),
+  toggleMode: (mode) => set({ mode: get().mode === mode ? 'normal' : mode }),
 }));