Skip to content

fix(ui): deleted units shouldn't be considered dead or alive #453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fank merged 1 commit into from
May 24, 2026
Merged

fix(ui): deleted units shouldn't be considered dead or alive #453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 149 additions & 7 deletions ui/src/pages/recording-playback/__tests__/UnitsTab.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ describe("UnitsTab", () => {
expect(twos.length).toBeGreaterThanOrEqual(1);
});

it("counts deleted units as dead and styles them", () => {
it("styles a despawned unit (no kill event) as inactive", () => {
const { engine, renderer } = createTestEngine();
engine.loadRecording(
makeManifest([
Expand All @@ -254,7 +254,6 @@ describe("UnitsTab", () => {
]),
);

// Advance to frame 1 so snapshots are populated
engine.seekTo(1);

render(() => (
Expand All @@ -263,12 +262,155 @@ describe("UnitsTab", () => {
</TestProviders>
));

// Deleted unit counts as dead: alive=1, total=2
expect(screen.getByText("1")).toBeTruthy(); // alive count in group header

// Deleted unit row must have the dead styling
// Unit despawned without a kill event → inactive styling, not dead
const deletedRow = screen.getByText("Deleted Unit").closest("button");
expect(deletedRow?.className).toMatch(/unitRowDead/);
expect(deletedRow?.className).toMatch(/unitRowInactive/);
expect(deletedRow?.className).not.toMatch(/unitRowDead/);
});

it("styles a unit killed by a kill event as dead", () => {
const { engine, renderer } = createTestEngine();
engine.loadRecording(
makeManifest(
[
unitDef({
id: 1,
name: "Victim",
side: "WEST",
groupName: "Alpha",
role: "Trooper",
positions: [
{ position: [100, 200], direction: 0, alive: 1 },
{ position: [100, 200], direction: 0, alive: 0 }, // dead at frame 1
],
}),
unitDef({
id: 2,
name: "Killer",
side: "EAST",
groupName: "Bravo",
role: "Trooper",
positions: [
{ position: [50, 50], direction: 0, alive: 1 },
{ position: [50, 50], direction: 0, alive: 1 },
],
}),
],
[killedEvent(1, 1, 2)],
),
);

setActiveSide("WEST");
engine.seekTo(1);

render(() => (
<TestProviders engine={engine} renderer={renderer}>
<UnitsTab />
</TestProviders>
));

const row = screen.getByText("Victim").closest("button");
expect(row?.className).toMatch(/unitRowDead/);
expect(row?.className).not.toMatch(/unitRowInactive/);
});

it("keeps dead styling for a killed unit even after body despawns", () => {
const { engine, renderer } = createTestEngine();
engine.loadRecording(
makeManifest(
[
unitDef({
id: 1,
name: "Victim",
side: "WEST",
groupName: "Alpha",
role: "Trooper",
endFrame: 3, // body despawns at frame 3
positions: [
{ position: [100, 200], direction: 0, alive: 1 },
{ position: [100, 200], direction: 0, alive: 0 },
{ position: [100, 200], direction: 0, alive: 0 },
{ position: [100, 200], direction: 0, alive: 0 },
],
}),
unitDef({
id: 2,
name: "Killer",
side: "EAST",
groupName: "Bravo",
role: "Trooper",
positions: [
{ position: [50, 50], direction: 0, alive: 1 },
{ position: [50, 50], direction: 0, alive: 1 },
],
}),
],
[killedEvent(1, 1, 2)],
),
);

setActiveSide("WEST");
engine.seekTo(10); // well beyond endFrame=3, no snapshot for unit 1

render(() => (
<TestProviders engine={engine} renderer={renderer}>
<UnitsTab />
</TestProviders>
));

// No snapshot but deaths=1 → still dead, not inactive
const row = screen.getByText("Victim").closest("button");
expect(row?.className).toMatch(/unitRowDead/);
expect(row?.className).not.toMatch(/unitRowInactive/);
});

it("styles a respawned unit as alive even when they have prior deaths", () => {
const { engine, renderer } = createTestEngine();
engine.loadRecording(
makeManifest(
[
unitDef({
id: 1,
name: "Respawner",
side: "WEST",
groupName: "Alpha",
role: "Trooper",
positions: [
{ position: [100, 200], direction: 0, alive: 1 },
{ position: [100, 200], direction: 0, alive: 0 }, // killed
{ position: [200, 300], direction: 0, alive: 1 }, // respawned
],
}),
unitDef({
id: 2,
name: "Killer",
side: "EAST",
groupName: "Bravo",
role: "Trooper",
positions: [
{ position: [50, 50], direction: 0, alive: 1 },
{ position: [50, 50], direction: 0, alive: 1 },
{ position: [50, 50], direction: 0, alive: 1 },
],
}),
],
[killedEvent(1, 1, 2)],
),
);

setActiveSide("WEST");
engine.seekTo(2); // respawned frame — alive=1 in snapshot, deaths=1 in events

render(() => (
<TestProviders engine={engine} renderer={renderer}>
<UnitsTab />
</TestProviders>
));

const row = screen.getByText("Respawner").closest("button");
// Alive in snapshot takes priority — no dead or inactive class
expect(row?.className).not.toMatch(/unitRowDead/);
expect(row?.className).not.toMatch(/unitRowInactive/);
});

it("only renders populated side tabs when multiple sides have units", () => {
Expand Down
10 changes: 9 additions & 1 deletion ui/src/pages/recording-playback/components/SidePanel.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,10 @@
background: color-mix(in srgb, var(--accent-primary) 8%, transparent);
}

.unitRowInactive {
opacity: 0.45;
}

.unitRowDead {
opacity: 0.45;
}
Expand Down Expand Up @@ -245,10 +249,14 @@
color: var(--text-secondary);
}

.unitNameDead {
.unitNameInactive {
color: var(--text-dimmer);
}

.unitNameDead {
color: var(--accent-warning);
}

.unitAiBadge {
font-size: var(--font-size-xs);
color: var(--text-dimmest);
Expand Down
27 changes: 16 additions & 11 deletions ui/src/pages/recording-playback/components/UnitsTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,18 @@ export function UnitsTab(props: UnitsTabProps): JSX.Element {
}
});

const isAlive = (unitId: number): boolean => {
const snap = engine.entitySnapshots().get(unitId);
return snap ? !!snap.alive : false;
};

// Frame-aware kill counts
const killDeathCounts = createMemo(() =>
engine.eventManager.getKillDeathCounts(engine.currentFrame()),
);

const getUnitStatus = (unitId: number): "alive" | "dead" | "inactive" => {
const snap = engine.entitySnapshots().get(unitId);
if (snap && snap.alive) return "alive";
if ((killDeathCounts().deaths.get(unitId) ?? 0) > 0) return "dead";
return "inactive";
};

const groups = createMemo((): GroupData[] => {
const units = unitsForSide(activeSide());
const groupMap = new Map<string, Unit[]>();
Expand Down Expand Up @@ -108,11 +110,12 @@ export function UnitsTab(props: UnitsTabProps): JSX.Element {
};

const aliveCount = (units: Unit[]): number => {
// Access snapshots for reactivity
// Access both reactive sources so this recomputes on snapshot or kill-event changes
engine.entitySnapshots();
killDeathCounts();
let count = 0;
for (const u of units) {
if (isAlive(u.id)) count++;
if (getUnitStatus(u.id) === "alive") count++;
}
return count;
};
Expand Down Expand Up @@ -185,15 +188,16 @@ export function UnitsTab(props: UnitsTabProps): JSX.Element {
<Show when={expanded()}>
<For each={group.units}>
{(unit) => {
const alive = () => isAlive(unit.id);
const status = () => getUnitStatus(unit.id);
const selected = () => selectedUnit() === unit.id;
return (
<>
<button
class={styles.unitRow}
classList={{
[styles.unitRowSelected]: selected(),
[styles.unitRowDead]: !alive(),
[styles.unitRowDead]: status() === "dead",
[styles.unitRowInactive]: status() === "inactive",
}}
onClick={() =>
setSelectedUnit(selected() ? null : unit.id)
Expand All @@ -211,8 +215,9 @@ export function UnitsTab(props: UnitsTabProps): JSX.Element {
<span
class={styles.unitName}
classList={{
[styles.unitNameAlive]: alive(),
[styles.unitNameDead]: !alive(),
[styles.unitNameAlive]: status() === "alive",
[styles.unitNameDead]: status() === "dead",
[styles.unitNameInactive]: status() === "inactive",
}}
>
{unit.name || `Unit ${unit.id}`}
Expand Down
Loading