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

Implement specific functional metadatas for Script instances that contain custom global classes #11238

Open
2 tasks done
Lazy-Rabbit-2001 opened this issue Nov 27, 2024 · 13 comments

Comments

@Lazy-Rabbit-2001
Copy link

Lazy-Rabbit-2001 commented Nov 27, 2024

Discussed in #11390 (also a vote)

Describe the project you are working on

Script improvement + Addons

Describe the problem or limitation you are having in your project

Extension of #1047
During the practice of Godot, I (together with some users) have encountered these inconveniences:

  1. Some custom global nodes/resources are always visible in create dialogue, which are, in practice, just for private use and polluting the dialog.
  2. Some custom global nodes/resources that should be visible displays with suffix, which would be a bit incoherent with other items in the create dialogue. Sometimes we want the suffix to be something else that contains some information.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

They all have one solution that is flexible and optional: metadata.
Since a metadata is allowed to be manually added to a script in the latter's inspector, this would be solved in few fingers.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Adding some metadata - object_show_in_dialog and script_suffix respectively.

  • object_show_in_dialog: The value should be a bool. When it is true or the metadata is not set, the node/resource will be shown in the create dialog; otherwise, the node/resource will be hidden from it.
  • script_suffix: The value can be anything, but better be a StringName or String. This allows a user to replace the suffix of a node/resource in create dialog to a custom one:
Node
|- MyCustomNode (test.gd)

to

Node
|- MyCustomNode (custom suffix)

Also, if the suffix is an empty string, there will be no suffix shown.
This is useful for adding a pseudo namespace for each nodes, or some short notes stick to the class name.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Since the create dialog and the scene tree dock is not exposed to the engine, it is impossible.

Is there a reason why this should be core and not an add-on in the asset library?

Same as "If this enhancement will not be used often, can it be worked around with a few lines of script?"

Progress

@Lazy-Rabbit-2001 Lazy-Rabbit-2001 changed the title Implement specific metadatas for Script instances so that they can provide extra technics and functionalities for these instances. Implement specific functional metadatas for Script instances that contains custom global classes. Nov 27, 2024
@Lazy-Rabbit-2001 Lazy-Rabbit-2001 changed the title Implement specific functional metadatas for Script instances that contains custom global classes. Implement specific functional metadatas for Script instances that contain custom global classes. Nov 27, 2024
@Lazy-Rabbit-2001
Copy link
Author

Lazy-Rabbit-2001 commented Nov 27, 2024

It is a problematic work to remove the script icon (or "open script" button) from the scene tree editor, and will cause potential issues, so I removed it from the list

@Calinou Calinou changed the title Implement specific functional metadatas for Script instances that contain custom global classes. Implement specific functional metadatas for Script instances that contain custom global classes Nov 27, 2024
@KoBeWi
Copy link
Member

KoBeWi commented Dec 17, 2024

Related to #1047 and #4493 (for hiding the types)
Hacking the create dialog with global settings sounds more like a workaround.

Also not sure about having a setting to remove prefixes. I mean, wanting to change or remove them makes sense, but a global setting that handles individual scripts seems weird.

@Lazy-Rabbit-2001
Copy link
Author

Lazy-Rabbit-2001 commented Dec 18, 2024

Related to #1047 and #4493 (for hiding the types)
Hacking the create dialog with global settings sounds more like a workaround.

Also not sure about having a setting to remove prefixes. I mean, wanting to change or remove them makes sense, but a global setting that handles individual scripts seems weird.

First, I used to rely on using gdmeta but that was yk a future topic (for G5). Second, using an annotation only solves the issue on GDS-side (and C#-side), but other scripts (i.e. scripts supported in other languages by GDExtension) have to get themselves compatible with the feature so they have to synchronize the feature as well, which, compared with global setting which does not require unnecessary changes to the source code, is redundant and unnecessary tbh.
LBNL, for addons, a main script can be responsible for controlling the settings, by inserting them into the list on registration or removing them from the list on unregistration, which is not difficult to do.

As for removing prefixes... pardon but I don't really get any idea where and when a prefix would appear in the dialog... Could you explain more about this?

@KoBeWi
Copy link
Member

KoBeWi commented Dec 18, 2024

Sorry, I meant suffix.

LBNL, for addons, a main script can be responsible for controlling the settings, by inserting them into the list on registration or removing them from the list on unregistration, which is not difficult to do.

Sounds like there should be a method that does that; modifying an array/dictionary setting manually is not very intuitive.
However this only solves the problem for addons, while the other proposals I linked are about scripts in general. It's not very convenient having to create an addon every time you want to hide a type.

@Lazy-Rabbit-2001
Copy link
Author

Lazy-Rabbit-2001 commented Dec 18, 2024

Sounds like there should be a method that does that; modifying an array/dictionary setting manually is not very intuitive.
However this only solves the problem for addons, while the other proposals I linked are about scripts in general. It's not very convenient having to create an addon every time you want to hide a type.

For non-addon scripts, the users can directly set the array/dictionary, which is a project setting in advanced mode, as long as the script is not going to be used like an addon that is shared with public users. If the script does be going to be addon-like, e.g. a global script class that provides extra functionalities for the engine, a main script of it is unavoidable if the developer wants the engine to automatically hide/show the type or add/remove a suffix on its registration or unregistration, as long as the developer wants to apply this feature to its plugins.

To get users quickly realize this, we can document how to use type blocklist and custom type suffixes in godot-doc, but ofc this is another topic.

TBH I don't really have any better idea to get this generally solved in fewer codes, but at least project setting solves 85%~90% of the problem we have imo. We can expect a future intelligent to help achieve it.

p.s. Maybe, for general scripts, we need a _global_class_register() and a _global_class_unregister() used on class (un)registration, and using tool script to do (un)registeration with operating these two global settings...

@KoBeWi
Copy link
Member

KoBeWi commented Dec 19, 2024

The problem with this is that if we eventually get a proper way to hide classes from CreateDialog (via annotation or special keyword or whatever), the settings will become some subpar legacy thing.
The suffix could also be customizable with an annotation, or maybe we can add script metadata like you suggested. The settings are nothing more than a temporary workaround.

While I'm personally rather against the current implementation, it's not a final decision. It would be nice if more contributors/users could voice their opinion. So far the proposal has mixed reaction.

@Lazy-Rabbit-2001
Copy link
Author

The problem with this is that if we eventually get a proper way to hide classes from CreateDialog (via annotation or special keyword or whatever), the settings will become some subpar legacy thing.
The suffix could also be customizable with an annotation, or maybe we can add script metadata like you suggested. The settings are nothing more than a temporary workaround.

While I'm personally rather against the current implementation, it's not a final decision. It would be nice if more contributors/users could voice their opinion. So far the proposal has mixed reaction.

As I said above I do personally not prefer adding elements on scripting side, and editor settings allows us to maximize the efficiency of synchronizing the feature among all possible languages.
Ofc we need a survey of other users' preference. I'm planning to make a vote in both rocket chat and the discussion to hear their voices on this proposal.

@HolonProduction
Copy link
Member

I'm very skeptical about the suffix part in general. This is information that the dialogue gives about the kind of thing I am going to add. I want this information from the dialogue and I don't want e.g. plugin authors to masquerade their custom script types as native types in the dialogue. Those kinds of types have different properties, so for me this is important information, not a canvas to add custom information.

If the suffix is too disruptive when navigating the dialogue, it could be reduced to an icon on the right side which has the script path as hover hint. But making it overridable has the risk of adding lots of noise when using a combination of different plugins which utilize it differently, and it would remove information from the dialogue, which might be important to some users (me 😢).

@dalexeev
Copy link
Member

I agree with HolonProduction. In my opinion, it was never intended to be a suffix, just an indicator to differentiate global script classes from native ones. We could replace the script file name with an icon and add filters to the Create New Node/Resource and Search Help dialogs, see:

I don't think we should add the ability to customize these editor dialogs. But if for some reason we decide to do so, it would be more appropriate to do it through an editor plugin, not through script metadata or annotations.

If you want to display some additional information about the class, you can use documentation system for that:

Image

As for hiding some custom classes from editor dialogs, there is already multiple proposals and PRs:

@KoBeWi
Copy link
Member

KoBeWi commented Dec 21, 2024

Note that you can already customize suffixes when opening custom CreateDialog (godotengine/godot#100135). If we redesign the prefixes, this functionality might need to be removed or reworked (which can realistically happen only until 4.4 stable).

@dalexeev
Copy link
Member

Note that you can already customize prefixes when opening custom CreateDialog (godotengine/godot#100135).

In my opinion, this is excessive functionality and overengineering.

@Lazy-Rabbit-2001
Copy link
Author

Lazy-Rabbit-2001 commented Dec 21, 2024

Sorry for what I was doing because of my hurry-doing mind when I totally forgot what i said in this thread:

As I said above I do personally not prefer adding elements on scripting side

Technically it never should be a problem solved by annotations. This is what we should insist imo.

Note that you can already customize prefixes when opening custom CreateDialog (godotengine/godot#100135). If we redesign the prefixes, this functionality might need to be removed or reworked (which can realistically happen only until 4.4 stable).

Thank you for mentioning this. But I don't think this is a trivial feature. At least there will be a fast access to get a window that allows a plugin developer not to make wheels. I think, if possible, we can refactor the indicator into a link: To remove the text suffix, and add a button with a link button, which allows to open the script that the type refers to. This is better to be done before 4.4 gets stable.

@HolonProduction
Copy link
Member

If we redesign the prefixes, this functionality might need to be removed or reworked (which can realistically happen only until 4.4 stable).

IMO this should have been marked experimental in the first place, and we should mark it as such before 4.4 to keep our options open (if not the whole method then at least the suffix parameter, which I'd honestly remove completly). If we keep exposing editor stuff with options that are specific to the current UI and provide stability guarantees for it, improving on the editor will become very difficult and slow.

Personally I think we should only provide stability guarantees on any editor stuff between patch versions (still keeping compat on best effort basis, but EditorInterface compatibility shouldn't hinder UX improvements IMO).

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