-
Notifications
You must be signed in to change notification settings - Fork 13
Fixed PropertyInfo Default Usage from Dictionary
#34
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
Conversation
📝 WalkthroughWalkthroughModified PropertyInfo's Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@include/godot_cpp/core/property_info.hpp`:
- Around line 83-85: The serialization omits the "usage" key when
PropertyInfo::usage == PROPERTY_USAGE_NONE, but PropertyInfo::from_dict()
currently falls back to PROPERTY_USAGE_DEFAULT when the key is absent, breaking
roundtrip fidelity; update PropertyInfo::from_dict() to set usage to
PROPERTY_USAGE_NONE when the "usage" key is missing (e.g., check
dict.has("usage") or equivalent) so that missing key restores
PROPERTY_USAGE_NONE instead of PROPERTY_USAGE_DEFAULT, referencing the
PropertyInfo struct and its usage field and the constants PROPERTY_USAGE_NONE /
PROPERTY_USAGE_DEFAULT to locate the change.
| if (usage != PROPERTY_USAGE_NONE) { | ||
| dict["usage"] = usage; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential roundtrip inconsistency with from_dict().
When usage == PROPERTY_USAGE_NONE, the "usage" key is now omitted from the dictionary. However, from_dict() (lines 106-108) leaves usage at the struct default PROPERTY_USAGE_DEFAULT when the key is missing. This means:
PropertyInfowithusage = PROPERTY_USAGE_NONE→Dictionary→from_dict()→usage = PROPERTY_USAGE_DEFAULT
If roundtrip fidelity is needed, consider updating from_dict() to default to PROPERTY_USAGE_NONE when the "usage" key is absent:
Proposed fix to align from_dict() behavior
static PropertyInfo from_dict(const Dictionary &p_dict) {
PropertyInfo pi;
+ pi.usage = PROPERTY_USAGE_NONE; // Match to_dict() behavior when key is missing
if (p_dict.has("type")) {
pi.type = Variant::Type(int(p_dict["type"]));
}🤖 Prompt for AI Agents
In `@include/godot_cpp/core/property_info.hpp` around lines 83 - 85, The
serialization omits the "usage" key when PropertyInfo::usage ==
PROPERTY_USAGE_NONE, but PropertyInfo::from_dict() currently falls back to
PROPERTY_USAGE_DEFAULT when the key is absent, breaking roundtrip fidelity;
update PropertyInfo::from_dict() to set usage to PROPERTY_USAGE_NONE when the
"usage" key is missing (e.g., check dict.has("usage") or equivalent) so that
missing key restores PROPERTY_USAGE_NONE instead of PROPERTY_USAGE_DEFAULT,
referencing the PropertyInfo struct and its usage field and the constants
PROPERTY_USAGE_NONE / PROPERTY_USAGE_DEFAULT to locate the change.
The editor currently doesn’t respect
PropertyInfousage flags when adding new Editor Settings through GDExtension. It always assigns a default usage value, which triggers warnings on startup.This PR adds a check for
PROPERTY_USAGE_NONEand skips setting the usage field entirely in that case, preventing the warning from being generated at startup.Summary by CodeRabbit