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.
This commit is contained in:
@@ -112,6 +112,49 @@ describe("usePlayerCharacters", () => {
|
|||||||
expect(result.current.characters[0].name).toBe("Vex'ahlia");
|
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", () => {
|
it("deleteCharacter removes character and persists", () => {
|
||||||
const { result } = renderHook(() => usePlayerCharacters(), { wrapper });
|
const { result } = renderHook(() => usePlayerCharacters(), { wrapper });
|
||||||
|
|
||||||
|
|||||||
@@ -9,10 +9,18 @@ import { isDomainError, playerCharacterId } from "@initiative/domain";
|
|||||||
import { useCallback, useEffect, useRef, useState } from "react";
|
import { useCallback, useEffect, useRef, useState } from "react";
|
||||||
import { useAdapters } from "../contexts/adapter-context.js";
|
import { useAdapters } from "../contexts/adapter-context.js";
|
||||||
|
|
||||||
let nextPcId = 0;
|
const PC_ID_PATTERN = /^pc-(\d+)$/;
|
||||||
|
|
||||||
function generatePcId(): PlayerCharacterId {
|
function generatePcId(existing: readonly PlayerCharacter[]): PlayerCharacterId {
|
||||||
return playerCharacterId(`pc-${++nextPcId}`);
|
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 {
|
interface EditFields {
|
||||||
@@ -55,7 +63,7 @@ export function usePlayerCharacters() {
|
|||||||
icon: string | undefined,
|
icon: string | undefined,
|
||||||
level: number | undefined,
|
level: number | undefined,
|
||||||
) => {
|
) => {
|
||||||
const id = generatePcId();
|
const id = generatePcId(charactersRef.current);
|
||||||
const result = createPlayerCharacterUseCase(
|
const result = createPlayerCharacterUseCase(
|
||||||
makeStore(),
|
makeStore(),
|
||||||
id,
|
id,
|
||||||
|
|||||||
Reference in New Issue
Block a user