Files
stickman/specs/phase_9_prop_editor_round2.md
ryan b66ded5f60 Add headless regression tests for Prop Editor functionality
- Implement tests for PropFile, including preset conversion, file dictionary conversion, round-trip serialization, version rejection, default values for missing keys, flattening, sanitization, and save/load functionality.
- Create tests for PropEditor, covering window title formatting, mode button text, safe saving, and recent file list management.
- Add tests for PropTheme, validating fallback behavior, invalid input handling, and default values for primitives and UI tokens.
- Introduce tests for ThumbnailCache, ensuring proper key formatting, PNG handling, and behavior with missing files.
2026-09-19 21:26:03 -04:00

23 KiB
Raw Permalink Blame History

Phase 9 Prop Editor — Round 2 (Usability & Interaction Fixes)

Status: approved (2026-09-18; all open choices resolved — two deviations: "Create Point" stays outline-only, and prop_theme.json keeps version "1.0").

Source: five user feedback points against the shipped Round 1 Prop Editor (scripts/prop_canvas.gd, scripts/prop_editor.gd, prop_theme.json).


0. Feedback summary (verbatim intent)

  1. Rotation is very hard — replace the ring-above-bbox with the stickman editor's bottom rotation dot (small dot below the bbox bottom-center, generous hit radius, incremental-angle drag, keep Ctrl 15° snap).
  2. Vertex node circles are too large and should stay constant screen-size — diagnose what is actually read as "too large" and shrink the offending gizmos; keep vertex handles at the stickman editor's 3 px.
  3. "Create Point" along a segment does nothing — diagnose the true root cause and give immediate visible feedback; decide whether it is available anywhere on the shape.
  4. Mode separation: vertex editing vs transform editing — design an explicit interaction model (evaluate ≥2 models, recommend one), with a canvas-owned mode + public API.
  5. Kid-friendly UI — bigger text, friendlier plain-language copy, larger hit targets, higher-contrast widget styling, via a ui section in prop_theme.json (local-theme-only).

1. Diagnoses

1.1 Point 3 — "Create Point" appears to do nothing (root cause + evidence)

The wiring is correct (MENU_CREATE_POINT → _create_point_on_edge → insert_vertex), so the failure is perceptual, not mechanical. Evidence from scripts/prop_canvas.gd:

  1. Collinear insertion (primary cause). _create_point_on_edge (lines 951–975) inserts the new vertex at the exact segment midpoint:

    var midpoint: Vector2 = (pts[best_edge] + pts[(best_edge + 1) % pts.size()]) * 0.5
    insert_vertex(_selected_shape_idx, insert_idx, midpoint)
    

    On any straight edge (box, plank, triangle side, line) the midpoint lies on the same line as its two neighbours. The polygon fill and outline are therefore geometrically identical before/after the insert — nothing changes visually. (The inserted vertex is collinear and would also be stripped by sanitize_shape() on the next vertex/rotate/scale commit, since TerrainUtils.sanitize_points drops collinear vertices.)

  2. The new handle is small and low-contrast. insert_vertex flags the vertex USER_CREATED (line 298), and _draw_shape renders it as a hollow rectangle of HANDLE_HOLLOW_SIZE / _zoom * 0.5 (≈3 px half-size) in white (HANDLE_COLOR, line 578). Over a light fill or at small zoom this is nearly invisible, so even the handle is not noticed. (Note: the stickman editor color-matches these to the shape — see 1.2.)

  3. Discoverability. "Create Point" only appears when _is_point_near_outline is true (line 730), i.e. a right-click within EDGE_HIT_THRESHOLD / _zoom (12 px) of an outline edge. Right-clicking the body of a filled shape shows Copy/Paste/Delete instead — a kid right-clicks the middle of the shape and never sees the option.

Recommendation (see §2.4 / §4.3): insert at the cursor's projection onto the nearest segment (clamped), auto-select the shape, emit a vertex_created signal, and flash the new vertex with a short pulsing highlight. Auto-begin-drag is evaluated as an alternative in §5. User-approved resolution: "Create Point" stays outline-only (the current EDGE_HIT_THRESHOLD / _zoom gating is kept).

1.2 Point 2 — what is actually read as "too large" (evidence)

The vertex handles are not the problem: they already divide by _zoom (3 px constant, lines 35, 576). The two things a user actually reads as "giant vertex circles" are the other two gizmos drawn on selection (_draw_selection_gizmos, lines 582–595):

Gizmo Current draw Effective on-screen size Read as
Corner scale handles draw_circle(corner, SCALE_HANDLE_DISTANCE / _zoom, …) (line 590) 8 px radius filled circles (16 px diameter) at the four bbox corners four "giant vertex handles"
Rotate ring draw_arc(ring_center, ROTATE_RING_RADIUS / _zoom, …) (line 595) + connector line 40 px-radius ring floating above the bbox another "giant handle"
Vertex handles draw_circle(pts[j], HANDLE_RADIUS / _zoom, …) (line 576) 3 px — already correct —

Two further divergences from the stickman editor worth fixing while here:

  • Vertex handles are drawn white (HANDLE_COLOR, line 574) instead of color-matched to the shape (body_part_panel.gd lines 399–407: handle_color := color, yellow while dragging). Color-matching makes 3 px handles legible and distinct from gizmos.
  • The rotate ring's hit test (line 1097–1103) is a fuzzy maxf(12/zoom, ring*0.3) circle — generous but at an awkward location (above the shape), which is why rotation is hard (point 1).

2. Static contracts

2.1 Constants — rotation dot + scale crosses (mirror whole_stickman_preview.gd)

Replace the ring/scale-circle constants. New values are exactly the stickman editor's (whole_stickman_preview.gd lines 31–35):

# prop_canvas.gd — REMOVE:
#   const ROTATE_RING_RADIUS: float = 40.0
#   const SCALE_HANDLE_DISTANCE: float = 8.0
# (keep HANDLE_RADIUS / HANDLE_RADIUS_SELECTION / HANDLE_HOLLOW_SIZE / EDGE_HIT_THRESHOLD)

# prop_canvas.gd — ADD (mirror whole_stickman_preview exactly):
const ROTATION_CIRCLE_RADIUS: float = 5.0          # draw radius, divided by _zoom
const ROTATION_CIRCLE_HIT_RADIUS: float = 12.0     # hit radius, divided by _zoom
const ROTATION_CIRCLE_OFFSET: float = 26.0         # bbox bottom-center -> dot center (world)
const SCALE_CROSS_SIZE: float = 5.0                # cross arm length, divided by _zoom
const SCALE_CROSS_HIT_RADIUS: float = 12.0         # hit radius, divided by _zoom

Drawing (_draw_selection_gizmos):

var b: Rect2 = _shape_bbox(sd)
if b.size == Vector2.ZERO:
    return
# Bounding box (unchanged)
draw_rect(b, GIZMO_COLOR, false, 1.5 / _zoom)
# Rotation dot BELOW the bbox bottom-center (was the ring above)
var rot_center := Vector2(b.position.x + b.size.x * 0.5, b.end.y + ROTATION_CIRCLE_OFFSET)
draw_circle(rot_center, ROTATION_CIRCLE_RADIUS / _zoom, GIZMO_COLOR)
# Corner scale CROSSES (was filled circles)
var cross_half := SCALE_CROSS_SIZE / _zoom
for corner: Vector2 in [b.position, Vector2(b.end.x, b.position.y), b.end, Vector2(b.position.x, b.end.y)]:
    draw_line(corner + Vector2(-cross_half, 0), corner + Vector2(cross_half, 0), GIZMO_COLOR, 1.5 / _zoom)
    draw_line(corner + Vector2(0, -cross_half), corner + Vector2(0, cross_half), GIZMO_COLOR, 1.5 / _zoom)

Rotation dot hit-test (replaces _hit_test_rotate_ring):

func _hit_test_rotation_dot(sd: Dictionary, world_pos: Vector2) -> bool:
    var b: Rect2 = _shape_bbox(sd)
    if b.size == Vector2.ZERO:
        return false
    var rot_center := Vector2(b.position.x + b.size.x * 0.5, b.end.y + ROTATION_CIRCLE_OFFSET)
    return world_pos.distance_to(rot_center) <= ROTATION_CIRCLE_HIT_RADIUS / _zoom

Scale cross hit-test (replaces _hit_test_scale_corner threshold):

var threshold: float = SCALE_CROSS_HIT_RADIUS / _zoom   # was SCALE_HANDLE_DISTANCE / _zoom

2.2 Rotation drag — incremental angle accumulation (point 1, hard-to-rotate fix)

The current drag is absolute-from-start (_rotate_start_angle set at press, delta = current_angle − start each frame, lines 765–770, 841–843) — it jumps when the cursor crosses ±180°. Adopt the stickman editor's incremental accumulation (whole_stickman_preview.gd lines 767–794):

# State (replace _rotate_start_angle):
var _rotate_total_delta: float = 0.0
var _rotate_prev_angle: float = 0.0

# On press (_handle_left_press, rotation branch):
_drag_mode = DragMode.ROTATE
_rotate_center = _shape_bbox(sd).get_center()
_rotate_total_delta = 0.0
_rotate_prev_angle = rad_to_deg(_rotate_center.angle_to_point(world_pos))
_gizmo_base_points = PackedVector2Array(sd.get("points", PackedVector2Array()))

# On motion (_handle_drag_motion, DragMode.ROTATE):
var current_angle := rad_to_deg(_rotate_center.angle_to_point(world_pos))
var frame_delta := current_angle - _rotate_prev_angle
if frame_delta > 180.0:
    frame_delta -= 360.0
elif frame_delta < -180.0:
    frame_delta += 360.0
_rotate_total_delta += frame_delta
_rotate_prev_angle = current_angle
var delta := deg_to_rad(_rotate_total_delta)
if ctrl_pressed:
    delta = snappedf(delta, deg_to_rad(ROTATION_SNAP_DEG))
# apply delta to _gizmo_base_points about _rotate_center (unchanged math)

2.3 Mode separation — enum, state, signals, public API (point 4)

Recommended model: (a) two explicit modes with a kid-friendly segmented toggle. Rationale and rejected alternatives in §2.3.1 and §5.

# prop_canvas.gd
enum EditMode { TRANSFORM, POINTS }        # kid-facing labels: "Move & Resize" / "Edit Points"

signal mode_changed(mode: int)             # emitted only when the mode actually changes

var _edit_mode: EditMode = EditMode.TRANSFORM    # default on load

func set_edit_mode(mode: int) -> void:
    var m: int = TRANSFORM if mode != POINTS else POINTS   # clamp anything unknown to TRANSFORM
    if m == _edit_mode:
        return
    _edit_mode = m
    _drag_mode = DragMode.NONE              # cancel any in-progress drag
    _dragging_vertex = -1
    queue_redraw()
    mode_changed.emit(_edit_mode)

func get_edit_mode() -> int:
    return _edit_mode

Mode ↔ existing public API mapping (the mode gates input, not the math APIs — tests and programmatic callers keep working in either mode):

API Mode behaviour
insert_vertex / remove_vertex mode-agnostic (programmatic); only the context menu hides "Create/Remove Point" outside POINTS mode
translate_selected / rotate_selected / scale_selected mode-agnostic (programmatic; tests)
sanitize_shape, set_shape_data, clear, copy_shape, paste_shape, mirror_x/y, bring_forward/send_back mode-agnostic
select / deselect / get_selected_index mode-agnostic; do not change the mode
set_shape_data / clear do not reset the mode (tool preference persists, like snap) — see §5

Drawing rules per mode (_draw_shape + _draw_selection_gizmos):

Element TRANSFORM POINTS
Bounding box (draw_rect) ✔ — (replaced by a light selection outline)
Rotation dot ✔ —
Scale crosses ✔ —
Vertex handles (color-matched circles / hollow rects) — ✔
Selection outline polyline (white, 2.5 / _zoom) ✔ (light) ✔

Hit-test priority per mode (_handle_left_press):

TRANSFORM (default):
  1. rotation dot          -> DragMode.ROTATE
  2. scale cross corner    -> DragMode.SCALE
  3. shape body (front→back, fill/outline) -> DragMode.TRANSLATE + select
  4. empty                 -> deselect

POINTS:
  1. vertex (selected shape first, then others front→back) -> DragMode.VERTEX
  2. shape body            -> select only (NO translate)    # see §5 open choice
  3. empty                 -> deselect

Right-click context menu per mode (_handle_right_click):

POINTS:
  on a vertex      -> "Remove Point" (only item)
  on a shape       -> "Create Point" + Copy/Paste/Delete/Mirror X/Mirror Y/Bring Forward/Send Back
  empty canvas     -> Add Box/Ball/Plank/Triangle/Line/Circle (+ Paste if clipboard)
TRANSFORM:
  on a shape       -> Copy/Paste/Delete/Mirror X/Mirror Y/Bring Forward/Send Back  (no Create/Remove Point)
  empty canvas     -> Add primitives (+ Paste if clipboard)

2.3.1 Model evaluation (point 4)

  • (a) Two explicit modes + segmented toggle — RECOMMENDED. Predictable, no keyboard, no ambiguity between "grab a vertex" and "move the whole shape". Mode state is canvas-owned with a public API (set_edit_mode/get_edit_mode + mode_changed), so the editor's toggle drives it and the headless suite can assert it. Cost: the kid must learn two modes and switch; the toggle lives next to the canvas (always visible) and the default (TRANSFORM) matches the most common first action ("move my shape").
  • (b) Automatic context switching — rejected. Clicking a vertex enters points mode, clicking the body transforms. A vertex sits on the body, so the two hit targets overlap; accidental vertex drags while trying to move a shape (and vice-versa) are exactly the conflict the user reported. Harder to assert headlessly, and the "current mode" is invisible until you miss.
  • (c) Per-selection mode + inspector "Edit Points" button — rejected as primary. Clean, but the affordance is hidden in the inspector column and changes per selection, which is less discoverable for 7–12 year-olds than an always-visible toggle. (Its "Edit Points" button is kept as an optional secondary affordance — see §5.)

2.4 Create Point — new public API + feedback (point 3)

signal vertex_created(shape_idx: int, vertex_idx: int)

# Returns the new vertex index, or -1 if no segment/point could be inserted.
func create_point_on_edge(world_pos: Vector2) -> int:
    # find nearest segment (existing _point_to_segment_distance loop, closed-aware)
    # projection := a + ab * clampf(ap.dot(ab)/ab_len_sq, 0, 1)   # cursor projected onto segment
    # insert_idx := best_edge + 1 (closed wrap: pts.size())
    insert_vertex(_selected_shape_idx, insert_idx, projection)
    vertex_created.emit(_selected_shape_idx, insert_idx)
    _flash_vertex = insert_idx
    _flash_remaining = FLASH_DURATION          # §2.4.1
    queue_redraw()
    return insert_idx

_create_point_on_edge (private) is refactored to delegate to create_point_on_edge (public), so the projection + signal + flash path is directly testable.

const FLASH_DURATION: float = 0.6             # seconds
const FLASH_RING_RADIUS: float = 12.0         # max ring radius, divided by _zoom
var _flash_vertex: int = -1
var _flash_remaining: float = 0.0

2.4.1 Flash feedback (recommended over auto-begin-drag). _process(delta) decrements _flash_remaining; _draw_shape draws a pulsing ring around the flashing vertex:

if j == _flash_vertex and _flash_remaining > 0.0:
    var t := 1.0 - _flash_remaining / FLASH_DURATION
    var ring_r := FLASH_RING_RADIUS * (0.5 + 0.5 * t) / _zoom
    draw_arc(pts[j], ring_r, 0.0, TAU, 24, Color.YELLOW, 2.0 / _zoom)

Rationale: a flash + visible (color-matched, §2.1) hollow handle is non-modal and unmistakeable; auto-begin-drag after a right-click menu choice can feel like the vertex "ran away" or the tool got stuck. (Auto-drag remains an open choice — §5.)

2.5 Kid-friendly UI — theme additions, accessors, copy (point 5)

Schema version: user-approved to stay "1.0" (the new top-level ui section is additive; no existing-key meaning changes). PropTheme.load_from is already version-agnostic (it deep-merges and never rejects on version), so an older file without ui still loads with fallbacks.

New section (committed res://prop_theme.json and PropTheme.DEFAULT_CONFIG):

"ui": {
  "fonts": {
    "body": 16,
    "header": 20,
    "menu": 16,
    "status": 14
  },
  "hit_targets": {
    "button_min_height": 36,
    "row_min_height": 36
  },
  "colors": {
    "text": "#F2F2F2",
    "text_muted": "#9A9A9A",
    "border": "#FFFFFF33",
    "accent": "#4FA3FF"
  }
}

New PropTheme accessors (clamped, fallback → DEFAULT_CONFIG):

func font_body() -> int:       return integer("ui.fonts.body", 16, 8, 72)
func font_header() -> int:     return integer("ui.fonts.header", 20, 8, 96)
func font_menu() -> int:       return integer("ui.fonts.menu", 16, 8, 72)
func font_status() -> int:     return integer("ui.fonts.status", 14, 8, 72)
func button_min_height() -> int: return integer("ui.hit_targets.button_min_height", 36, 16, 200)
func row_min_height() -> int:    return integer("ui.hit_targets.row_min_height", 36, 16, 200)
func text_color() -> Color:      return color("ui.colors.text", Color("#F2F2F2"))
func text_muted_color() -> Color: return color("ui.colors.text_muted", Color("#9A9A9A"))
func border_color() -> Color:    return color("ui.colors.border", Color(1.0, 1.0, 1.0, 0.2))
func accent_color() -> Color:    return color("ui.colors.accent", Color("#4FA3FF"))

prop_editor.gd _build_control_theme() replaces the hardcoded 14/16 font sizes and the hardcoded text/border colors with these tokens; _build_inspector() applies button_min_height to buttons and row_min_height to inspector rows (custom_minimum_size.y). Local Theme only — no global GUI mutation.

Copy changes (plain-language, no emojis):

Current Proposed
"Add" (button) "Add a Shape"
"Delete Shape" (inspector button) "Erase Shape"
"Delete" (context menu) "Erase"
"Mirror X" / "Mirror Y" (context menu) "Flip Left-Right" / "Flip Up-Down"
"Create Point" / "Remove Point" keep ("Create Point" / "Remove Point")
"Bring Forward" / "Send Back" keep ("Bring Forward" / "Send Back")
"Selected Shape" / "Add Shape" (section headers) keep, but rendered at font_header

("Erase" vs "Delete", and whether to also relabel Mirror as "Flip", are open choices — §5.)


3. Task-to-implementation mapping & acceptance criteria

# Feedback point Target Acceptance criteria
1 Hard rotation scripts/prop_canvas.gd ROTATE_RING_RADIUS removed; ROTATION_CIRCLE_RADIUS/HIT_RADIUS/OFFSET = 5.0/12.0/26.0; dot drawn below bbox bottom-center at ROTATION_CIRCLE_RADIUS / _zoom; hit radius ROTATION_CIRCLE_HIT_RADIUS / _zoom; drag uses incremental accumulation (no ±180° jump); Ctrl snaps 15°.
2 Giant circles scripts/prop_canvas.gd SCALE_HANDLE_DISTANCE removed; corner handles drawn as SCALE_CROSS_SIZE / _zoom crosses; vertex handles stay HANDLE_RADIUS / _zoom (3 px) and become color-matched (yellow while dragging), user-created = hollow rect.
3 Create Point scripts/prop_canvas.gd create_point_on_edge(world_pos) -> int inserts at the cursor projection (clamped), emits vertex_created, flashes the vertex 0.6 s; "Create Point" remains available near the outline only (EDGE_HIT_THRESHOLD / _zoom, unchanged — user decision); _create_point_on_edge delegates to it.
4 Mode separation scripts/prop_canvas.gd + scripts/prop_editor.gd enum EditMode { TRANSFORM, POINTS }; default TRANSFORM; set_edit_mode/get_edit_mode/mode_changed; drawing + hit-test + context menu obey §2.3; editor renders a segmented toggle ("Move & Resize" / "Edit Points") that drives set_edit_mode and syncs from mode_changed; shape-editor controls unaffected in both modes.
5 Kid-friendly UI scripts/prop_editor.gd + scripts/prop_theme.gd + prop_theme.json ui section + accessors per §2.5; font sizes/tokens drive _build_control_theme/_build_inspector (no hardcoded 14/16); min-height hit targets applied; copy table applied; local theme only.

Test updates — tests/test_prop_editor_canvas.gd (headless; public API under test):

  • New _test_edit_mode(): default get_edit_mode() == TRANSFORM; set_edit_mode(POINTS) → get_edit_mode() == POINTS and mode_changed fires exactly once; re-setting the same mode fires no extra mode_changed; unknown value clamps to TRANSFORM; select/deselect/ clear leave the mode unchanged.
  • New _test_gizmo_constants(): assert ROTATION_CIRCLE_RADIUS == 5.0, ROTATION_CIRCLE_HIT_RADIUS == 12.0, ROTATION_CIRCLE_OFFSET == 26.0, SCALE_CROSS_SIZE == 5.0, SCALE_CROSS_HIT_RADIUS == 12.0, HANDLE_RADIUS == 3.0; assert ROTATE_RING_RADIUS and SCALE_HANDLE_DISTANCE are absent from PROP_CANVAS.get_script().get_script_constant_map().
  • New _test_create_point_on_edge(): box at (100,100); create_point_on_edge(Vector2 (124, 76)) (off-midpoint, above the top edge) → returned index ≥ 0, inserted point == the cursor's projection onto the top edge (not the midpoint (124,76) itself and not (124,100)), vertex_flags[idx] == USER_CREATED, and vertex_created emitted with the right (shape_idx, vertex_idx).
  • Existing suites unchanged (all existing APIs remain mode-agnostic).

Manual F6 verification: drag the bottom dot in a full circle (no ±180° flip); scale crosses; create a point then drag it to kink a rectangle edge; toggle modes and confirm gizmos/handles swap and body-drag is inert in POINTS mode; confirm fonts/targets.


4. Files modified

File Changes
scripts/prop_canvas.gd Rotation dot + incremental drag; scale crosses; color-matched vertex handles; EditMode enum + set_edit_mode/get_edit_mode/mode_changed; mode-gated drawing/hit-test/context-menu; create_point_on_edge + vertex_created + flash state; "Create Point" anywhere on shape.
scripts/prop_editor.gd Segmented mode toggle + mode_changed wiring; theme-driven fonts/colors/hit-targets (replace hardcoded 14/16); copy changes.
scripts/prop_theme.gd ui section in DEFAULT_CONFIG + accessors (§2.5).
prop_theme.json version stays "1.0" (user decision); add ui section.
tests/test_prop_editor_canvas.gd Add _test_edit_mode, _test_gizmo_constants, _test_create_point_on_edge.
docs/tech_debt_and_optimizations.md Log the explicit-mode interaction decision and the "Create Point" feedback design.

5. Design choices — user-approved resolutions

  1. Mode names / labels. Approved: "Move & Resize" (TRANSFORM) and "Edit Points" (POINTS).
  2. Default mode on load. Approved: TRANSFORM ("Move & Resize").
  3. Mode persistence across Clear/Load. Approved: mode persists (tool preference, like snap).
  4. Shape-body drag in POINTS mode. Approved: select-only (no translate).
  5. Create Point feedback. Approved: insert at cursor projection + auto-select + flash ring (0.6 s).
  6. Create Point availability. Approved (deviation): keep outline-only (EDGE_HIT_THRESHOLD / _zoom gating unchanged).
  7. Theme schema version. Approved (deviation): stay "1.0" (additive ui section only).
  8. "Erase" vs "Delete", Mirror→"Flip" relabel. Approved: "Erase Shape"/"Erase", "Flip Left-Right"/"Flip Up-Down"; "Create Point"/"Remove Point"/"Bring Forward"/"Send Back" unchanged.
  9. Mode toggle placement. Approved: a segmented two-button toolbar row above the canvas (always visible), synced to mode_changed.

6. Implementation order

  1. prop_theme.gd + prop_theme.json (theme tokens + accessors) — independent of canvas work.
  2. prop_canvas.gd — constants, rotation dot + incremental drag, scale crosses, color-matched handles (points 1 & 2).
  3. prop_canvas.gd — create_point_on_edge + vertex_created + flash + "anywhere on shape" (point 3).
  4. prop_canvas.gd — EditMode + gating (point 4).
  5. prop_editor.gd — mode toggle + theme-driven fonts/targets + copy (points 4 & 5).
  6. tests/test_prop_editor_canvas.gd — three new suites.
  7. Parse check + headless run + manual F6 verification; docs/tech-debt note.