diff --git a/src/editor/context-menu.js b/src/editor/context-menu.js
index f9cd288..a35eb2b 100644
--- a/src/editor/context-menu.js
+++ b/src/editor/context-menu.js
@@ -79,7 +79,7 @@ export class ContextMenu {
this._ballBtn.appendChild(ballLabel);
this._ballBtn.addEventListener('click', (e) => {
e.stopPropagation();
- this._toggleBall();
+ this._assignBall();
});
// Delete button
@@ -150,10 +150,17 @@ export class ContextMenu {
this.menu.style.left = left + 'px';
this.menu.style.top = top + 'px';
- // Reflect has_ball state
- const entity = this.state.doc.entities?.[entityKey];
- if (this._ballBtn && entity) {
- this._ballBtn.classList.toggle('active', !!entity.has_ball);
+ // Ball possession only applies to players. Reflect whether this player
+ // currently holds the ball (i.e. the ball entity sits on top of them).
+ if (this._ballBtn) {
+ const type = pos.entity?.type;
+ const isPlayer = type === 'offense' || type === 'defense' || type === 'coach';
+ this._ballBtn.style.display = isPlayer ? '' : 'none';
+ const ballPos = positions.get('ball');
+ const holds = isPlayer && ballPos
+ && Math.abs(ballPos.x - pos.x) < 0.01
+ && Math.abs(ballPos.y - pos.y) < 0.01;
+ this._ballBtn.classList.toggle('active', !!holds);
}
}
@@ -161,22 +168,11 @@ export class ContextMenu {
this.menu.classList.add('hidden');
}
- _toggleBall() {
+ _assignBall() {
const key = this.state.selectedEntityKey;
if (!key) return;
- const entity = this.state.doc.entities?.[key];
- if (!entity) return;
-
- this.state.saveUndo();
- entity.has_ball = !entity.has_ball;
-
- // Only one player can have the ball at a time
- if (entity.has_ball) {
- for (const [k, e] of Object.entries(this.state.doc.entities || {})) {
- if (k !== key) e.has_ball = false;
- }
- }
-
- this.state.notify('has-ball-changed');
+ // Possession is modeled by the ball entity's position (OCF has a single
+ // ball). assignBall creates/moves the ball onto the selected player.
+ this.state.assignBall(key);
}
}
diff --git a/src/editor/editor.js b/src/editor/editor.js
index d2b4aaa..84ddfe2 100644
--- a/src/editor/editor.js
+++ b/src/editor/editor.js
@@ -4,7 +4,7 @@
*/
import { createBlankDocument } from '../schema.js';
-import { entityKey } from '../court/renderer.js';
+import { entityKey, resolveEntityPositions } from '../court/renderer.js';
const MAX_UNDO = 30;
@@ -142,13 +142,53 @@ export class EditorState {
}
}
- /** Get next available number for a given entity type. */
+ /**
+ * Get next available number for a given entity type.
+ * offense/defense are capped at 9 (schema limit); returns null if full.
+ */
nextEntityNr(type) {
const existing = this.doc.entities.filter(e => e.type === type).map(e => e.nr || 0);
- for (let i = 1; i <= 99; i++) {
+ const max = (type === 'offense' || type === 'defense') ? 9 : 99;
+ for (let i = 1; i <= max; i++) {
if (!existing.includes(i)) return i;
}
- return existing.length + 1;
+ return null;
+ }
+
+ /**
+ * Assign ball possession to a player by moving the ball entity onto them.
+ * Creates the ball entity if it does not exist yet. Since there is only one
+ * ball, possession is inherently exclusive. Honors the current frame: on
+ * frame 0 the base position is updated, otherwise a frame delta is written.
+ */
+ assignBall(playerKey) {
+ const positions = resolveEntityPositions(this.doc, this.currentFrameIndex);
+ const target = positions.get(playerKey);
+ if (!target) return;
+ const round = (v) => Math.round(v * 100) / 100;
+
+ this.saveUndo();
+
+ // Ensure a ball entity exists (base position seeded from frame 0).
+ let ball = this.doc.entities.find(e => e.type === 'ball');
+ if (!ball) {
+ const base = resolveEntityPositions(this.doc, 0).get(playerKey) || target;
+ ball = { type: 'ball', x: round(base.x), y: round(base.y) };
+ this.doc.entities.push(ball);
+ }
+
+ // Place the ball on the player for the current frame.
+ if (this.currentFrameIndex === 0) {
+ ball.x = round(target.x);
+ ball.y = round(target.y);
+ } else {
+ const frame = this.doc.frames[this.currentFrameIndex];
+ if (frame) {
+ if (!frame.entity_states) frame.entity_states = {};
+ frame.entity_states['ball'] = { x: round(target.x), y: round(target.y) };
+ }
+ }
+ this.notify('ball-assign');
}
// --- Line Operations ---
diff --git a/src/editor/interaction.js b/src/editor/interaction.js
index 8d8af76..f3fe1bd 100644
--- a/src/editor/interaction.js
+++ b/src/editor/interaction.js
@@ -3,7 +3,17 @@
*/
import { resolveEntityPositions, entityKey } from '../court/renderer.js';
-import { findSnapPosition } from '../court/positions.js';
+import { findSnapPosition, resolveCoordinate } from '../court/positions.js';
+
+/** Shortest distance from point p to line segment a–b (in pixels). */
+function distToSegment(p, a, b) {
+ const dx = b.x - a.x, dy = b.y - a.y;
+ const lenSq = dx * dx + dy * dy;
+ let t = lenSq === 0 ? 0 : ((p.x - a.x) * dx + (p.y - a.y) * dy) / lenSq;
+ t = Math.max(0, Math.min(1, t));
+ const cx = a.x + t * dx, cy = a.y + t * dy;
+ return Math.hypot(p.x - cx, p.y - cy);
+}
export class InteractionManager {
constructor(svgElement, editorState, getTransform) {
@@ -65,6 +75,34 @@ export class InteractionManager {
return closest;
}
+ /** Find which line (in the current frame) is at a given SVG position. */
+ _hitTestLine(svgX, svgY) {
+ const t = this.getTransform();
+ if (!t) return null;
+ const frame = this.state.doc.frames[this.state.currentFrameIndex];
+ if (!frame || !frame.lines) return null;
+ const ruleset = this.state.doc.court?.ruleset || 'fiba';
+ const customPos = this.state.doc.named_positions?.custom || {};
+ const threshold = 8; // hit distance in pixels
+ let best = null;
+ let bestDist = threshold;
+
+ for (let i = 0; i < frame.lines.length; i++) {
+ const pts = (frame.lines[i].coords || []).map(c => {
+ const abs = resolveCoordinate(c, ruleset, customPos);
+ return t.toSvg(abs.x, abs.y);
+ });
+ for (let s = 0; s + 1 < pts.length; s++) {
+ const d = distToSegment({ x: svgX, y: svgY }, pts[s], pts[s + 1]);
+ if (d < bestDist) {
+ bestDist = d;
+ best = i;
+ }
+ }
+ }
+ return best;
+ }
+
_onMouseDown(e) {
if (e.button !== 0) return;
if (this.state.activeTool !== 'select') return;
@@ -160,8 +198,13 @@ export class InteractionManager {
const hitKey = this._hitTestEntity(svgPt.x, svgPt.y);
if (hitKey) {
this.state.select(hitKey);
+ return;
+ }
+ // No entity hit — try lines, otherwise deselect.
+ const lineIdx = this._hitTestLine(svgPt.x, svgPt.y);
+ if (lineIdx !== null) {
+ this.state.selectLine(lineIdx);
} else if (!this.dragging || !this.dragging?.hasMoved) {
- // Check if we clicked a line (approximate)
this.state.deselect();
}
}
@@ -188,11 +231,14 @@ export class InteractionManager {
}
_onContextMenu(e) {
+ // Suppress the browser menu and instead select the entity so the
+ // floating action menu (with its Delete button) appears. Right-click no
+ // longer deletes immediately, which was surprising and destructive.
e.preventDefault();
const svgPt = this._svgPoint(e);
const hitKey = this._hitTestEntity(svgPt.x, svgPt.y);
if (hitKey) {
- this.state.removeEntity(hitKey);
+ this.state.select(hitKey);
}
}
@@ -228,7 +274,8 @@ export class InteractionManager {
entity = { type: 'station', nr: this.state.nextEntityNr('station'), x, y };
break;
}
- if (entity) this.state.addEntity(entity);
+ // nextEntityNr returns null when the numbered slots are exhausted.
+ if (entity && entity.nr !== null) this.state.addEntity(entity);
}
_onKeyDown(e) {
@@ -287,6 +334,10 @@ export class InteractionManager {
this.snapIndicator.setAttribute('stroke-width', '2');
this.snapIndicator.setAttribute('stroke-dasharray', '3,2');
this.snapIndicator.setAttribute('pointer-events', 'none');
+ }
+ // renderEditor() resets svg.innerHTML on every move, detaching the
+ // indicator — re-append it whenever it is not currently in the DOM.
+ if (!this.snapIndicator.isConnected) {
this.svg.appendChild(this.snapIndicator);
}
this.snapIndicator.setAttribute('cx', svgPt.x);
diff --git a/src/editor/palette.js b/src/editor/palette.js
index 0599f49..1d586a3 100644
--- a/src/editor/palette.js
+++ b/src/editor/palette.js
@@ -73,7 +73,8 @@ export class Palette {
entity = { type: 'station', nr: this.state.nextEntityNr('station'), x: pos.x + 2, y: pos.y };
break;
}
- if (entity) this.state.addEntity(entity);
+ // nextEntityNr returns null when the numbered slots are exhausted.
+ if (entity && entity.nr !== null) this.state.addEntity(entity);
}
}
diff --git a/src/main.js b/src/main.js
index 5263175..8e8ea08 100644
--- a/src/main.js
+++ b/src/main.js
@@ -5,7 +5,8 @@
import { renderOCF, resolveEntityPositions, entityKey } from './court/renderer.js';
import { createTransform } from './court/court-svg.js';
-import { straightPath } from './court/curves.js';
+import { straightPath, curvedPath } from './court/curves.js';
+import { resolveCoordinate } from './court/positions.js';
import { FramePlayer } from './player/player.js';
import { EditorState } from './editor/editor.js';
import { Palette } from './editor/palette.js';
@@ -20,11 +21,16 @@ import { exportJSON, importJSON, toJSONString } from './export/json.js';
let currentMode = 'editor'; // 'editor' | 'viewer'
let currentTransform = null;
+/** Current court transform (court units ↔ viewBox units). Exposed for tests. */
+export function getCurrentTransform() {
+ return currentTransform;
+}
+
// --- Editor State ---
-const editorState = new EditorState({ ruleset: 'fiba', courtType: 'half_court', drillFocus: 'offense' });
+export const editorState = new EditorState({ ruleset: 'fiba', courtType: 'half_court', drillFocus: 'offense' });
// --- Frame Player ---
-const player = new FramePlayer({
+export const player = new FramePlayer({
onFrameChange: (idx) => {
if (currentMode === 'viewer') renderViewer();
updatePlayerControls();
@@ -201,6 +207,29 @@ function renderEditor() {
}
}
+ // Selected line highlight
+ if (editorState.selectedLineIndex !== null && transform) {
+ const frame = editorState.doc.frames[editorState.currentFrameIndex];
+ const line = frame?.lines?.[editorState.selectedLineIndex];
+ if (line) {
+ const ruleset = editorState.doc.court?.ruleset || 'fiba';
+ const customPos = editorState.doc.named_positions?.custom || {};
+ const pts = (line.coords || []).map(c => {
+ const abs = resolveCoordinate(c, ruleset, customPos);
+ return transform.toSvg(abs.x, abs.y);
+ });
+ if (pts.length >= 2) {
+ const d = line.curved ? curvedPath(pts) : straightPath(pts);
+ if (d) {
+ overlay += ``;
+ }
+ for (const pt of pts) {
+ overlay += ``;
+ }
+ }
+ }
+ }
+
// Line drawing preview
if (editorState.lineWaypoints.length > 0 && transform) {
const pts = editorState.lineWaypoints.map(p => transform.toSvg(p.x, p.y));
diff --git a/tests/regressions.spec.js b/tests/regressions.spec.js
new file mode 100644
index 0000000..4c0e72a
--- /dev/null
+++ b/tests/regressions.spec.js
@@ -0,0 +1,231 @@
+/**
+ * Regression tests for the editor bugfixes:
+ * #1 ball-possession button (context menu) was a no-op
+ * #2 drawn lines could never be selected / edited / deleted
+ * #3 snap indicator vanished during drag (wiped by the per-move re-render)
+ * #4 right-click deleted an entity instead of selecting it
+ * + offense/defense numbering must stay capped at 9 (schema limit)
+ *
+ * These drive the real bundle via the exposed `window.OCFEditor` globals
+ * (`editorState`, `getCurrentTransform`).
+ */
+
+import { test, expect } from '@playwright/test';
+
+// ─── Helpers ─────────────────────────────────────────────────────────────────
+
+async function openEditor(page) {
+ await page.goto('/');
+ await expect(page.locator('#court-svg rect[fill="#d4a06a"]')).toBeVisible();
+}
+
+function paletteBtn(page, label) {
+ return page.locator('.palette-item .pal-label', { hasText: label }).locator('..');
+}
+
+/** Convert a court coordinate to a client (screen) point over the SVG.
+ * Mirrors InteractionManager._svgPoint: the handler reads `clientX - rect.left`
+ * and feeds it straight into the viewBox-based transform, so a viewBox unit maps
+ * to (box.x + svgX) regardless of any CSS scaling. */
+async function courtToClient(page, courtX, courtY) {
+ const svg = await page.evaluate(([cx, cy]) => {
+ const t = window.OCFEditor.getCurrentTransform();
+ return t.toSvg(cx, cy);
+ }, [courtX, courtY]);
+ const box = await page.locator('#court-svg').boundingBox();
+ return { x: box.x + svg.x, y: box.y + svg.y };
+}
+
+// ─── #1 Ball possession ───────────────────────────────────────────────────────
+
+test.describe('Regression #1: ball possession button', () => {
+ test('clicking Ball assigns the ball entity onto the selected player', async ({ page }) => {
+ await openEditor(page);
+ await paletteBtn(page, 'Offense').click();
+
+ // Select the player → the floating action menu appears.
+ await page.evaluate(() => window.OCFEditor.editorState.select('offense_1'));
+ const ballBtn = page.locator('#ctx-btn-ball');
+ await expect(ballBtn).toBeVisible();
+
+ await ballBtn.click();
+
+ const result = await page.evaluate(() => {
+ const doc = window.OCFEditor.editorState.doc;
+ const ball = doc.entities.find(e => e.type === 'ball');
+ const player = doc.entities.find(e => e.type === 'offense' && e.nr === 1);
+ return { ball, player };
+ });
+
+ // A ball entity now exists and sits on the player.
+ expect(result.ball).toBeTruthy();
+ expect(result.ball.x).toBeCloseTo(result.player.x, 5);
+ expect(result.ball.y).toBeCloseTo(result.player.y, 5);
+ // And the button reflects possession.
+ await expect(ballBtn).toHaveClass(/active/);
+ });
+
+ test('assigning the ball on a later frame writes a frame delta, not the base', async ({ page }) => {
+ await openEditor(page);
+ await paletteBtn(page, 'Offense').click();
+
+ await page.evaluate(() => {
+ const s = window.OCFEditor.editorState;
+ s.addFrame(); // currentFrameIndex -> 1
+ s.select('offense_1');
+ });
+ await page.locator('#ctx-btn-ball').click();
+
+ const res = await page.evaluate(() => {
+ const doc = window.OCFEditor.editorState.doc;
+ return {
+ delta: doc.frames[1].entity_states?.ball || null,
+ ballExists: doc.entities.some(e => e.type === 'ball'),
+ frame0HasDelta: !!doc.frames[0].entity_states,
+ };
+ });
+
+ expect(res.ballExists).toBe(true);
+ expect(res.delta).toBeTruthy(); // possession recorded as a frame delta
+ expect(res.frame0HasDelta).toBe(false); // base frame untouched
+ });
+});
+
+// ─── #2 Line selection / editing / deletion ─────────────────────────────────────
+
+test.describe('Regression #2: lines are selectable and deletable', () => {
+ async function seedLine(page) {
+ await page.evaluate(() => {
+ const s = window.OCFEditor.editorState;
+ const doc = JSON.parse(JSON.stringify(s.doc));
+ doc.frames[0].lines = [
+ { type: 'movement', curved: false, coords: [{ x: -4, y: 6 }, { x: 4, y: 6 }] },
+ ];
+ s.loadDocument(doc);
+ });
+ }
+
+ test('clicking a drawn line selects it and shows its properties', async ({ page }) => {
+ await openEditor(page);
+ await seedLine(page);
+
+ const mid = await courtToClient(page, 0, 6); // midpoint of the line
+ await page.mouse.click(mid.x, mid.y);
+
+ const selected = await page.evaluate(() => window.OCFEditor.editorState.selectedLineIndex);
+ expect(selected).toBe(0);
+ await expect(page.locator('#properties .prop-title')).toHaveText(/Line/);
+ });
+
+ test('a selected line can be deleted with the Delete key', async ({ page }) => {
+ await openEditor(page);
+ await seedLine(page);
+
+ const mid = await courtToClient(page, 0, 6);
+ await page.mouse.click(mid.x, mid.y);
+ await expect.poll(() =>
+ page.evaluate(() => window.OCFEditor.editorState.selectedLineIndex),
+ ).toBe(0);
+
+ await page.keyboard.press('Delete');
+
+ const remaining = await page.evaluate(
+ () => window.OCFEditor.editorState.doc.frames[0].lines.length,
+ );
+ expect(remaining).toBe(0);
+ });
+});
+
+// ─── #3 Snap indicator ──────────────────────────────────────────────────────────
+
+test.describe('Regression #3: snap indicator survives the per-move re-render', () => {
+ test('snap indicator stays in the DOM across consecutive snapping moves', async ({ page }) => {
+ await openEditor(page);
+ await paletteBtn(page, 'Offense').click(); // fiba default: offense_1 at (0, 7)
+
+ // Drive the real InteractionManager handlers with synthetic mouse events so
+ // the sequence is deterministic. The handler reads `clientX - rect.left` and
+ // feeds it straight into the viewBox transform, so a viewBox unit maps to
+ // (rect.left + svgX). We snap onto two named positions in a row.
+ const result = await page.evaluate(() => {
+ const svg = document.getElementById('court-svg');
+ const t = window.OCFEditor.getCurrentTransform();
+ const rect = svg.getBoundingClientRect();
+ const fire = (type, c, buttons) => svg.dispatchEvent(new MouseEvent(type, {
+ bubbles: true, cancelable: true, view: window,
+ clientX: rect.left + c.x, clientY: rect.top + c.y, button: 0, buttons,
+ }));
+ const isConnected = () => {
+ const c = svg.querySelector('circle[stroke="#00cc66"]');
+ return !!c && c.isConnected;
+ };
+
+ const start = t.toSvg(0, 7); // the player
+ const snap1 = t.toSvg(0, 8.2); // free_throw_line (fiba)
+ const snap2 = t.toSvg(0, 5.68); // top_of_the_key (fiba)
+
+ fire('mousedown', start, 1);
+ fire('mousemove', snap1, 1); // first snap → indicator created + appended
+ const afterFirst = isConnected();
+ fire('mousemove', snap2, 1); // second snap → re-render wipes innerHTML
+ const afterSecond = isConnected();
+ fire('mouseup', snap2, 0);
+
+ const player = window.OCFEditor.editorState.doc.entities.find(e => e.type === 'offense');
+ return { afterFirst, afterSecond, movedTo: player.y };
+ });
+
+ // Sanity: the drag actually snapped the player onto the named position.
+ expect(result.movedTo).toBeCloseTo(5.68, 5);
+ expect(result.afterFirst).toBe(true);
+ // The regression: before the fix the indicator was detached by the
+ // per-move svg.innerHTML reset and never re-appended.
+ expect(result.afterSecond).toBe(true);
+ });
+});
+
+// ─── #4 Right-click ─────────────────────────────────────────────────────────────
+
+test.describe('Regression #4: right-click selects instead of deleting', () => {
+ test('right-clicking a player keeps it and selects it', async ({ page }) => {
+ await openEditor(page);
+ await paletteBtn(page, 'Offense').click();
+
+ const before = await page.evaluate(
+ () => window.OCFEditor.editorState.doc.entities.length,
+ );
+
+ const at = await courtToClient(page, 0, 7); // the offense default position
+ await page.mouse.click(at.x, at.y, { button: 'right' });
+
+ const after = await page.evaluate(() => ({
+ count: window.OCFEditor.editorState.doc.entities.length,
+ selected: window.OCFEditor.editorState.selectedEntityKey,
+ }));
+
+ expect(after.count).toBe(before); // not deleted
+ expect(after.selected).toBe('offense_1'); // selected instead
+ });
+});
+
+// ─── Numbering cap ──────────────────────────────────────────────────────────────
+
+test.describe('Regression: offense/defense numbering capped at 9', () => {
+ test('the 10th offense slot is unavailable (null) and cones are unaffected', async ({ page }) => {
+ await openEditor(page);
+ const result = await page.evaluate(() => {
+ const s = window.OCFEditor.editorState;
+ for (let i = 1; i <= 9; i++) {
+ s.addEntity({ type: 'offense', nr: s.nextEntityNr('offense'), x: 0, y: i });
+ }
+ return {
+ count: s.doc.entities.length,
+ next: s.nextEntityNr('offense'),
+ cone: s.nextEntityNr('cone'),
+ };
+ });
+ expect(result.count).toBe(9);
+ expect(result.next).toBeNull();
+ expect(result.cone).toBe(1);
+ });
+});