From 3b2fb99b37220fa9ddb197b26f1f0cb4712e9eba Mon Sep 17 00:00:00 2001
From: Lukas <lukas.richter@maibornwolff.de>
Date: Fri, 19 Jun 2026 16:36:20 +0200
Subject: [PATCH] Fix duplicate player character ids after page reload

The PC id counter lived in a module-level let that reset to 0 on
every page load. After rehydrating PCs from localStorage, the next
create would hand out pc-1 again, colliding with an existing id.
That broke React's keyed reconciliation and caused the wrong PC
to be deleted (deletePlayerCharacter matches the first occurrence
of the id, so deleting the new pc-1 would remove the rehydrated
one instead).

Derive the next id from the max numeric suffix of existing
characters at the moment of creation. No more shared counter, so
no more reset on reload and no collision after import.
---
 .../__tests__/use-player-characters.test.tsx  | 43 +++++++++++++++++++
 apps/web/src/hooks/use-player-characters.ts   | 16 +++++--
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/apps/web/src/hooks/__tests__/use-player-characters.test.tsx b/apps/web/src/hooks/__tests__/use-player-characters.test.tsx
index c79457d..101243e 100644
--- a/apps/web/src/hooks/__tests__/use-player-characters.test.tsx
+++ b/apps/web/src/hooks/__tests__/use-player-characters.test.tsx
@@ -112,6 +112,49 @@ describe("usePlayerCharacters", () => {
 		expect(result.current.characters[0].name).toBe("Vex'ahlia");
 	});
 
+	it("createCharacter assigns a fresh id after rehydration from persistence", () => {
+		const stored = [
+			{
+				id: playerCharacterId("pc-1"),
+				name: "Mikka",
+				ac: 12,
+				maxHp: 58,
+				color: undefined,
+				icon: undefined,
+			},
+			{
+				id: playerCharacterId("pc-3"),
+				name: "Bob",
+				ac: 14,
+				maxHp: 40,
+				color: undefined,
+				icon: undefined,
+			},
+		];
+		const adapters = createTestAdapters({ playerCharacters: stored });
+
+		const { result } = renderHook(() => usePlayerCharacters(), {
+			wrapper: ({ children }: { children: ReactNode }) => (
+				<AllProviders adapters={adapters}>{children}</AllProviders>
+			),
+		});
+
+		act(() => {
+			result.current.createCharacter(
+				"Charlie",
+				13,
+				25,
+				undefined,
+				undefined,
+				undefined,
+			);
+		});
+
+		const ids = result.current.characters.map((pc) => pc.id);
+		expect(new Set(ids).size).toBe(ids.length);
+		expect(ids).toContain(playerCharacterId("pc-4"));
+	});
+
 	it("deleteCharacter removes character and persists", () => {
 		const { result } = renderHook(() => usePlayerCharacters(), { wrapper });
 
diff --git a/apps/web/src/hooks/use-player-characters.ts b/apps/web/src/hooks/use-player-characters.ts
index 12638fa..9a747e0 100644
--- a/apps/web/src/hooks/use-player-characters.ts
+++ b/apps/web/src/hooks/use-player-characters.ts
@@ -9,10 +9,18 @@ import { isDomainError, playerCharacterId } from "@initiative/domain";
 import { useCallback, useEffect, useRef, useState } from "react";
 import { useAdapters } from "../contexts/adapter-context.js";
 
-let nextPcId = 0;
+const PC_ID_PATTERN = /^pc-(\d+)$/;
 
-function generatePcId(): PlayerCharacterId {
-	return playerCharacterId(`pc-${++nextPcId}`);
+function generatePcId(existing: readonly PlayerCharacter[]): PlayerCharacterId {
+	let max = 0;
+	for (const pc of existing) {
+		const match = PC_ID_PATTERN.exec(pc.id);
+		if (match) {
+			const n = Number(match[1]);
+			if (n > max) max = n;
+		}
+	}
+	return playerCharacterId(`pc-${max + 1}`);
 }
 
 interface EditFields {
@@ -55,7 +63,7 @@ export function usePlayerCharacters() {
 			icon: string | undefined,
 			level: number | undefined,
 		) => {
-			const id = generatePcId();
+			const id = generatePcId(charactersRef.current);
 			const result = createPlayerCharacterUseCase(
 				makeStore(),
 				id,