godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.08k stars 69 forks source link

Expose PropertyUsageFlags for External Access from Godot in C# and GDExtension dump #7150

Closed Faolan-Rad closed 11 months ago

Faolan-Rad commented 1 year ago

Describe the project you are working on

RhubarbVR a real time collaborative engine excrescences

Describe the problem or limitation you are having in your project

In C# and also in a GDExtension dump there is no way of knowing about the usings to find out info of a property like if it is serialized or not.

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

Adds the number of the property using to the JSON in the API dump for GDExtension and adds a attribute to the C# properties for Reflection and also for c# de-comp referencing

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

Explained above how it is implemented Pull Request

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

It could be used for refreshing when debugging code in C# but even if it was not used often someone would have to generate the code themself and run there own fork of the engine for this feature

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

This is editing the generators for both C# and GD Extension which can not be extended with an asset library

AThousandShips commented 1 year ago
AThousandShips commented 1 year ago

What situations are we dealing with here? How would this be used for doing bindings generation specifically, because you can easily access this information via ClassDB, just not for generating the bindings themselves.

raulsntos commented 1 year ago

It is as @AThousandShips says, this information should be available by using ClassDB which is already exposed to C# and GDExtension (although adding this information to the extension_api.json dump, if it's not already, sounds good to me).

Adding an attribute to every member is something we'll likely want to avoid since it increases the size of the assembly.

I also don't really like that this attribute would be added to the generated C# classes from the engine but they won't be available in user classes unless they manually add them.

Instead of an attribute I would suggest a method that retrieves the PropertyInfo for the requested member, but as mentioned earlier this already exists: ClassDB.ClassGetPropertyList.

Faolan-Rad commented 1 year ago

Mind you I was also sort of wanting to do it for referencing purposes so it could on the engine code gen add a bit to the summary comment instead of adding attributes

AThousandShips commented 1 year ago

I'm still wondering over the actual need for this, like how would bindings generators etc. use this? Because it adds quite a bit of extra data to the extension dump so some more concrete examples of how this data will be used is needed here

Faolan-Rad commented 1 year ago

I so I plan to generate networking coded based off every property and scene object in the engine and I would like to know things like if it serialized or not to know if it should be networked and it would also be nice for me to make my own inspectors/editor UI for the networked scene because I know what is supposed to show up in the editor or not and I like to do this with generated code and not runtime code do to complexions and speed problems

AThousandShips commented 1 year ago

Also I question the usefullness of this given that usage flags are not static, they are mutable and depend on things in the editor and the status of other properties, so I don't think it's even useful to export this like this as you can't rely on it, just take this as an example:

var mat := StandardMaterial3D.new()
mat.shading_mode = BaseMaterial3D.SHADING_MODE_PER_PIXEL
for p in mat.get_property_list():
    if p["name"] == "ao_enabled":
        print(p["usage"])
mat.shading_mode = BaseMaterial3D.SHADING_MODE_UNSHADED
for p in mat.get_property_list():
    if p["name"] == "ao_enabled":
        print(p["usage"])

Will print:

6 (i.e. Editor and Storage)
0 (i.e. None)

For example if you rely only on the API file you won't be able to handle alpha_hash_scale in StandardMaterial3D as it is going to return PROPERTY_USAGE_NONE when generating the API

dsnopek commented 1 year ago

So, this information is NOT for generating a GDExtension binding, but is for a custom application that is part of your project?

If so, couldn't you make a GDScript (or C# script) in your Godot project that looped through all the classes in ClassDB and output the data you needed into a file for you? You could run it based on command-line parameters. Then that data file could be fed into your other application that needs it.

In any case, if this isn't needed for generating a GDExtension binding, I'd argue that we don't need it in extension_api.json.