godotengine / godot-proposals

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

In C#, add an additional parameter to `[Export]` to customize the exported property's name on the property list. #3510

Open MagdielM opened 2 years ago

MagdielM commented 2 years ago

Describe the project you are working on

A 3D prototype with classes that export a considerable amount of properties.

Describe the problem or limitation you are having in your project

Currently, the process to categorize exported script properties in the inspector involves overriding _get(), _set(), and _get_property_list(), ensuring all variables that should be exported are accounted for in each override, as well as making the script in question a tool script, which adds another consideration to keep track of when writing scripts (!Engine.editor_hint). This process can be quite cumbersome, as shown by #3451 and #1255. However, export already exposes much of the functionality you get from adding property entries manually via _get_property_list(), with the only prominent exception being the property's name, since it takes the name of the declared property.

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

Godot already has magic separators (/) to use in place of exporting category properties with PROPERTY_USAGE_CATEGORY, but these can't be used in the variable name in either GDScript or C#. I'm hesitant to suggest this for GDScript given all the 2.0 changes, but for C#, the Export attribute could take on an additional optional parameter to customize the property's display name.

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

An additional overload for the [Export] attribute:

public ExportAttribute(PropertyHint hint = PropertyHint.None, string hintString = "", string displayName = null);

where this:

[Export(PropertyHint.Range, "0, 20", "SomeCategory/SomeProperty")]
public int Property { get; set; }

would serve as syntax sugar for this:

public int Property { get; set; }

public override object _Get(string property)
{
   if (property == "SomeCategory/SomeProperty")
   {
      return Property;
   }
   else
   {
      return base._Get(property);
   }
}

public override bool _Set(string property, object value)
{
   if (property == "SomeCategory/SomeProperty")
   {
      Property = (int)value;
      return true;
   }
   else
   {
      return base._Set(property, value);
   }
}

public override void _GetPropertyList()
{
   return new Godot.Collections.Array
   {
      new Godot.Collections.Dictionary
      {
         { "name", "SomeCategory/SomeProperty" },
         { "type", Variant.Type.Int },
         { "hint", PropertyHint.Range },
         { "hint_string", "0, 20" },
         { "usage", PropertyUsageFlags.Default }
      }
   }
}

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

It can be worked around with very many lines of script and the tedium of making sure to wrap any code you don't want to run in the editor around if (!Engine.EditorHint).

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

Addons can't modify the way Godot's C# attributes work.

andy-noisyduck commented 2 years ago

A similar alternative would be to keep the handling of the property name as it is now, but allow a category string to be passed via the attribute.

MagdielM commented 2 years ago

A similar alternative would be to keep the handling of the property name as it is now, but allow a category string to be passed via the attribute.

True, but letting you customize the name also opens the door for displaying things with names that variables can't have, like names starting with numbers ("2DSomethingOrOther"). Although you could have both, I guess. Maybe the attribute could treat "names" ending with a separator as category names and tack on the property name at the end.

YuriSizov commented 2 years ago

Just a clarification, because you called it displayName in the example:

Properties can't have a display name. You can change the name of the property itself, but its display name is exactly what the property name is with additional formatting optionally applied. What you suggest here is a way to define the exposed property under a different name than the backing internal field.

PS. It would also make sense to not limit this to just Mono.

MagdielM commented 2 years ago

Properties can't have a display name. You can change the name of the property itself, but its display name is exactly what the property name is with additional formatting optionally applied. What you suggest here is a way to define the exposed property under a different name than the backing internal field.

Oh, so this is why we override _get() and _set(). Interesting.

Also yeah I thought about making it a general proposal, but I haven't looked into how the new @export annotation is different from the keyword, so I'm not in a position to suggest changes to how it's handled in GDScript.

GeorgeS2019 commented 2 years ago

This PR: Add C# resource export could be relevant

willnationsdev commented 2 years ago

My C# resource export PR currently does not account for all of the different arguments, and if you use [Export("SomeName")], then that becomes short for [Export(hint=PROPERTY_HINT_RESOURCE_TYPE,hintString="SomeName")]. It wouldn't be hard to add the extra simplified constructor as an option though.

fhd commented 1 year ago

I'd love to have this mainly to stay consistent between properties exported from GDScript and C#. Right now my options are:

  1. Use lower_case_with_underscores for exported properties in C#.
  2. Use PascalCase for exported properties in GDScript.
  3. Have PascalCase and lower_case_with_underscores depending on whether a script was written in C# or GDScript (i.e. give up on consistent naming).

I'm currently going with option 1, but it's a bit irritating.

natrist commented 10 months ago

Is there any progress on this issue?