Skip to content

Exchange use of ClassDB::is_parent_class() with Object::is_class() where possible#118575

Open
DeeJayLSP wants to merge 1 commit intogodotengine:masterfrom
DeeJayLSP:gdtype-name-hierarchy
Open

Exchange use of ClassDB::is_parent_class() with Object::is_class() where possible#118575
DeeJayLSP wants to merge 1 commit intogodotengine:masterfrom
DeeJayLSP:gdtype-name-hierarchy

Conversation

@DeeJayLSP
Copy link
Copy Markdown
Contributor

@DeeJayLSP DeeJayLSP commented Apr 14, 2026

Replaces instances of ClassDB::is_parent_class() where you have access to the object itself, like:

ClassDB::is_parent_class(object->get_class(), inherits);
ClassDB::is_parent_class(object->get_class_name(), inherits);
String class_name = object->get_class_name();
ClassDB::is_parent_class(class_name, inherits);

With:

object->is_class(inherits);

Functionally identical, except we skip ClassDB's lookup by name and directly check using the object's own GDType.

Hierarchy is constructed alongside the GDType itself (not during initialization) and tests pass, so I assume this is a safe change.

@DeeJayLSP DeeJayLSP requested review from a team as code owners April 14, 2026 17:49
@DeeJayLSP DeeJayLSP force-pushed the gdtype-name-hierarchy branch from 1cd656f to aa86599 Compare April 14, 2026 22:20
@DeeJayLSP DeeJayLSP changed the title Ask GDType for hierarchy instead of ClassDB where possible Ask Object for hierarchy instead of ClassDB where possible Apr 14, 2026
@DeeJayLSP DeeJayLSP force-pushed the gdtype-name-hierarchy branch from aa86599 to fc30511 Compare April 14, 2026 22:50
@DeeJayLSP DeeJayLSP changed the title Ask Object for hierarchy instead of ClassDB where possible Ask GDType for hierarchy instead of ClassDB where possible Apr 14, 2026
@AThousandShips AThousandShips added this to the 4.x milestone Apr 15, 2026
@DeeJayLSP DeeJayLSP force-pushed the gdtype-name-hierarchy branch from fc30511 to 4769777 Compare April 15, 2026 22:22
@DeeJayLSP DeeJayLSP changed the title Ask GDType for hierarchy instead of ClassDB where possible Exchange use of ClassDB::is_parent_class() with Object::is_type() where possible Apr 15, 2026
@DeeJayLSP
Copy link
Copy Markdown
Contributor Author

@Ivorforce this may be a reasonable follow-up of #118582

@Ivorforce Ivorforce changed the title Exchange use of ClassDB::is_parent_class() with Object::is_type() where possible Exchange use of ClassDB::is_parent_class() with Object::is_class() where possible Apr 16, 2026
Comment thread editor/script/script_text_editor.cpp Outdated
}
}
const bool is_script = ClassDB::is_parent_class(p_resource->get_class(), "Script");
const bool is_script = p_resource->is_class("Script");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get an existing StringName rather than parsing a char *? For example:

Suggested change
const bool is_script = p_resource->is_class("Script");
const bool is_script = p_resource->is_class(Script::get_class_static());

Copy link
Copy Markdown
Member

@KoBeWi KoBeWi Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of is_class() is to avoid including the class.

And if the class is already included, it's better to use Object::cast_to<>() or derives_from(). Using is_class() with get_class_static() does not make much sense.

Though for static strings, you can use SNAME at least.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringName lookup by key is about 3x faster than String allocation for the same key (i tested this once). ClassDB's lock + has lookup makes it even worse. So while using SNAME would be even faster, most (90+%?) of the performance gap is already addressed in this PR, even without SNAME use.

@DeeJayLSP DeeJayLSP force-pushed the gdtype-name-hierarchy branch from 4769777 to d376b20 Compare April 16, 2026 15:38
@DeeJayLSP
Copy link
Copy Markdown
Contributor Author

Applied the SNAME suggestion to script_text_editor.cpp as I found it reasonable + fixed the commit title.

@DeeJayLSP DeeJayLSP requested a review from Ivorforce April 16, 2026 16:40
Copy link
Copy Markdown
Member

@Ivorforce Ivorforce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

@Repiteo Repiteo modified the milestones: 4.x, 4.8 Apr 17, 2026
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.

6 participants