add_custom_type() does not register the added type as a class

[aabmets]

Documentation Link: Custom Node - Making Plugins

Problem:
add_custom_type("NodeName", "ParentNodeType", preload("script.gd"), preload("icon.png"))
does not register the new NodeName node in the editor as a valid reference-able class name, when implemented as a new node type through the plugin system.

Steps to reproduce:
Create a plugin as described in the doc and then try to create in code a new NodeName node in any other script file. The editor throws: "Identifier 'NodeName' is not defined in the current scope."

Expected Behavior:
Work the same way as HTTPRequests node does for example. One can add it manually in the scene tree editor or in code using var x = HTTPRequest.new()

[bojidar-bg]

Tagging as discussion, since I feel it might be by-design. If you want class_name-like functionality, use class_name-s.

[aabmets]

If you pick randomly any pre-existing node and write into script
var x = RandomNode.new() it will work in 100% of cases.
Then how come is it reasonable that a custom node implemented by an add_custom_type func in a plugin script has no such capability, which is directly counter-intuitive to any semblance of consistency?

Second of all, class_name produces an entry in the New Node Wizard
in the form of "NodeName (script.gd)".
How can it be considered to make any sense that a custom node, which is introduced by a plugin,
has its source script specifically mentioned in the New Node Wizard?
A user dealing with a tailored plugin designed for a specific functionality only cares about the endpoints
which the plugin exposes, not where they originate from.
By that idea, it is completely useless and even incorrect to expect plugin devs to use the class_name keyword to implement custom nodes from within a plugin.
If the source script reference is absolutely required in the New Node Wizard, in the case of a node implemented by a plugin, it would make the most sense if the (script.gd) reference is
replaced by the name of the plugin.

Pinging @reduz & @RandomShaper for sage advice.

[aabmets]

Also, when you add a class_name defined node from the New Node Wizard, it will automatically attach the relevant script to the node, which it does the same for a node defined in a plugin with add_custom_type.
How come it makes sense that if both of them behave the same way in the scene tree editor, it should not be possible to instance both of them the same way in code?

[toiletsnakes]

There actually are some old threads about this:
#6067
#17093
related #17368
#10799

I really like this solution suggested: #6067 (comment)

[bojidar-bg]

I really like this solution suggested: …

This was already implemented by #30697

[aaronfranke]

See also: #26875

[jonri]

I've just run into the same situation - I'm developing a plugin that I'd like to be able to reference by name both from the Create New Node dialog and from GDScript. Here's what I've tried:

• Using add_custom_type() to register it. As described in this task, you can add it by name to the scene tree, and you get all the nice benefits already linked here. But the one thing you don't get is the ability to reference it by name within GDScript.
• Using class_name instead. This silently fails, because it is forbidden within an addon by design: [Using class_name in a newly created script does not register it in the node creation dialog, if under addons/ (unless part of an enabled editor plugin) #30048]
• Using both add_custom_type() and class_name. This causes duplicate names to appear [Duplicate Custom Node entry when using a plugin with class_name and registering the node #27132] and the class_name takes precedence, erasing the other benefits from using add_custom_type() [class_name overrides AutoLoad global name if using the same name #28187]

Here are the three options I see to make this possible:

• Make add_custom_type() register the type as a class, like class_name does. I can't see any downsides to this.
• Make class_name work within addons. I don't favor this approach as it seems clear that class_name was not intended for this case.
• Support using add_custom_type() and class_name simultaneously. In this case, the fix would be to let the add_custom_type() UI features take precedence and remove the duplicate entry. Things could get confusing with the ability to enable/disable a plugin. But this could be an option if there was a good reason for add_custom_type() not to register the type as a class by default.

I would lean towards the first solution, is there any reason it wouldn't be a good idea?

[toiletsnakes]

I like it! I just use class_name and avoid using register, it’s been working pretty nicely for me. But it’d be great to have your suggested fix

…

Sent from my iPhone

On Aug 5, 2019, at 10:49 AM, jonri ***@***.***> wrote: I've just run into the same situation - I'm developing a plugin that I'd like to be able to reference by name both from the Create New Node dialog and from GDScript. Here's what I've tried: Using add_custom_type() to register it. As described in this task, you can add it by name to the scene tree, and you get all the nice benefits already linked here. But the one thing you don't get is the ability to reference it by name within GDScript. Using class_name instead. This silently fails, because it is forbidden within an addon by design: [#30048] Using both add_custom_type() and class_name. This causes duplicate names to appear [#27132] and the class_name takes precedence, erasing the other benefits from using add_custom_type() [#28187] Here are the three options I see to make this possible: Make add_custom_type() register the type as a class, like class_name does. I can't see any downsides to this. Make class_name work within addons. I don't favor this approach as it seems clear that class_name was not intended for this case. Support using add_custom_type() and class_name simultaneously. In this case, the fix would be to let the add_custom_type() UI features take precedence and remove the duplicate entry. Things could get confusing with the ability to enable/disable a plugin. But this could be an option if there was a good reason for add_custom_type() not to register the type as a class by default. I would lean towards the first solution, is there any reason it wouldn't be a good idea? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[jonri]

Indeed, I developed the whole thing using class_name and didn't have any trouble until I decided to turn it into a plugin to use with multiple projects and eventually upload to the AssetLib. I suppose I could still do that without using the addons folder, but in addition to going against conventions I'd also lose the grayed-out script and proper tooltips that got added for plugins (and explicitly not for class_name). After reading through all the related issues, it really seems like class_name is only for "your own" code and not for "external" code like a plugin.

[aabmets]

There should be two ways of adding classes: one for local in-project classes and the other for plugin classes, because the needs are different and it makes no sense to reference the class script file name in the node selection dialogue for plugin classes. I'm just glad that I'm not the only one who sees it this way.

…

On Mon, 5 Aug 2019 at 21:22, jonri ***@***.***> wrote: Indeed, I developed the whole thing using class_name and didn't have any trouble until I decided to turn it into a plugin to use with multiple projects and eventually upload to the AssetLib. I suppose I could still do that without using the addons folder, but in addition to going against conventions I'd also lose the grayed-out script and proper tooltips that got added for plugins (and explicitly not for class_name). After reading through all the related issues, it really seems like class_name is only for "your own" code and not for "external" code like a plugin. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#29548?email_source=notifications&email_token=ABVAJRB3Z7N4ASIQJ6TX6KTQDBVUZA5CNFSM4HVDRYLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3SUZKY#issuecomment-518343851>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABVAJRCTH6AZDWT4ULQ6Y5LQDBVUZANCNFSM4HVDRYLA> .

[toiletsnakes]

I’m not sure I understand. On 3.1.1 I’m using class_name for my Plugins and it’s working great. Even when I export the plugin and use it in other projects. Genuinely curious, in your experience how are the needs different? And would there be any pros / cons if they accomplished the same thing? Like if register_type and class_name accomplished the same thing, would that be bad?

…

Sent from my iPhone

On Aug 5, 2019, at 11:39 AM, Mattias Aabmets ***@***.***> wrote: There should be two ways of adding classes: one for local in-project classes and the other for plugin classes, because the needs are different and it makes no sense to reference the class script file name in the node selection dialogue for plugin classes. I'm just glad that I'm not the only one who sees it this way. On Mon, 5 Aug 2019 at 21:22, jonri ***@***.***> wrote: > Indeed, I developed the whole thing using class_name and didn't have any > trouble until I decided to turn it into a plugin to use with multiple > projects and eventually upload to the AssetLib. I suppose I could still do > that without using the addons folder, but in addition to going against > conventions I'd also lose the grayed-out script and proper tooltips that > got added for plugins (and explicitly not for class_name). After reading > through all the related issues, it really seems like class_name is only for > "your own" code and not for "external" code like a plugin. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#29548?email_source=notifications&email_token=ABVAJRB3Z7N4ASIQJ6TX6KTQDBVUZA5CNFSM4HVDRYLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3SUZKY#issuecomment-518343851>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABVAJRCTH6AZDWT4ULQ6Y5LQDBVUZANCNFSM4HVDRYLA> > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[jonri]

@toiletsnakes are you putting your plugins in the addons folder? See #30048, this is the same thing that is happening to me. Additionally, I'm guessing that when you add your plugin to the scene tree, its script icon is not dimmed and the tooltip when you mouse over your item does not show your custom name. Both of those things are there when you use add_custom_type().

If it were up to me, we would have a way for plugins/addons to register themselves in the ClassDB and be on equal footing with the builtin objects provided by the engine. The decision was made not to do that which I respect, and the niceties added to add_custom_type() are a good compromise between having your plugin fully encapsulated and having your plugin's script treated as just another script.

Object created using class_name don't have those niceties, and from what I can see the rationale is that this separates it from the pseudo-encapsulation of a plugin. The other difference using class_name is that there is no way to enable or disable it, like you can with a plugin now. These limitations are all perfectly fine when you are writing your own classes within your project. In my personal opinion, though, it would also be acceptable to me if class_name was made into a full replacement for add_custom_type, but others will disagree.

I haven't even touched on the disparities between GDScript, NativeScript, C#, etc. I think everyone realizes that the whole situation needs to be improved and I'm hoping that a bit of a redesign and cleanup can happen as part of 4.0. In the meantime, though, it seems to me like fixing up add_custom_type() would rock the fewest boats.

[aaronfranke]

@toiletsnakes With addons, you can enable and disable them at will, and you write some GDScript functions to enable and disable them. With add_custom_type() and remove_custom_type() you have the ability to disable the plugin, while class_name makes it always enabled if the file exists. EDIT: Now class_name does not register a global class if the plugin is disabled.

Also, thanks to #30697, add_custom_type() means that the script icons are faded.

[jonri]

@toiletsnakes I tried using class_name with my plugin again in a fresh project and I actually got it to show up! Furthermore, using enable/disable plugin does show/hide my class_name plugin in the node list, which is really unexpected.

Now I'm not sure which version is actually working correctly, or what should be expected...

[jonri]

I'm chalking my previous problem using class_name up to a fluke. It looks like class_name is supported in plugins, and there is actually a special case in the engine code to automatically show/hide the registered classes when you enable/disable the plugin they reside within. So, it seems to me that class_name is in fact a viable replacement for add_custom_type, unless you have a special case that needs more control over when a class gets registered.

In the latest master (but not in 3.1.1) the tooltips in the scene tree show your class_name as well, so that is the same between both methods.

The only other difference I see right now is that the script icons aren't faded when you use class_name in a plugin. It would be nice to support this for class_name, but only for objects that are defined within an addon (like how there is a also special case for enabling/disabling them).

This should also really be documented on the "Making plugins" page with an alternative example using class_name instead of add_custom_type.

I'll be switching back to class_name for my plugin, but since these two functions are essentially the same, I'd still support the OP's request to let add_custom_type() register the class. In the long term though, hopefully we can have a single way to register classes that works across all scripting types.

[aaronfranke]

Assigning the 4.0 milestone because I believe the goal is for the class_name type system to eventually fully replace the add_custom_type() type system. Removing add_custom_type() breaks compatibility, so this needs to be decided on for 4.0. I'm not assigning the "breaks compat" label because this isn't a proposal about breaking compat, but the solution I listed does.

[akien-mga]

What's the status on this issue/discussion in the current master branch?

[KoBeWi]

This is documented now:

Note: Custom types added this way are not true classes. They are just a helper to create a node with specific script.

If you want a class, use class_name. Closing.