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

get_class() and is_class() not returning class_name #21789

Open
ghost opened this issue Sep 6, 2018 · 58 comments
Open

get_class() and is_class() not returning class_name #21789

ghost opened this issue Sep 6, 2018 · 58 comments

Comments

@ghost
Copy link

ghost commented Sep 6, 2018

v3.1.alpha.custom_build.5307043

The new class_name keyword doesn't affect the results of the methods for get_class() and is_class().

I noticed this earlier when encountering #21461, and it feels like maybe it might have been overlooked. Not sure if it falls under bug or a feature request, or if maybe its even intentional and required.

It would be very useful to have these methods return the custom class types in certain situations.

@vnen
Copy link
Member

vnen commented Sep 7, 2018

This is expected, those methods get the name of the native class. It never worked with inner classes, which also have names.

Also, it's not clear what would this return if the script class don't have a name.

@ghost
Copy link
Author

ghost commented Sep 8, 2018

Also, it's not clear what would this return if the script class don't have a name.

I imagined that the class_name would override the native class name if you used it. If you didn't it would work as usual. At least in my mind extends Sprite would report being a "Sprite", but if you tagged on class_name CustomSprite, it would start reporting that it was a CustomSprite.

It never worked with inner classes, which also have names.

Good point. First time looking at those in 3.1, it seems at least class_name isn't valid there, but at least not needed for is to compare with them. So then it would only exclude them from being pushed up into the node list.

Since it would be enhancement request, and this makes sense to do, maybe inner classes would override in the same way with their given names.

The usefulness for having class_name appear over top the predecessor would be for some niche situations here and there, such as match and a few other things. Nothing major, but seems like it would be expected to work this way.

match node.get_class():
	"Polygon2D": print("Poly")
	"Sprite": print("Sprite.")
	"AnimatedSprite": print("AnimSprite")
	"CustomSprite": print("My sprite.")

In the meanwhile I'm doing an override in every custom class to enable that behavior:

extends Node2D

class_name CustomClass

func get_class(): return "CustomClass"
func is_class(name): return name == "CustomClass" or .is_class(name) 


func _ready():
	print(get_class())
	print(is_class("CustomClass"))
	print(is_class("Node2D"))

@vnen
Copy link
Member

vnen commented Sep 8, 2018

BTW, I'm not really against it, but technically it breaks compatibility.

@Piet-G
Copy link
Contributor

Piet-G commented Sep 8, 2018

In my opinion it seems really odd to me that it doesn't return the custom types. It should definitely be documented though either way.

@ghost
Copy link
Author

ghost commented Sep 9, 2018

@vnen I'm glad you're weighing in on it for that reason. These are just the "would be nice" thoughts of an end-user. X)

I am curious though the nature of breakage. I'm not sure if you mean for users going from 3.0 to 3.1, or something more on the internals side of things.

@PLyczkowski
Copy link

I would maybe go with get_class() -> returns base class name, and get_class_name() -> returns class_name?
If that's too confusing get_class() could be renamed to get_base_class(), and get_class() would then return class_name.

@ghost
Copy link
Author

ghost commented Sep 18, 2018

@PLyczkowski At least one way to determine an extended class currently is to do is_class("Node2D") say on a Sprite or something higher up. The base class of most things (if not everything) I believe would just be Object.

@PLyczkowski
Copy link

@avencherus This thread is about this new functionality: https://godot.readthedocs.io/en/latest/getting_started/step_by_step/scripting_continued.html#register-scripts-as-classes

So for example this would work like this:

extends "res://effect.gd" # extends Node, class_name Effect
class_name Damage

func _ready():
    self.get_base_class() # Returns Node
    self.get_class_name() # Returns "Damage"
    var effect = load("res://effect.gd").new()
    effect.get_class_name() # Returns "Effect"

@DerRidda
Copy link

I have also just encountered it and would like if there was at least another method to get the actual class name but that really makes the API dirty. BC break for get_class() would be the cleaner solution.

@xDGameStudios
Copy link
Contributor

I think it would make more sense to make the get_class return the class_name...
or create a "Type" structure like C# that return a bunch of packed info regarding the type of a given class.

get_type() -> Type:

then you could have:

type.class -> return the class_name
type.base_class -> returns the base class name
type.path -> returns the path to the gdscript where it is defined
...

@Jeto143
Copy link

Jeto143 commented May 6, 2019

Is there any plans for this? This would help tremendously, especially in debugging scenarii. And I guess it's probably not a huge thing to implement.

@Jeto143
Copy link

Jeto143 commented May 6, 2019

Here's a clumsy but actually easy/working solution for people using tools capable of mass replacing text across a whole folder/subfolders using regexes, such as IntelliJ Idea.

Replace:

class_name (\w+)\s*\n(?!func get_class_name\(\))

with:

class_name $1
func get_class_name() -> String:
	return '$1'

Do test it on one file though in case your IDE/tool uses different regex syntax (you don't want to fail a mass replace, especially when some editors don't allow that to be undone).

You can run this easily as many times as you want and it'll just add the missing ones.

@greenfox1505
Copy link
Contributor

I have an object that, when it enters the scene, it needs to connect relevant signals. I'd like to use class names in this process. To accomplish this I've overloaded get_class(). I expect I'll need to this soon too:

func is_class(name:String)->bool:
   return .is_class(name) or (get_class() == name)

This feels extremely clumsy. This functionality could be all contained with the class_name HelloWorld syntax but instead I have to add a overload of get_class() and is_class() to every class. This is 5 lines for what really should be 1.

@vickyorlo
Copy link

A very useful use case for this would be in cross-scripting scenarios. Currently, when accessing GDScript scipts from C# code, as far as I can tell, there is no way to check the class name of a node. The workarounds mentioned here do not work, as you cannot overload C#'s IsClass/GetClass from GDScript code.
It is technically doable by using Call(), but that's gonna throw an exception if you try it on anything that doesn't implement the workaround and clashes with the var is class syntax that can be used on C# classes.

@hakro
Copy link
Contributor

hakro commented Apr 21, 2020

I was about to open a new issued but stumbled upon this one instead.
I confirm that it's a bit confusing to have the native class returned.
I had 2 classes : with class_name = "Collectible" OR "Damageable",
Both are Area2D.
I needed to get which class they are at some point, but get_class() returned Area2D for both.
Which led me to get rid of class_name all together, and add variable 'name' instead.
I think the clearer approach would be to have get_class return class_name. But if it's not, it should return the parent class (Area2D in this specific case)

@xDGameStudios
Copy link
Contributor

You can override the method inside the class:

def class_name():
    return "MyCustomName;"

you need to do this manually though! :(

@hakro
Copy link
Contributor

hakro commented Apr 21, 2020

Sure, but this is rather a workaround than a solution :)
It can be very error prone if you have a lot of classes, since you need to do this manually for each of them.
Cheers

@DerRidda
Copy link

Since Godot 4.0 is supposed to have a number of BC breaks anyway, making this behave as expected instead of the way it does now would be ideal.

@willnationsdev
Copy link
Contributor

willnationsdev commented Apr 23, 2020

Yeah, with 4.0 breaking compatibility, it's a good opportunity to make this much cleaner:

# my_node.gd
extends Node
class_name MyNode

# derived_node.gd
extends MyNode
class_name DerivedNode

# usage
my_node.get_class() # "MyNode"
my_node.get_base_class() # "Node"
my_node.get_native_class() # "Node"

derived_node.get_class() # "DerivedNode"
derived_node.get_base_class() # "MyNode"
derived_node.get_native_class() # "Node"

This provides you with full access to everything you need. Just need to have the get_base_class() method check for a script first, check if there's a base script, check if that script is a script class, i.e. a global class, and then return that name if all true, rather than just the native class. And if there is no script, then you revert back to using ClassDB.get_parent_class().

Of course, to make this work well, we also need to have script class support for all programming languages. I've got a branch with VisualScript support, but my C# one has been broken for a long time cause I haven't been able to get the ScriptClass class attribute I made actually detected by the mono runtime.

@PLyczkowski
Copy link

@willnationsdev Perfect

@mechPenSketch
Copy link
Contributor

How is is_class() actually written in code, and why does it return false even when I both override get_class() and set class_name accordingly?

@vnen
Copy link
Member

vnen commented Aug 28, 2020

@mechPenSketch internal engine methods never call script methods (except for virtual methods which are intended to be overridden). So the script get_class() won't be called even if the engine uses it internally.

@zenasican
Copy link

Recently happened upon this when wanting to create a list from a custom class type "StatusEffect" with child scripts such as "BurnStatus" and "PoisonStatus".

It now requires that I set up some sort of enum or resource_name to compare the different children or need to do a long list of If statements to manually check: If status is BurnStatus and other_status is BurnStatus.

overriding get_class() now also pushes an error, so that is not an option.

having access to the custom class_name when calling get_class() would be extremely appreciated behaviour and is also what one would expect it to do at first glance.

For the time being I add get_class_name() as a custom function down the inheritance tree returning a string that I put in manually. But definitely looking forward to 4.1.

@Saltlight-Studio
Copy link

Recently happened upon this when wanting to create a list from a custom class type "StatusEffect" with child scripts such as "BurnStatus" and "PoisonStatus".
It now requires that I set up some sort of enum or resource_name to compare the different children or need to do a long list of If statements to manually check: If status is BurnStatus and other_status is BurnStatus.
overriding get_class() now also pushes an error, so that is not an option.
having access to the custom class_name when calling get_class() would be extremely appreciated behaviour and is also what one would expect it to do at first glance.

For the time being I add get_class_name() as a custom function down the inheritance tree returning a string that I put in manually. But definitely looking forward to 4.1.

In my case I am using Resources, so then I can make use of resource_names and pushing a warning when a name is not present, but yeah 4.1 is gonna be awesome

@markusneg
Copy link
Contributor

overriding get_class() now also pushes an error, so that is not an option.

Actually, the error can be turned into a warning or even muted in the project settings.

@Snirpsi
Copy link

Snirpsi commented Jul 6, 2023

Would it be possible to create a method called get_script_class and is_script_class and leave anything else as is?

@Derpford
Copy link

Derpford commented Jul 6, 2023

Someone mentioned in #62273 that Script::get_global_name() needs to be exposed to gdscript and that it'd resolve this problem. What exactly does Script::get_global_name() do? Is it just a cleaner, standardized way to get the classname when a custom class is present?

@Smalldy
Copy link

Smalldy commented Jul 24, 2023

This is a great time to change behavior, the more versions change, the heavier the compatibility baggage will be, and I hope this issue will be resolved soon!

@dalexeev
Copy link
Member

dalexeev commented Jul 24, 2023

I don't think changing get_class() behavior is a good idea. Yes, we should unify the three-level type system we have, but in a different way. For example, with a class that represents a type, see the discussion:

I have a GDType class prototype (written in GDScript). I hope to finish it soon and post it to test the idea.

Screenshot from 4.1


Screenshot with #79366

rvbatt added a commit to rvbatt/rule-based-godot that referenced this issue Aug 11, 2023
Get resource_id() from class_name, on all RuleBasedResources

Godot doesn't have an easy way to get a custom class name, because
get_class() only gets base classes and apperently there is no better
way [1], so the solution was to get the source code for the class and
find a "class_name" definition

[1]: godotengine/godot#21789

Signed-off-by: Rodrigo Volpe Battistin <rv.battistin@usp.br>
@Anyeos
Copy link

Anyeos commented Sep 17, 2023

On my opinion, first, I agree with the suggestions here and second I want to add my experience:

I had the get_class() function shadowed, on Godot 3.x. Now, trying to go to Godot 4 I decided to de-shadow all. And for my surprise, all things broke because I depend too much on get_class() on my code just to determine what kind of Item is it. And everything returns "Node2D"!! LoL. And the player character too returns "KinematicBody2D", so my player is of class "KinematicBody2D". xD I don't know from what realm is that character but I think he is from the town of the computer science xD

Well, now I have to use something like "item is Weapon". Leaving me sometimes lead to recursive references that ends in an error on the Godot Editor. So that is not always possible.

I wanted to share my experience to make it more important to take in account. Because I really depends on knowing the class_name. And as I read above I am not the only one.
I don't know for what Godot use get_class() internally but I think it is important for the right functioning of the engine. At least can we have some other function to return a String of the class_name of our own scripts? I hope yes.

Now for my characters I decided to create a redundant function just to define the same as class_name but returning a String. And for items I need to save that to a file so I really don't know how I will do for now. Because I need to save the class_name to a file so I can load the same item class. LoL, that really broke my game!

@m21-cerutti
Copy link

m21-cerutti commented Mar 13, 2024

Would be great for tools too, it's not just the get_class function but also to have it in class_name field of the get_property_list(). Would be so much convenient to define it and make some UI by types.

@Smalldy
Copy link

Smalldy commented Mar 13, 2024

According to my understanding, the demand for this feature often stems from "type recognition." We need some built-in and convenient way to distinguish different types and then... call their functions. I suddenly realized that this is actually an "interface" issue. GDScript does not have true interfaces, and it is foreseeable that it will not introduce interfaces like C++ or C#. However, I think there is a design that can be borrowed, which is the interface of the GO language. Some people may not be familiar with GO, so I can briefly introduce it: Simply put, if you want to implement an interface, you don't need to inherit it in any way, but simply implement the methods it requires:

// An interface
type Duck interface {
  public function gagaga();
  public function fly();
}

// No need to inherit
type MagicDuck struct{

}

func (d MagicDuck) gagaga(){
	//xxxxxxxxxxxxxxx
}


func (d MagicDuck) fly(){
	//xxxxxxxxxxxxxxx
}


MagicDuck does not need to inherit any interface. It implements all the required methods of Duck, so some_duck: Duck = MagicDuck() is valid, that is the logic of go.

Perhaps GDScript can draw inspiration from this method to solve the interface problem.

@m21-cerutti
Copy link

m21-cerutti commented Mar 13, 2024

According to my understanding, the demand for this feature often stems from "type recognition." We need some built-in and convenient way to distinguish different types and then... call their functions. I suddenly realized that this is actually an "interface" issue. GDScript does not have true interfaces, and it is foreseeable that it will not introduce interfaces like C++ or C#. However, I think there is a design that can be borrowed, which is the interface of the GO language. Some people may not be familiar with GO, so I can briefly introduce it: Simply put, if you want to implement an interface, you don't need to inherit it in any way, but simply implement the methods it requires:

// An interface
type Duck interface {
  public function gagaga();
  public function fly();
}

// No need to inherit
type MagicDuck struct{

}

func (d Duck) gagaga(){
	//xxxxxxxxxxxxxxx
}


func (d Duck) fly(){
	//xxxxxxxxxxxxxxx
}

MagicDuck does not need to inherit any interface. It implements all the required methods of Duck, so some_duck: Duck = MagicDuck() is valid, that is the logic of go.

Perhaps GDScript can draw inspiration from this method to solve the interface problem.

@Smalldy You don't need to make func (d MagicDuck) fly { ... } ?

But yeah make me think there is a has_method() on the Object that allow basic checks, there is even meta_data system but I think it's kind of redundant and not straightforward when defining a class_name.
And defining them in each class of your project... It is not scaling.

@Smalldy
Copy link

Smalldy commented Mar 14, 2024

sure it will be func (d MagicDuck) Fly { ... } , I made some mistake :)

@mberdev
Copy link

mberdev commented May 25, 2024

If you're ever wondering why almost all Godot tools and plugins written in GDScript, no matter how awesome and complex, stay exactly only one level of abstraction above the Editor's native features (i.e. they remain desperately self-contained, without ever being built on top of other plugins, or without ever being designed as base under other plugins), this is it. Look no further.

By "it", I mean the constant headaches when trying to know exactly where you stand (what classes exist, what are their names, in what class am I currently, etc.). This thread is a case of "it is comically hard to know the type of a given object, because the method for it doesn't exist, while there does exist a misleading method that actually does something else".

Meanwhile, the C# code base becomes more robust every day, creating an ever-growing drift between "serious" Godot (C#) and "small teams/small projects" Godot (GDScript).

@Luke9389
Copy link

Luke9389 commented Aug 4, 2024

Can anyone explain why we can't just make a new method that returns the custom class_name of an object?

@Calinou
Copy link
Member

Calinou commented Aug 5, 2024

Can anyone explain why we can't just make a new method that returns the custom class_name of an object?

The global name is exposed in #80487 since 4.3.beta.

@JuDelCo
Copy link

JuDelCo commented Aug 5, 2024

Can anyone explain why we can't just make a new method that returns the custom class_name of an object?

The global name is exposed in #80487 since 4.3.beta.

That seems to work, but not when using inner classes 🤔

@faulknermano
Copy link

I was trying to find a way to to use is but it doesn't accept passed vars, similar to what this person was doing.

A workaround I found was to use var names and the in operator to check. So with every class_name declaration, I also declare a var with it.

# Mover.gd
extends Character
class_name Mover
var class_mover
# Character.gd
extends Node2D
class_name Character
var class_character
is_object_inherit_from_class(obj, class_str):
    var _class_name = "class_%s" % class_str.to_lower()
    if _class_name in obj:
        return true
    return false

in operator will traverse down what is being extended.

We could potentially add one parameter to is_class to extend down to derived classes as well. I find it indispensable.

@kleonc
Copy link
Member

kleonc commented Sep 1, 2024

@faulknermano See is_instance_of.

@faulknermano
Copy link

faulknermano commented Sep 3, 2024

EDIT: Scratch what I said below, I was mixing variables around. It actually works. Thank you!


@faulknermano See is_instance_of.

Hi @kleonc , thanks, I've not seen that before.

However, this doesn't seem to work in my implementation quite in the same way. To recap: my aim is to use a static function to determine if a given object is derived from a particular class. When I pass a type to this function, it doesn't seem come out the way I expect it.

I've rewritten my func to use is_instance_of and it looks like:

static func get_parent_of_class_type(object: Node, class_type):
    """ Get first parent with attached script. Will keep on iterating up the hierarchy until it one. First one that matches is returned. If not, then returns null """
    var parent = object.get_parent()
    while parent != null:
        print("object: ", str(object))
        print("class_type: ", str(class_type))
        print("is_instance_of(object, class_type): ", str(is_instance_of(object, class_type)))
        if is_instance_of(object, class_type):
            return parent
        # if parent == class_id:
        #     return parent
        
        parent = parent.get_parent()
        if parent == null:
            return    

In the output I see something like:

character: PlayerCombatCharacter:<Node#60314092950>
class_type: <GDScript#-9223372009289808666>
is_instance_of(object, class_type): false

My call is like this:

var __character
var character:
    get:
        if __character == null:
            # __character = Statics.get_parent_of_class(self, "Character")
            __character = Statics.get_parent_of_class_type(self, Character)
        return __character
    set(value): pass

I.e.

__character = Statics.get_parent_of_class_type(self, Character)

.. where PlayerCombatCharacter extends from Character.

Am I supposed to transform the class type GDScript into something that I can use to compare?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.