neikeq / GodotSharp

Moved to the Godot repo: https://github.com/godotengine/godot/tree/master/modules/mono
MIT License
60 stars 8 forks source link

Exported variables #4

Open neikeq opened 8 years ago

neikeq commented 8 years ago

Implement the Export attribute including a PropertyUsage enum (Actually, the PROPERTY_USAGE_* constants will be exposed; preferable as enum constants) and a PropertyHint (PROPERTY_HINT_* constants will be exposed too) interface with its implementations classes.

There is range for improvement here in comparison with GDScript's keyword. C# is a statically typed language so the type does not need to be inferred or manually specified. C# supports enums, so the enum hint is unnecessary.

ghost commented 6 years ago

Has this been implemented?

neikeq commented 6 years ago

Yes, but there are still improvements we can make compared to GDScript. e.g.: Filling hint_string automatically for enums.

ghost commented 6 years ago

Great! Are there any examples about how to export variables as in GDScript?

neikeq commented 6 years ago

The most basic one would be:

[Export]
float speed = 200f;
NathanWarden commented 6 years ago

Nice to know this is working... I think it also needs to be public, so

[Export]
public float speed = 200f;
neikeq commented 6 years ago

BTW, is that ok? Should we also allow it for non-public fields as well? Another TODO is to make it work for properties as well.

NathanWarden commented 6 years ago

Thanks for asking. Since we're explicitly placing an attribute, I think it would be good to do it for private fields too. You may want to change these fields per node in the editor, but still keep the code clean by keeping them private from other classes.

Valentactive commented 6 years ago

Yeah it should be public and I don't know if it makes sense to expose also private ones. Normally private variables are manipulated in C# either through methods or through a public propriety with getter/setter logic.

neikeq commented 6 years ago

Yes, I think the good practice is to allow it only for public members. The problem is C# attributes cannot be made to target only public fields, so it would be a runtime error printed by the editor or silently ignoring it like it's done right now.

NathanWarden commented 6 years ago

As an example, in Unity they automatically export all public variables, but you can place an attribute in order to expose a private field to the editor. Just so I don't misrepresent myself, I like having to add an attribute on all exported variables, as this keeps the inspector clean :) (IE. exposing all public variables automatically can get messy)

Several Unity developers/plugins do expose private variables to the inspector. I think the most common reason for this is that there are sometimes values you want the user to be able to change as initial values, but they shouldn't be changed at runtime. So they're exposed to the inspector, but not via the API.

If a person feels they should only have exported variables be public, they can always simply choose not to put the export attribute on any of their private variables. In other words, allowing for both public and private variables to be exported allows everybody to work according to their own style.

ghost commented 6 years ago

@neikeq New script variables appear in the property editor only after restarting the editor. I tried saving the script file, closing and reopening the scene but it hasn't worked so far.