Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update checked based on the variant type of the value #60786

Merged
merged 1 commit into from
Sep 16, 2022

Conversation

Rindbee
Copy link
Contributor

@Rindbee Rindbee commented May 5, 2022

In EditorProperty::update_revert_and_pin_status, if checkable is true, update checked based on the variant type of the value, and rename the function as EditorProperty::update_editor_property_status.

This patch can fix #55455.

Known problems:

  1. Unable to check Checkbox for properties of type Object when it is unchecked. This is probably because during value store/fetch, the nullptr (variant type is Variant::OBJECT) eventually becomes Variant() (variant type is Variant::NIL).
  2. For theme overrides, click the revert icon will uncheck the Checkbox. This is probably because EditorPropertyRevert::get_property_revert_value returns Variant().

@akien-mga akien-mga added this to the 4.0 milestone May 5, 2022
@akien-mga akien-mga requested review from RandomShaper and a team May 5, 2022 12:02
@RandomShaper
Copy link
Member

Looks good overall, but I'm not familiar enough with the exact mechanics of theme overriding to be confident to approve.

@Rindbee
Copy link
Contributor Author

Rindbee commented May 6, 2022

For using Object::get to get theme overrides, the codes involved are as follows:

// Something inside the object... :|
bool success = _getv(p_name, ret);
if (success) {
if (r_valid) {
*r_valid = true;
}
return ret;
}

godot/core/object/object.h

Lines 389 to 396 in 066692b

virtual bool _getv(const StringName &p_name, Variant &r_ret) const override { \
if (m_class::_get_get() != m_inherits::_get_get()) { \
if (_get(p_name, r_ret)) { \
return true; \
} \
} \
return m_inherits::_getv(p_name, r_ret); \
} \

godot/scene/gui/control.cpp

Lines 336 to 365 in 066692b

bool Control::_get(const StringName &p_name, Variant &r_ret) const {
String sname = p_name;
if (!sname.begins_with("theme_override")) {
return false;
}
if (sname.begins_with("theme_override_icons/")) {
String name = sname.get_slicec('/', 1);
r_ret = data.icon_override.has(name) ? Variant(data.icon_override[name]) : Variant();
} else if (sname.begins_with("theme_override_styles/")) {
String name = sname.get_slicec('/', 1);
r_ret = data.style_override.has(name) ? Variant(data.style_override[name]) : Variant();
} else if (sname.begins_with("theme_override_fonts/")) {
String name = sname.get_slicec('/', 1);
r_ret = data.font_override.has(name) ? Variant(data.font_override[name]) : Variant();
} else if (sname.begins_with("theme_override_font_sizes/")) {
String name = sname.get_slicec('/', 1);
r_ret = data.font_size_override.has(name) ? Variant(data.font_size_override[name]) : Variant();
} else if (sname.begins_with("theme_override_colors/")) {
String name = sname.get_slicec('/', 1);
r_ret = data.color_override.has(name) ? Variant(data.color_override[name]) : Variant();
} else if (sname.begins_with("theme_override_constants/")) {
String name = sname.get_slicec('/', 1);
r_ret = data.constant_override.has(name) ? Variant(data.constant_override[name]) : Variant();
} else {
return false;
}
return true;
}

For Control::_get, if theme override exists, fetch the overridden value, otherwise fetch Variant(). Returns true if the prefix matches.

In `EditorProperty::update_revert_and_pin_status`, if `checkable` is `true`, update `checked` based on the variant type of the value, and rename the function as `EditorProperty::update_editor_property_status`.

**Known problems:**
1. Unable to check Checkbox for properties of type `Object` when it is unchecked. This is probably because during value store/fetch, the `nullptr` (variant type is `Variant::OBJECT`) eventually becomes `Variant()` (variant type is `Variant::NIL`).
2. For theme overrides, click the revert icon will uncheck the Checkbox. This is probably because `EditorPropertyRevert::get_property_revert_value` returns `Variant()`.
@Rindbee Rindbee force-pushed the update-editor-property-status branch from 0f6c074 to 9066d55 Compare September 16, 2022 00:21
@akien-mga akien-mga merged commit 3546add into godotengine:master Sep 16, 2022
@akien-mga
Copy link
Member

Thanks!

@Rindbee Rindbee deleted the update-editor-property-status branch September 16, 2022 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theme override checkbox is oblivious to undo
3 participants