-
-
Notifications
You must be signed in to change notification settings - Fork 21.9k
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
Fix gdscript analyzer error when instantiating EditorPlugins. #74025
Conversation
Editor code is not instantiable outside of the editor (https://github.com/godotengine/godot/blob/1d14c054a12dacdc193b589e4afb0ef319ee2aae/core/object/class_db.cpp#L369). This is fine for editor plugins and the like, but the GDScript analyzer balks at it, causing F5 runs to fail: godotengine#73525. Instead, we really just want to know if the type is abstract - so add a new ClassDB method to check that and nothing else.
} | ||
return false; | ||
} | ||
return (ti->creation_func == nullptr && (!ti->gdextension || ti->gdextension->create_instance == nullptr)); |
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.
Shouldn't this also test ti->disabled
like in can_instantiate()
?
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.
Probably, if a class is disabled, it does not cease to be abstract. That is, whether the class is enabled is not important for is_abstract()
, but it is important for can_instantiate()
.
I build Godot rebased on master and run the example project The parse error For my unit testing, this PR unblocks running my tests So the parse error is fixed by this PR 👍 |
@vnen @YuriSizov this issue is resolved and fixes the parsing error. |
static bool is_abstract(const StringName &p_class); | ||
static bool can_instantiate(const StringName &p_class); | ||
static bool is_virtual(const StringName &p_class); |
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.
static bool is_abstract(const StringName &p_class); | |
static bool can_instantiate(const StringName &p_class); | |
static bool is_virtual(const StringName &p_class); | |
static bool can_instantiate(const StringName &p_class); | |
static bool is_abstract(const StringName &p_class); | |
static bool is_virtual(const StringName &p_class); |
It's a nitpick, but I'd rather have is_abstract()
and is_virtual()
next to each other. This also matches the order in .cpp
.
} | ||
return false; | ||
} | ||
return (ti->creation_func == nullptr && (!ti->gdextension || ti->gdextension->create_instance == nullptr)); |
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.
Probably, if a class is disabled, it does not cease to be abstract. That is, whether the class is enabled is not important for is_abstract()
, but it is important for can_instantiate()
.
} | ||
return false; | ||
} | ||
return (ti->creation_func == nullptr && (!ti->gdextension || ti->gdextension->create_instance == nullptr)); |
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.
return (ti->creation_func == nullptr && (!ti->gdextension || ti->gdextension->create_instance == nullptr)); | |
return ti->creation_func == nullptr && (!ti->gdextension || ti->gdextension->create_instance == nullptr); |
if (!ScriptServer::is_global_class(p_class)) { | ||
ERR_FAIL_V_MSG(false, "Cannot get class '" + String(p_class) + "'."); | ||
} | ||
return false; |
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.
return false; | |
String path = ScriptServer::get_global_class_path(p_class); | |
Ref<Script> scr = ResourceLoader::load(path); | |
return scr.is_valid() && scr->is_valid() && scr->is_abstract(); |
Not sure about this, just copied from ClassDB::is_virtual()
.
@dalexeev i can't push to the original forked repo, should i create a new pr? |
@baptr are you able/interested in continuing working on this? |
Superseded by #93942. Thanks for the contribution! |
Editor code is not instantiable outside of the editor (
godot/core/object/class_db.cpp
Lines 369 to 370 in 1d14c05
Instead, we really just want to know if the type is abstract - so add a new ClassDB method to check that and nothing else.
Closes #73525