Add local editing to the inspector details panel - #25923
jbuehler23 wants to merge 2 commits into
Conversation
f8fe006 to
ca2e64e
Compare
ca2e64e to
5d4b1fb
Compare
a8522cf to
ff64fa9
Compare
Nilirad
left a comment
There was a problem hiding this comment.
Edits that set the same value as the previous one (i.e. changes that change nothing) should not mark the component as changed in write_field_edit. That would unnecessarily pollute change detection data. For example, while dragging with the mouse pointer the event is emitted each frame the pointer moves, even for integer types whose value actually change once every 100 pixels, creating a cascade of meaningless changes.
This is caused by the writing functions for the various reflected kinds write unconditionally without checking the old value, and return a bool based on whether a write happened. I propose to instead define a FieldWrite enum with three states (written, unchanged, rejected), and instead of unconditionally set the value, wrap most changes into an assign function:
/// Describes the outcome of a field write attempt.
enum FieldWrite {
Written,
Unchanged,
Rejected
}
/// Assigns `value` to `target`, reporting the change based on previous data.
///
/// If the `target`'s old value is the same of `value`,
/// returns `FieldWrite::Unchanged` without assigning the value.
fn assign<T: PartialEq>(target: &mut T, value: T) -> FieldWrite {
if *target == value {
FieldWrite::Unchanged
} else {
*target = value;
FieldWrite::Written
}
}This allows replacing most of the write sites (assign(target, value) instead of *target = value; return true), apart from a couple of cases that require manual production of FieldWrite:
write_text_field: here theStringandCowbranches take by value, and usingassignwould allocate before comparing. Instead, we can first check for equality, and allocate only if there is a difference. Thecharbranch can useassignthough.write_variant_field: noPartialEq.FieldWritecan be done by comparing the variant string name.
ff64fa9 to
4227b53
Compare
|
@Nilirad adopted the |
Objective
Part of #23013.
Let the inspector edit the selected entity's components from the details panel, instead of only showing them.
Solution
Each field widget knows which component and field path it edits. Changing it triggers a
FieldEditevent, and an observer writes the value into the component through reflection. Going through an event means a remote source can apply the same edit later without the panel changing.Colours and switching to enum variants with fields are left for the next PR.
AI disclosure
Same approach as the earlier pieces. I planned the scope, AI did the mechanical port from Jackdaw and a first check for problems, and I reviewed it and tested it by hand. Testing by hand is where the main issue showed up. The edits worked in unit tests but not when clicking the real widgets, because the feathers number input and checkbox don't update themselves and the panel skipped writing the value back. The tests now drive the actual widgets headlessly for better testing coverage
Showcase