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.

Sign up for GitHub

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

Jump to bottom

Increase Script Class Name access. #62273

Closed
wants to merge 1 commit into from
Closed

Increase Script Class Name access. #62273

wants to merge 1 commit into from

Conversation

Diddykonga
Copy link
Contributor

@Diddykonga Diddykonga commented Jun 21, 2022 •
edited
Loading

Added StringName ScriptServer::get_global_class_name(StringName script_path)
Added StringName Object::get_script_class_name()
Added StringName Script::get_global_class_name()
Changed *::is_class() to check Script Class Names as well. (Supports Script Inheritance)

Fixes: #21789
Might Fix: godotengine/godot-proposals#4708

Test Project: ClassNameTest.zip

@Diddykonga Diddykonga requested review from a team as code owners June 21, 2022 10:56
@Calinou Calinou added this to the 4.0 milestone Jun 21, 2022
@Diddykonga
Increase Script Class Name access.
28f6bb5 
Added `ScriptServer::get_global_class_name()`
Added `Object::get_script_class_name()`
Added `Script::get_global_class_name()`
Changed `*::is_class()` to check Script Class Names as well.
@Diddykonga Diddykonga force-pushed the script_class_name_imrpovements branch from a9e0511 to 28f6bb5 Compare June 21, 2022 15:19
willnationsdev
willnationsdev suggested changes Jul 7, 2022
View reviewed changes
Copy link
Contributor

@willnationsdev willnationsdev left a comment

Choose a reason for hiding this comment

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

I don't have too much authority, but I think that, for the most part, you've got a nice set of changes here. There's just a little tweaking necessary to really make it compatible with the rest of the codebase without forcing huge portions of the engine to be refactored and retested.

core/object/script_language.cpp Show resolved Hide resolved
core/object/object.h Show resolved Hide resolved
@Malcolmnixon Malcolmnixon mentioned this pull request Nov 15, 2022
Closed
@feefladder feefladder mentioned this pull request Jan 9, 2023
Open
@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023
@YuriSizov
Copy link
Contributor

Note that we now have a method in ProjectSettings to get a list of global classes, so something about this PR should likely be changed.

@Diddykonga
Copy link
Contributor Author

After reviewing what has changed, and also the state of extensions, I believe this PR is no longer needed.

Script::get_global_name() just needs to be exposed to GDScript.

@Diddykonga Diddykonga closed this Mar 13, 2023
@YuriSizov YuriSizov removed this from the 4.1 milestone Mar 13, 2023
@Derpford Derpford mentioned this pull request Jul 6, 2023
Open
@API-Beast
Copy link

After reviewing what has changed, and also the state of extensions, I believe this PR is no longer needed.

Script::get_global_name() just needs to be exposed to GDScript.

As of January 2024, Script::get_global_name is still not exposed.

@AThousandShips
Copy link
Member

AThousandShips commented Jan 12, 2024 •
edited
Loading

Yes it is? It's been for almost a month.

@akien-mga
Copy link
Member

It was exposed by #80487, which will be in 4.3. I assume @API-Beast tested on 4.2.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willnationsdev willnationsdev willnationsdev requested changes

Assignees
No one assigned
Labels
archived enhancement topic:core topic:gdscript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ability to get class name string from Reference get_class() and is_class() not returning class_name
7 participants
@Diddykonga @YuriSizov @API-Beast @AThousandShips @akien-mga @willnationsdev @Calinou