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

C#: Add an easy and accessible way of organizing export variables #3451

Closed
svprdga opened this issue Oct 20, 2021 · 10 comments
Closed

C#: Add an easy and accessible way of organizing export variables #3451

svprdga opened this issue Oct 20, 2021 · 10 comments

Comments

@svprdga
Copy link

svprdga commented Oct 20, 2021

Describe the project you are working on

I am working in a 2D platformer, but this proposal is applicable to any kind of game made within Godot.

Describe the problem or limitation you are having in your project

I have several nodes with attached scripts that contain many export variables. When the number of export variables starts to grow, it becomes harder and harder to organize them in a visible way, as there are many of them and one has to spend some seconds finding the right var in the editor.

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

I know that the docs already explain how to create groups, but honestly I don't understand anything. Also, it seems that I have to state that the script is a tool, but it is not a tool, it's a regular script attached to a node, and I would like to easily organize the export vars.

So I think that to add some way of organize and add a header title to a group of variables would be helpfull.

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

Let's take a look at how Unity handles that:

        [Header("References")]

        [SerializeField]
        private HealthBar healthBar;

        [Header("Layer masks")]

        [SerializeField]
        private LayerMask enemy;

In this example, the SerializeField is the equivalent of Godot's export var. In this Unity script, one can easily create groups with a header using the Header annotation, including the name of the group within it.

It would be great that something like that could be added to Godot.

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

Based on the documentation, it seems to me like it can be worked around but not with a few lines of script. Also, the documentation does not explain clearly how to do it, in my opinion.

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

This should be core because organizing export vars is something that should be inherent to the workflow of creating a game. I think that it's a natural desire for a developer to organize things into things, and when you have a script that contains many many variables you start to feel the need to organize them, in the same way that we organize our project file structure, we organize our nodes...etc.

@YuriSizov
Copy link
Contributor

Also, it seems that I have to state that the script is a tool, but it is not a tool, it's a regular script attached to a node, and I would like to easily organize the export vars.

A tool script is a script that is run in the editor, that's all. And yes, you would need to declare it as such for the _get_property_list() to work.

See also #1255

@svprdga
Copy link
Author

svprdga commented Oct 20, 2021

What I don't get from the documentation is the next step after creating the array of properties, so one has to call _get_property_list() and then what needs to be done with that value? Why a group can't be added naturally in its place between each export var? That's the kind of enhancement I'm proposing.

@YuriSizov
Copy link
Contributor

What I don't get from the documentation is the next step after creating the array of properties

You don't create it. The way it works is that whenever the properly list is believed to have been changed (either from internal processes or from you calling property_list_changed_notify()) this method is called to contribute to the list of properties. Properties defined with this method are added to the end of the property list, after the native built-in properties of the class and after the properties marked for export (note: export is just a keyword, but internally it works the same as if you were to define the properties in _get_property_list()).

You can't define groups for export-marked properties because there is no syntax for it in GDScript, and therefore no syntax for it in other languages. As a workaround you would need to define all your grouped properties in _get_property_list() along with groups. It's a low-level way, but it's the same as the engine works.

Depending on how you organize things your _get_property_list() may be enough as it will fall back on the existing class members, or you may need to implement _set() and _get() as well.

For example (in GDScript):

extends Node

var dictionary : Dictionary = {}

func _get_property_list() -> Array:
    var props := []

    for key in dictionary:
        props.append({
            "name": "some_prefix/%s" % [ key ],
            "type": TYPE_...,
            "usage": PROPERTY_USAGE_DEFAULT, # Default is basically the same as what `export` does, but you can do other things. For example NOEDITOR will still save it, but hide from the inspector.
            
            "hint": ..., # If required by the type/your custom logic
            "hint_text": ...,

        })

    return props

func _set(name: String, value) -> void:
    if (name.begins_with("some_prefix/")):
        var dict_key = name.trim_prefix("some_prefix/")
        dictionary[dict_key] = value
        return true
    
    return false

func _get(name: String):
    if (name.begins_with("some_prefix/")):
        var dict_key = name.trim_prefix("some_prefix/")
        if (dictionary.has(dict_key)):
            return dictionary[dict_key]
    
    return null

Or an example with grouping from a nice plugin by Arnklit: https://github.com/Arnklit/ShellFurGodot/blob/main/addons/shell_fur/shell_fur_manager.gd

It's a very powerful system, as it allows you to implement dynamic properties, changing what you display in the Inspector based on context, creating pure storage fields or pure editor-visible fields, etc. So it's worth learning even beyond grouping.

@svprdga
Copy link
Author

svprdga commented Oct 20, 2021

I see, I have tried your code but I don't see changes; I have implemented your three methods and then I have called in the _ready() function of the node:

_set("some_prefix_test", 0)
property_list_changed_notify()

What is supposed to happen after this?

@YuriSizov
Copy link
Contributor

What kind of changes do you expect to see? What do you see now?

By the way, when the node is initially loaded it already calls _get_property_list(), it is called automatically by the engine as the list of properties is initially read. What you see in the Inspector depends on what your _get() method returns. Or, if you just use a backing field with the same name, it will just display its value (see the linked example from a plugin above for that kind of deal).

@svprdga
Copy link
Author

svprdga commented Oct 20, 2021

I respect that properties system, and I have no doubt that is a powerful system to do stuff; but this is not my proposal. My proposal is to add a extremely simple way to group exported vars, just like the Unity example that I have posted. If this can be done via this properties systems is wonderful, but I think that for most people, including myself, just adding some This is my header in the script above the exported vars that I want to group would be better, because I don't see the point in messing arround with multiple lines of code when this could be done with one single statement.

I understand that with the current state of GDScript this cannot be done; then my proposal is to modify GDScript to allow it.

@Calinou
Copy link
Member

Calinou commented Oct 20, 2021

I respect that properties system, and I have no doubt that is a powerful system to do stuff; but this is not my proposal. My proposal is to add a extremely simple way to group exported vars, just like the Unity example that I have posted.

In this case, this proposal is a duplicate of #1255.

@DriNeo
Copy link

DriNeo commented Oct 20, 2021

Why not export a data structure that represents the new inspector section ? A dictionnary annotated as "export" as instance ?

@YuriSizov
Copy link
Contributor

YuriSizov commented Jul 4, 2022

In this case, this proposal is a duplicate of #1255.

I think we can keep this one open specifically for a .Net solution. #1255 is for GDScript, and implementation of this feature is independent for both languages, and doesn't require any common internal changes.

@YuriSizov YuriSizov changed the title Add an easy and accessible way of organizing export variables C#: Add an easy and accessible way of organizing export variables Jul 4, 2022
@raulsntos
Copy link
Member

raulsntos commented Aug 28, 2022

Implemented by godotengine/godot#64742 and godotengine/godot#64852

@raulsntos raulsntos added this to the 4.0 milestone Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants