[Refactoring] Audit all Python property setters for missing markDirty() calls #291

New issue

Closed

opened 2026-03-08 16:36:06 +00:00 by john · 0 comments

john commented

2026-03-08 16:36:06 +00:00

Owner

Problem

Issues #288, #289, and #290 reveal a systemic pattern: the render cache dirty-flag system (#144) was added to the C++ setProperty() animation path but not consistently to the Python tp_getset property setter path.

The two code paths exist because:

setProperty(name, value) — C++ method, used by animation system. Has markDirty() everywhere.
tp_getset setters — Python property accessors (e.g., set_text, set_color_member). Some have markDirty(), some don't.

Scope of audit

Every drawable type needs its Python setters checked:

Type	`setProperty()` path	`tp_getset` path	Status
UIFrame	All call markDirty()	`set_color_member` calls markDirty()	Mostly OK
UICaption	All call markDirty()	`set_text`, `set_color_member`, `set_float_member` do NOT	Broken (#289)
UISprite	All call markDirty()	Uses `setProperty()` dispatch	Needs verification
UICircle	All call markDirty()	Uses `setProperty()` dispatch	Needs verification
UILine	All call markDirty()	Uses `setProperty()` dispatch	Needs verification
UIArc	All call markDirty()	Uses `setProperty()` dispatch	Needs verification
UIGrid	All call markDirty()	Uses `setProperty()` dispatch	Needs verification
UIDrawable (base)	`set_rotation`, `set_origin` OK	`set_float_member` (x,y) missing	Broken (#290)

Root cause

The setProperty() path was retrofitted with dirty flags in #144, but tp_getset setters were written earlier and weren't updated. Some types (Sprite, Circle, Line, Arc) use a unified setProperty() dispatch even from tp_getset, so they're likely fine. Caption has its own legacy setters that bypass setProperty() entirely.

Recommended approach

For each type, check every tp_getset setter that modifies visual state
Ensure it calls the appropriate dirty method:
- Content changes (color, text, size, texture): markDirty() / markContentDirty()
- Position changes (x, y, pos): markCompositeDirty()
Consider whether onPositionChanged() should call markCompositeDirty() by default in the base class, eliminating the need for every caller to remember

#144 — Original render cache implementation
#288 — UICollection mutations missing cache invalidation
#289 — Caption setters missing markDirty()
#290 — Base class position setters missing markCompositeDirty()

## Problem Issues #288, #289, and #290 reveal a systemic pattern: the render cache dirty-flag system (#144) was added to the C++ `setProperty()` animation path but **not consistently** to the Python `tp_getset` property setter path. The two code paths exist because: - **`setProperty(name, value)`** — C++ method, used by animation system. Has `markDirty()` everywhere. - **`tp_getset` setters** — Python property accessors (e.g., `set_text`, `set_color_member`). Some have `markDirty()`, some don't. ### Scope of audit Every drawable type needs its Python setters checked: | Type | `setProperty()` path | `tp_getset` path | Status | |------|---------------------|-------------------|--------| | **UIFrame** | All call markDirty() | `set_color_member` calls markDirty() | Mostly OK | | **UICaption** | All call markDirty() | `set_text`, `set_color_member`, `set_float_member` do NOT | **Broken** (#289) | | **UISprite** | All call markDirty() | Uses `setProperty()` dispatch | Needs verification | | **UICircle** | All call markDirty() | Uses `setProperty()` dispatch | Needs verification | | **UILine** | All call markDirty() | Uses `setProperty()` dispatch | Needs verification | | **UIArc** | All call markDirty() | Uses `setProperty()` dispatch | Needs verification | | **UIGrid** | All call markDirty() | Uses `setProperty()` dispatch | Needs verification | | **UIDrawable (base)** | `set_rotation`, `set_origin` OK | `set_float_member` (x,y) missing | **Broken** (#290) | ### Root cause The `setProperty()` path was retrofitted with dirty flags in #144, but `tp_getset` setters were written earlier and weren't updated. Some types (Sprite, Circle, Line, Arc) use a unified `setProperty()` dispatch even from tp_getset, so they're likely fine. Caption has its own legacy setters that bypass `setProperty()` entirely. ### Recommended approach 1. For each type, check every `tp_getset` setter that modifies visual state 2. Ensure it calls the appropriate dirty method: - Content changes (color, text, size, texture): `markDirty()` / `markContentDirty()` - Position changes (x, y, pos): `markCompositeDirty()` 3. Consider whether `onPositionChanged()` should call `markCompositeDirty()` by default in the base class, eliminating the need for every caller to remember ### Related - #144 — Original render cache implementation - #288 — UICollection mutations missing cache invalidation - #289 — Caption setters missing markDirty() - #290 — Base class position setters missing markCompositeDirty()

john referenced this issue from a commit

2026-04-10 05:09:18 +00:00

Add missing markDirty()/markCompositeDirty() to all Python property setters

john closed this issue

2026-04-10 05:09:18 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

john/McRogueFace#291

No description provided.

Rows
Columns