- 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.
23 KiB
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)
- 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).
- 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.
- "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.
- Mode separation: vertex editing vs transform editing — design an explicit interaction model (evaluate ≥2 models, recommend one), with a canvas-owned mode + public API.
- Kid-friendly UI — bigger text, friendlier plain-language copy, larger hit targets,
higher-contrast widget styling, via a
uisection inprop_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:
-
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, sinceTerrainUtils.sanitize_pointsdrops collinear vertices.) -
The new handle is small and low-contrast.
insert_vertexflags the vertexUSER_CREATED(line 298), and_draw_shaperenders it as a hollow rectangle ofHANDLE_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.) -
Discoverability. "Create Point" only appears when
_is_point_near_outlineis true (line 730), i.e. a right-click withinEDGE_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.gdlines 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(): defaultget_edit_mode() == TRANSFORM;set_edit_mode(POINTS)→get_edit_mode() == POINTSandmode_changedfires exactly once; re-setting the same mode fires no extramode_changed; unknown value clamps toTRANSFORM;select/deselect/clearleave the mode unchanged. - New
_test_gizmo_constants(): assertROTATION_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; assertROTATE_RING_RADIUSandSCALE_HANDLE_DISTANCEare absent fromPROP_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, andvertex_createdemitted 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
- Mode names / labels. Approved:
"Move & Resize"(TRANSFORM) and"Edit Points"(POINTS). - Default mode on load. Approved:
TRANSFORM("Move & Resize"). - Mode persistence across Clear/Load. Approved: mode persists (tool preference, like snap).
- Shape-body drag in POINTS mode. Approved: select-only (no translate).
- Create Point feedback. Approved: insert at cursor projection + auto-select + flash ring (0.6 s).
- Create Point availability. Approved (deviation): keep outline-only (
EDGE_HIT_THRESHOLD / _zoomgating unchanged). - Theme schema version. Approved (deviation): stay
"1.0"(additiveuisection only). - "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.
- Mode toggle placement. Approved: a segmented two-button toolbar row above the canvas (always visible), synced to
mode_changed.
6. Implementation order
prop_theme.gd+prop_theme.json(theme tokens + accessors) — independent of canvas work.prop_canvas.gd— constants, rotation dot + incremental drag, scale crosses, color-matched handles (points 1 & 2).prop_canvas.gd—create_point_on_edge+vertex_created+ flash + "anywhere on shape" (point 3).prop_canvas.gd—EditMode+ gating (point 4).prop_editor.gd— mode toggle + theme-driven fonts/targets + copy (points 4 & 5).tests/test_prop_editor_canvas.gd— three new suites.- Parse check + headless run + manual F6 verification; docs/tech-debt note.