john/McRogueFace

Fork 0

[1.0 Decision] Color/Vector copy semantics: value types vs write-through proxies #326

New issue

Closed

opened 2026-07-02 12:24:32 +00:00 by john · 1 comment

john commented

2026-07-02 12:24:32 +00:00

Owner

From the 2026-07-02 pre-1.0 memory-model review. This is a semantic decision that freezes at 1.0 — it cannot be changed compatibly afterward.

Current behavior

PyColorObject and PyVectorObject store their data by value (sf::Color / sf::Vector2f inline, no shared_ptr, no weakreflist — src/PyColor.h:8-11, src/PyVector.h:6-9). Every property getter returns a fresh copy:

frame.fill_color → PyColor(color).pyObject() allocates a new object per read (src/UIFrame.cpp:346-371)
Consequence: frame.fill_color.r = 255 is a silent no-op — you mutate a copy. The PyColor docstring already warns about this (src/PyColor.h:63-77).

Unlike Frame/Caption/Sprite/Grid/Entity (which get identity preservation via PythonObjectCache), Color/Vector have no serial number and are never cached.

The decision

Commit to value semantics forever. Cheap, simple, matches how sf::Color works in C++. The .r = 255 no-op footgun stays; mitigate with documentation and possibly a runtime warning on mutating an orphaned color.
Switch to write-through proxy views (color/vector objects returned from properties hold a back-reference and write through on mutation). Fixes the footgun, but adds complexity (dangling-parent lifetime, identity questions like frame.fill_color is frame.fill_color) and must land before 1.0.

Either answer is fine; not deciding is the only wrong option.

PythonObjectCache identity mechanism: src/PythonObjectCache.h
Perf note: Vector construction via .pos getter is also pathologically slow (#331).

From the 2026-07-02 pre-1.0 memory-model review. This is a **semantic decision that freezes at 1.0** — it cannot be changed compatibly afterward. ## Current behavior `PyColorObject` and `PyVectorObject` store their data **by value** (`sf::Color` / `sf::Vector2f` inline, no `shared_ptr`, no weakreflist — `src/PyColor.h:8-11`, `src/PyVector.h:6-9`). Every property getter returns a **fresh copy**: - `frame.fill_color` → `PyColor(color).pyObject()` allocates a new object per read (`src/UIFrame.cpp:346-371`) - Consequence: `frame.fill_color.r = 255` is a **silent no-op** — you mutate a copy. The PyColor docstring already warns about this (`src/PyColor.h:63-77`). Unlike Frame/Caption/Sprite/Grid/Entity (which get identity preservation via `PythonObjectCache`), Color/Vector have no serial number and are never cached. ## The decision 1. **Commit to value semantics forever.** Cheap, simple, matches how `sf::Color` works in C++. The `.r = 255` no-op footgun stays; mitigate with documentation and possibly a runtime warning on mutating an orphaned color. 2. **Switch to write-through proxy views** (color/vector objects returned from properties hold a back-reference and write through on mutation). Fixes the footgun, but adds complexity (dangling-parent lifetime, identity questions like `frame.fill_color is frame.fill_color`) and must land **before** 1.0. Either answer is fine; not deciding is the only wrong option. ## Related - `PythonObjectCache` identity mechanism: `src/PythonObjectCache.h` - Perf note: Vector construction via `.pos` getter is also pathologically slow (#331).

john added the

Minor Feature

priority:tier1-active

system:python-binding

labels

2026-07-02 12:26:48 +00:00

john referenced this issue

2026-07-02 12:30:41 +00:00

Fast path for .pos and similar getters: stop importing mcrfpy per property read #331

john commented

2026-07-03 00:04:03 +00:00

Author

Owner

DECIDED 2026-07-02: Option 1 — value semantics, forever.

Color/Vector remain plain value copies at every property boundary. frame.fill_color.r = 255 mutating a throwaway copy is the documented, frozen behavior; the supported idioms are read-modify-writeback (c = frame.fill_color; c.r = 255; frame.fill_color = c) or whole-value assignment (frame.fill_color = mcrfpy.Color(...)).

Rationale: zero pre-1.0 implementation risk, matches sf::Color and the existing PyColor docstring warning, and avoids the dangling-parent lifetime / identity questions write-through proxies would freeze into the contract.

Follow-through (docs pass, will close this issue): make the copy-semantics statement explicit and normative in the PyColor/PyVector class docstrings (MCRF macros), the API reference intro, and the 1.0 compatibility policy alongside #327/#330.

**DECIDED 2026-07-02: Option 1 — value semantics, forever.** Color/Vector remain plain value copies at every property boundary. `frame.fill_color.r = 255` mutating a throwaway copy is the documented, frozen behavior; the supported idioms are read-modify-writeback (`c = frame.fill_color; c.r = 255; frame.fill_color = c`) or whole-value assignment (`frame.fill_color = mcrfpy.Color(...)`). Rationale: zero pre-1.0 implementation risk, matches sf::Color and the existing PyColor docstring warning, and avoids the dangling-parent lifetime / identity questions write-through proxies would freeze into the contract. Follow-through (docs pass, will close this issue): make the copy-semantics statement explicit and normative in the PyColor/PyVector class docstrings (MCRF macros), the API reference intro, and the 1.0 compatibility policy alongside #327/#330.

john referenced this issue from a commit

2026-07-03 03:45:58 +00:00

Freeze value semantics + record 1.0 compatibility policy

john closed this issue

2026-07-03 03:45:58 +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#326

No description provided.

Rows
Columns