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

Export statements in GDScript are confusing #298

Closed
YuriSizov opened this issue Dec 11, 2019 · 17 comments
Closed

Export statements in GDScript are confusing #298

YuriSizov opened this issue Dec 11, 2019 · 17 comments
Milestone

Comments

@YuriSizov
Copy link
Contributor

Describe the project you are working on:
Not applicable.

Describe the problem or limitation you are having in your project:
Hinted export statements lack clarity, consist of arbitrary list of arguments and conflict with typed declarations. Single export keyword is clean and concise, but if hints are needed to support property editor, then having documentation handy is a requirement to deal with them, unless you have memorized it already.

  • As the only operative word in the whole statement is export, and everything else is varying depending on the first argument (the hinted type), number of arguments, their types and special meaning, one cannot read and understand such statements without looking up the documentation, which is unfriendly to newcomers and a nuisance for advanced users.
  • Another problem with an arbitrary list of arguments is the lack of code hinting/completion, as overloading possibilities overlap, so a developer doesn't even know what options are available for hinting without consulting with the documentation, again.
  • Since we have typed statements now, having an export with another type reference causes another nuisance (you cannot omit it if you need other hints) and allows for mixed intentions, such as
# Works for types that are cross-convertible, I guess
export(int) var variable1 : float
export(float) var variable2 : int

# Works for any extended type and their ancestor, but not the other way
export (Script) var resource : Resource
  • Big number of hints extends the line of code noticeably, which limits its comprehension, as we can easily push past comfortable length with an export statement, a typed declaration and a setget statement. Arguably, the most important part of the line is declaration, and it is somewhere in the middle of the line.

Describe how this feature / enhancement will help you overcome this problem or limitation:
Improving export statement syntax can make code clearer to read and and cleaner to write. This feature feels like it was introduced a long time ago and is heavily outdated in the current state of the language. I think, that in such state extended export hinting is unusable, unless you really need to have it.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:
Not applicable.

Describe implementation detail for your proposal (in code), if possible:
I can only imagine annotations as being a viable solution, which is suggested in #20318. However, this solution was not approved by the core team and is up in the air for the moment.

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

Is there a reason why this should be core and not an add-on in the asset library?:
It involves GDScript parser and editor changes.

@willnationsdev
Copy link
Contributor

To add to this: there is not yet any way to have fine-tuned control over exports either, nor is there an idiomatic way of conveying what goes into defining an export because the entire concept of PropertyInfo and MethodInfo objects is hidden from the end-user.

I would argue that being more direct with end-users about the use of *Info classes would go a long way towards clarifying and easing their use.

  1. Make PropertyInfo and MethodInfo built-in types that are possible Variant states (how feasible is this?).
  2. Grant the script-exposed versions of these classes various constructors and/or static factory methods for generating specific configurations of them.
  3. Modify GDScript's export(), C#'s [Export], and/or the exports in VisualScript/NativeScript to support directly assigning *Info objects to an export declaration. Ensure that the interface for building these objects statically still has autocompletion of their required/optional fields, constructors/factory methods, etc.
  4. Create API documentation that outlines how those objects are built, what their fields are, when and how they should be used, complete with examples.

@KoBeWi The stated complaints about exports are applicable to all languages, not just GDScript, since each language currently takes a similar approach to defining exports, i.e. hides *Info objects from users. As such, I think this would be a concrete solution to making the experience of dealing with exports better for all languages. It also makes the documentation regarding reflection methods like get_property_list() and get_method_list() a lot clearer to understand.

@YuriSizov
Copy link
Contributor Author

@willnationsdev Can you elaborate what using such *Info structs may look like in code?

@willnationsdev
Copy link
Contributor

willnationsdev commented Dec 18, 2019

@pycbouh So, for example, a PropertyInfo struct represented as a Dictionary in GDScript (from, e.g. get_property_list()), looks like this:

{
    "name": <string>,
    "type": <TYPE_* int>,
    "hint": <PROPERTY_HINT_* int>,
    "hint_string": <string>,
    "usage": <PROPERTY_USAGE_* bitflag int>,
    "class_name": <string>
}

When you declare that a property is exported, it automatically modifies the usage field to add PROPERTY_USAGE_EDITOR to the bitflag value. It also automatically grabs the property name and uses that to construct the corresponding field. If it is a statically typed language/value, then it can infer the type value; otherwise, you will need to provide a type hint to the export. But, you can also create a type hint that exports a type different from the type of the actual property, as you observed:

# A float value that nonetheless exposes an integer property editor to the Inspector.
# The value is casted to a float when the scene initializes the node's/resource's properties.
export(int) var num := 0.0

The most direct way to control the content would be to have the ability to directly assign values to the PropertyInfo object. And in fact, C#'s [Export] keyword does this, enabling you to pass in the hint and hint_string fields directly as parameters of the [Export] attribute:

[Export(PROPERTY_HINT_FILE, "*.tscn, *.escn, *.scn)]
public string scenePath = "";

On the upside, it enables people to rely on the @GlobalScope.PROPERTY_HINT_* flags to get autocompletion on the hint value. On the downside, it still requires that users be intimately familiar with what format of hint_string goes paired with what hint value. In addition, some hints are only compatible with certain types. For example, PROPERTY_HINT_ENUM can only be used with TYPE_INT (and sometimes TYPE_STRING?) and expects a comma-delimited list of the strings that will be displayed in the Inspector's dropdown list, top-down.

enum CardType {
    CLUBS,
    SPADES,
    HEARTS,
    DIAMONDS
}

# GDScript
export(int, ENUM, "Clubs", "Spades", "Hearts", "Diamonds") var card_type := CardType.CLUBS

# GDScript since 3.1.x iirc
export(CardType) var card_type := CardType.CLUBS

// C#
[Export(PROPERTY_HINT_ENUM, "Clubs, Spades, Hearts, Diamonds")]
public CardType cardType = CardType.CLUBS;

Another example is exporting resources which must have hint=PROPERTY_HINT_RESOURCE_TYPE, hint_string=<class_name>,class_name=<class_name>. Iirc, the hint_string class_name is for identifying the specific property editor in the Inspector (which affects what value ends up being stored in the scene and assigned to the property later) and the second class_name class_name is so that reflection can determine what actual type the property is supposed to be (since TYPE_OBJECT isn't specific enough). If there is a type mismatch between the hint_string and the class_name to where hint_string could never validly be assigned to a property of type class_name, then you would get an error. Example, PackedScene can be assigned to Resource, but Resource can't be assigned to PackedScene.

And, unfortunately, absolutely none of this information is properly documented in the official documentation because it all happens behind-the-scenes in the engine code. And yet, there is just enough of it exposed to the end-user that they still need to know about it, and yet can't find any tutorials on it, any API docs about PropertyInfo/MethodInfo, any explanations as to how the property/method/signal serialization and reflection process works in the engine at all. You just have to be "in-the-know" by looking at the source code.

So, aside from exposing all of this information, demanding that users use the full globals (to get autocompletion) and deal with full PropertyInfo structs (so that we can get them into the documentation), and then actually creating good documentation about the content and how it works, there isn't a lot more we can do.

An example I could see of an updated version of GDScript using full stuff would be something like this (more verbose, but potentially has better autocompletion/docs):

# Method 1: C#-like, just give hint/hint_string
export(PROPERTY_HINT_FILE, "*.tscn, *.escn, *.scn") var scene_path: PackedScene = null

# Method 2: able to plug in PropertyInfo objects that are statically accessible
# This is useful for transient Resource properties exposed to the Inspector, but which you don't want to bother saving.
const EXPOSE_BUT_DONT_SAVE = (PROPERTY_USAGE_DEFAULT & ~PROPERTY_USAGE_STORAGE) | PROPERTY_USAGE_SCRIPT_VARIABLE | PROPERTY_USAGE_EDITOR
const SCENE_DISPLAY = PropertyInfo(PROPERTY_HINT_FILE, "*.tscn, *.escn, *.scn", EXPOSE_BUT_DONT_SAVE)
# could re-use the constant as needed
export(SCENE_DISPLAY) var scene_path_1 := ""
export(SCENE_DISPLAY) var scene_path_2 := ""

# Method 3: able to plug in static function calls, mixed into other options
static func scene_exts() -> String:
    var arr = ResourceLoader.get_recognized_extensions_for_type("PackedScene")
    for i in len(arr):
        arr[i] = "*." + arr[i]
    return arr.join(",")
export(PROPERTY_HINT_FILE, scene_exts()) var scene_path_1 := ""

# Method 4: Annotations, direct overrides (one of the cleanest ways).
# With this, you would get
# - autocompletion on what annotations can be defined
# - autocompletion on possible values for each of those annotations
@hint PROPERTY_HINT_FILE
@hint_string scene_exts()
var scene_path_1 := ""

I know @reduz at one point mentioned that he had ideas on how to improve/change the export process, but I'm not sure what those ideas entailed. Perhaps that could illuminate another path forward.

If you really wanted to improve the readability of the codebase using PropertyInfos and whatnot, without taking options away from the user, I feel like you'd have to make deeper changes to the system and the way it works.

@willnationsdev
Copy link
Contributor

willnationsdev commented Dec 18, 2019

What's funny is, if you look at the content that GDScript actually requires users to put in the export parameters, it really is just a more concise and readable version of the bare minimum of fields already present in the PropertyInfo struct. It lends itself to GDScript's readability. It just omits the PROPERTY_HINT_ part of PROPERTY_HINT_FILE, assumes that you are making a script variable and that you want to export the value to the Inspector, infers the name of the exported property by the variable name, infers the variable type if possible, etc., but otherwise, it more or less works the same way:

# some GDScript code
export(String, FILE, "*.tscn") var scene setget setter, getter
vs.
// similar C++ engine code
ADD_PROPERTY(PropertyInfo("scene", TYPE_STRING, PROPERTY_HINT_FILE, "*.tscn", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_SCRIPT_VARIABLE, ""), "setter", "getter");

@cgbeutler
Copy link

cgbeutler commented Jan 22, 2020

Good discussion so far.
I think it's also worth noting that the current export syntax has potential issues for supporting types with multiple sub-types, like dictionary (e.g. confusing to see export(Dictionary, Array, int, int)) The current sytax has no way of marking the start and end of a sub-type. With this lack of context boundaries, optional arguments become either confusing or impossible.

One way to maybe clarify things would be to add in context boundaries. This could be done with a list, which would also keep things backwards compatible (as export can just check for a list or not):

# Export could take in arrays for sub-types.
export( Array, [int, "left", "center", "right"] ) var col_alignments

# This would allow export to have context-boundaries for its sub-types
export( Array, [Array, [int, "left", "center", "right"] ] ) var cell_alignments

# It would maybe also make it easier to support future types like
# dictionaries, where multiple sub-types will be needed
export( Dictionary, Array, int, int ) # confusing
export( Dictionary, [Array, int], int ) # less confusing
# even more complex example:
export( Dictionary,
    [String, "StateA", "StateB", "StateC"],
    [Array, [String, "StateA", "StateB", "StateC"] ]
) var transitions

There are multiple variations of where the braces could go, so it may still be a bit confusing in the end. Overall, I would prefer a new syntax that was even more friendly, but I would settle for this fairly minimal change (syntax-wise. Don't know how hard it would be to implement code-wise.)

@vnen
Copy link
Member

vnen commented Jan 23, 2020

Haven't read the full discussion, but we already have plans to change the export syntax to make it more straightforward.

@willnationsdev
Copy link
Contributor

@vnen are these plans detailed anywhere? I'm curious about it.

@vnen
Copy link
Member

vnen commented Jan 23, 2020

@reduz did put some ideas in a document. I'll paste here the relevant for the export syntax. But this is just a draft will still be fully discussed. (Sorry for the formatting, this wasn't markdown).


Right now GDScript has it’s own export hint syntax, while Godot uses another one (check object.h). The one from Godot is very complete, but not much is available on GDScript. Added to this, given GDScript has optional typing, the old export syntax does not make much sense.

I think we should

Remove the old export syntax “export(type)” and just go with “export var something:type”.
Remove the old export() hint system and re-bind the PROPERTY_HINT system. We could do by code that the syntax is changed as the following:
Remove “PROPERTY_HINT” and convert the rest to a CamelCase identifier, this way: “PROPERTY_HINT_ENUM -> Enum” or “PROPERTY_HINT_MULTILINE_TEXT” -> “MultilineText”.
Arguments after the hint identifier will be always treated as strings, no matter if they are integers, strings or identifiers. If more than one argument is passed, they will be joined in the string with commas (“,”). This way everything should work fine.
For some exports, the type is obvious, so we should have a table with default types for each (or should it be forced?)

Examples for new syntax:

The following are all the same:

export(Enum,”One”,”Two”,”Three”) var myenum : int
export(Enum,”One,Two,Three”) var myenum : int

The following are also all the same:

export(Range,0,100,1) var myrange : int
export(Range,”0,100,1”) var myrange : int
export(Range,”0”,”100”,”1”) var myrange : int

Same here:

export(File,”.png,.jpg”) file_path : String
export(File,”.png”,”.jpg”) file_path : String

@willnationsdev
Copy link
Contributor

willnationsdev commented Jan 23, 2020

I like this idea, but what do you do to replicate features that have non-user-friendly manipulation of the hint string? For example, Array type hints have a very confusing hint string that stores multiple values in a specific format.

In addition, I think we should enable users to supply custom usage flags this way, as the final parameter perhaps.

@YuriSizov
Copy link
Contributor Author

@vnen
So far this draft only covers the fact that there is a type system now, so double type declarations are unnecessary and sometimes confusing. The three other problems I've listed are still present, plus another one has been produced. To recap:

  1. While given examples are obvious, in practice we would still have cases where export declaration is unreadable without good knowledge of documentation. Reading code is by far more important and time consuming when writing it, so I feel that obviousness of intent should be put first. For example, just as @cgbeutler mentioned, Arrays and Dictionarys would still be a mess.
  2. For the same reasons it is hard to interpret such syntax, it is too arbitrary to have proper code completion. Passing everything as a single string is even more confusing, as it is not how arguments are actually used in any programming language. How am I as a user of Godot supposed to know, what types of arguments does this or that hint support, if every one of them is a string and can be combined into a single string?
  3. Complex export statements would still make declaration hidden somewhere in the middle of the line. This hurts readability, and this draft only reduces the offset by one argument (the type), but then it introduces new hints that previously were inferred in its place.

And the new one is:

  • Bundling every argument into a single string may look cool for a beginner that is only learning (game) programming, but introduces "magic" behavior that is harder to control and work around. I see no reason an experienced programmer would want such "magic" and for beginners the other issues must be more prevalent, in my opinion.

In my opinion, if we are going to stick to some resemblance of the current syntax, because of the believe that it is easier to use for someone who is just introduced to the engine and its scripting language, then we should probably make the following adjustments:

  • Go along with this draft and allow a few ad-hoc cases to be setup using it, but probably reduce supported cases to only those that do not bring more confusion into it (e.g. everything with nesting goes out of the window)
  • Allow the first argument to be a special export struct / export typedef, that can be setup by a poweruser to produce a clear definition for something more complex or space consuming.

That way we can keep a simple way to setup a simple case, but there will be a solution for those, that require finesse.

@vnen
Copy link
Member

vnen commented Jan 24, 2020

First of all, draft isn't really mine. I was just sharing for context. We haven't discussed it yet, so don't treat it as final (hence why called "draft").

  1. While given examples are obvious, in practice we would still have cases where export declaration is unreadable without good knowledge of documentation. Reading code is by far more important and time consuming when writing it, so I feel that obviousness of intent should be put first. For example, just as @cgbeutler mentioned, Arrays and Dictionarys would still be a mess.

While I do agree that reading is important, the context of the hints is not a hindrance to understand what the code does. You know there's an export and the PROPERTY_HINT_* name should be fairly obvious. Even if you don't understand the hint at first, even with the ability to check the Inspector and see what it does, it won't stop you from understanding the rest of the code.

Those mentions of "syntax mess" for Arrays is more an issue of the current system than the new one. The new won't even need any hint, since we will have typed arrays anyway. Just use export without hint and declare the variable as an array of ints, it should just work. Or an array of an enum, for that matter.

AFAIK there's no typed dictionaries. Not sure if it's gonna be added (by setting types for both key and value), but if so the explicit type specifier should be enough too.

2. For the same reasons it is hard to interpret such syntax, it is too arbitrary to have proper code completion. Passing everything as a single string is even more confusing, as it is not how arguments are actually used in any programming language. How am I as a user of Godot supposed to know, what types of arguments does this or that hint support, if every one of them is a string and can be combined into a single string?

It can still have code completion. The first argument to export is the hint, which is already a limited list. After selecting this, it can show the arguments types/format based on the hint. Having a different syntax for each hint would be much more taxing on memory than understanding the different strings (most are quite trivial to learn, only a few are quite hairy).

They are treated as string (since they are used like this internally) but they need not to be. You can write with actual values (say, numbers for the range hint). I could concede having some restrictions based on the hint, so there's only a single way to do it, but that can get quite tricky to implement.

3. Complex export statements would still make declaration hidden somewhere in the middle of the line.

Well, I might have missed, but haven't seen a proposal here that solves the length issue. Only exception is annotations, since they go in their own line, but we likely won't use annotations for core features. In theory we could allow the var to be on the following line, without needing actual annotations for this.

This hurts readability, and this draft only reduces the offset by one argument (the type), but then it introduces new hints that previously were inferred in its place.

Which hints are introduced by this new system that were inferred before?

  • Go along with this draft and allow a few ad-hoc cases to be setup using it, but probably reduce supported cases to only those that do not bring more confusion into it (e.g. everything with nesting goes out of the window)

I don't think there'll be any nested structure. If anything, we're getting rid of those.

  • Allow the first argument to be a special export struct / export typedef, that can be setup by a poweruser to produce a clear definition for something more complex or space consuming.

This makes sense only if the use case surfaces. Only thing I can see is changing the usage field of the PropertyInfo, which isn't possible with the current (or new) export syntax (you can workaround with _get_property_list() only).

In my opinion, if we are going to stick to some resemblance of the current syntax, because of the believe that it is easier to use for someone who is just introduced to the engine and its scripting language, then we should probably make the following adjustments:

Well, I can understand the critics to the syntax. But without a new counter-proposal to compare, it's hard to tell what you have in mind. I'm missing some new ideas in this thread.

@willnationsdev gave a few examples in #298 (comment) but honestly they look harder to read than the current way.


All-in-all, I did get some insights from the thread. I still think the new syntax will be simpler to write, read, and implement (even with completion support). The current export syntax is one of the most complex part of the GDScript parser, which I definitely want to change.

@YuriSizov
Copy link
Contributor Author

First of all, draft isn't really mine. I was just sharing for context. We haven't discussed it yet, so don't treat it as final (hence why called "draft").

Yes, that was clear. But given that it was still a proposition from a core team member on the same subject as my proposal, I gave it a quick evaluation from my perspective anyway. Did not mean to overly critique it as if it was final, sorry for misconception.

Which hints are introduced by this new system that were inferred before?

Aren't Enum and Range implicit in GDScript currently? Enum is inferred from a list of strings, and Range from one, two or three numeric arguments, according to docs.

Well, I might have missed, but haven't seen a proposal here that solves the length issue.

In my latest comment, I've mentioned an export struct/typedef, that can be used as a sole argument for export(), holding everything within it and defined somewhere else in a file. In effect, that would be like a custom export hint akin to Enum, Range, File, Color, but user defined (and probably parameter-less). That would make statements lines as short as

export(ExportType) var my_var : Type = initial_value

At the very least, we could allow a multiline export statement, because as of 3.1.2 this is invalid:

export(String,
  "Long Option Number One",
  "Long Option Number Two"
) var my_var : String

I don't think there'll be any nested structure. If anything, we're getting rid of those.

How would you unpack this example from the docs, but with further hints for each inner type? For example, if that float is hinted as a range.

export(Array, Array, float) var two_dimensional = [[1.0, 2.0], [3.0, 4.0]]

And though Dictionarys may not be typed in the language at the moment, they may require some sort of hinting in export statements. I know of two use-cases for export statements: serialization (for resources) and Inspector Panel (for resources, scenes and nodes). In both cases, it is useful to have a say at what types of values are allowed in a Dictionary typed field. So this kind of hint may be introduced down the road, and I don't see yet how the draft is covering nesting structures to make such declarations simpler.

This makes sense only if the use case surfaces.

I may not have an eye-opening example that shows how flawed export statements are currently. My main gripe is that what we as developers have right now is an attempt to crumple a complex system into a one-liner, which is by its nature very implicit. I see an appeal of such syntax for an ad-hoc solutions and it is easy to share with newcomers. But as a seasoned programmer I prefer to have explicit declarations in places, where there are multiple ways to configure script's logic.

I don't see, how am I supposed to figure out from code that there is an option or how auto-completion can work in cases where we start to introduce new modifiers in between of old arguments:

export(String, FILE, "*.txt") var f
export(String, FILE, GLOBAL, "*.png") var tool_image

What if there is a need for another flag like GLOBAL? Does order matter? Will this magical list of arguments just expand with an unlimited amount of permutations? And so, it's on a developer to check the docs and find that the options are there in the first place, how to arrange and use them.

But without a new counter-proposal to compare, it's hard to tell what you have in mind.

Well, I suggested a way to replace a string of arbitrary hint arguments with a struct. So something like

export(String, FILE, GLOBAL, "*.png,*.jpg,*.bmp,*.gif") var tool_image

can turn into

export_type ToolImagePath {
  extends: File,
  is_global: true,
  extensions: "*.png,*.jpg,*.bmp,*.gif"
}
export(ToolImagePath) var tool_image

This is explicit, non-ambiguous and can be code-completed the same way that the list of properties is after a "dot". There is no need to introduce new statements, if multiline exports are allowed:

export({
  extends: File,
  is_global: true,
  extensions: "*.png,*.jpg,*.bmp,*.gif"
}) var tool_image

And with this addition, I would prefer to limit the scope of one-liner export statements to a subset of what is currently accepted, and introduce new options in export structs, rather than new hints. So, maybe we can have File there, but not Global; remove Exp from Range, but allow to set it in export struct as one of the functions possible; and no one-line hinting for Arrays.

@vnen
Copy link
Member

vnen commented Jan 24, 2020

Aren't Enum and Range implicit in GDScript currently? Enum is inferred from a list of strings, and Range from one, two or three numeric arguments, according to docs.

Yes, but you also need the type. Since the type won't be needed anymore, adding the hint won't increase the number of arguments. Also, for Enum you can simply type as a GDScript enum, there's no need to use any hint at all.

In my latest comment, I've mentioned an export struct/typedef, that can be used as a sole argument for export(), holding everything within it and defined somewhere else in a file. In effect, that would be like a custom export hint akin to Enum, Range, File, Color, but user defined (and probably parameter-less). That would make statements lines as short as

export(ExportType) var my_var : Type = initial_value

That helps reading if you don't care about what the export does. Otherwise you'll have to scan for that type to understand it. But if don't care, you can just skip the hint entirely. Again, I feel like most of export hints will be quite short.

At the very least, we could allow a multiline export statement, because as of 3.1.2 this is invalid:

export(String,
  "Long Option Number One",
  "Long Option Number Two"
) var my_var : String

We will likely allow that anyway. Also, as I mentioned, we might allow the var to be on the next line as well, so export "feels like" an annotation if you want

How would you unpack this example from the docs, but with further hints for each inner type? For example, if that float is hinted as a range.

export(Array, Array, float) var two_dimensional = [[1.0, 2.0], [3.0, 4.0]]

Again, with typed arrays this won't be needed. Don't really have a syntax for that yet, but think something like this (borrowing from C++ templates/Java generics):

export var two_dimensional: Array<Array<float>> = [[1.0, 2.0], [3.0, 4.0]]

No hint needed at all, since it can be inferred from type.

And though Dictionarys may not be typed in the language at the moment, they may require some sort of hinting in export statements. I know of two use-cases for export statements: serialization (for resources) and Inspector Panel (for resources, scenes and nodes). In both cases, it is useful to have a say at what types of values are allowed in a Dictionary typed field. So this kind of hint may be introduced down the road, and I don't see yet how the draft is covering nesting structures to make such declarations simpler.

AFAIK there's no editor support for typed dictionaries in the Inspector. No reason to consider as a language feature without editor support. Typing dictionaries is a bit more challenging if you are expecting a specific structure (I don't think this will ever happen). What might happen is setting one type for the keys and one for the values. Then again, no export hint would be needed since the variable can be typed.

I don't see, how am I supposed to figure out from code that there is an option or how auto-completion can work in cases where we start to introduce new modifiers in between of old arguments:

export(String, FILE, "*.txt") var f
export(String, FILE, GLOBAL, "*.png") var tool_image

Those modifiers will be removed. That's a main point of the simplification. Instead you'll do:

export(File, "*.txt") var f
export(GlobalFile, "*.png") var tool_image

What if there is a need for another flag like GLOBAL? Does order matter? Will this magical list of arguments just expand with an unlimited amount of permutations? And so, it's on a developer to check the docs and find that the options are there in the first place, how to arrange and use them.

So there won't be "magical" arguments. There's only the property hint and the hint string with the specification when needed, the same way it happens internally. The point of the modification in the draft is exactly to get rid of those.

Well, I suggested a way to replace a string of arbitrary hint arguments with a struct. So something like

export(String, FILE, GLOBAL, "*.png,*.jpg,*.bmp,*.gif") var tool_image

can turn into

export_type ToolImagePath {
  extends: File,
  is_global: true,
  extensions: "*.png,*.jpg,*.bmp,*.gif"
}
export(ToolImagePath) var tool_image

I don't think that's really super easier to read. As I mentioned above, if I want to know what the export does I need to scan the source to find it. If the export struct is right before the declaration than it makes the important part (as you said, the var declaration) further below.

Note that the new system will get rid of the type and special arguments. In the end it'll be like this:

export(GlobalFile, "*.png,*.jpg,*.bmp,*.gif") var tool_image

Only two arguments: hint and hint string. It's not that long. If we allow a line break:

export(GlobalFile, "*.png", "*.jpg", "*.bmp", "*.gif")
var tool_image

This is explicit, non-ambiguous and can be code-completed the same way that the list of properties is after a "dot". There is no need to introduce new statements, if multiline exports are allowed:

export({
  extends: File,
  is_global: true,
  extensions: "*.png,*.jpg,*.bmp,*.gif"
}) var tool_image

In this case, we have to understand a whole load of different structs, making the parser way more complicated. Honestly, how difficult is to understand when reading what kind of stuff this export needs:

export(GlobalFile, "*.png,*.jpg,*.bmp,*.gif") var tool_image

You know it's a global file, and should be easy to see that the string means the possible extensions.

Now see the struct example:

export({
  extends: File,
  is_global: true,
  extensions: "*.png,*.jpg,*.bmp,*.gif"
}) var tool_image

You have to scan way more text, cluttering the actual declaration. It's harder to tell if it's a global file, because you have to read the is_global element.

Think about how completion would work with a struct. The system will have to understand the struct (which will be in the middle of writing), it needs to see the extends key and get its value. From them, it needs to suggest all the possible keys to the struct and don't suggest a repeated one, so it needs to keep track of a lot of things.

Then for parsing this we would need to create a special struct for each kind of hint, and validate them all to make sure they are correctly filled. Then we need to create the PropertyInfo internally based on that struct. If a new PropertyInfo is created, we need to devise a new struct and update the GDScript parser before it's available for scripting.

If besides that we allow a shorthand syntax from some cases, then we need to add still more effort to the parser to validate those.

@vnen
Copy link
Member

vnen commented Jan 24, 2020

Honestly, I understand how the current syntax can be seem as magical and harder to understand the available options. But the draft I shared vastly simplifies that and remove all the magic that is. No more special arguments: only the hint and hint string.

Instead of:

export(float, EXP, 100, 1000, 20) var l

You'll use:

export(ExpRange, 100, 1000, 20) var l: float

Instead of:

export(Color, RGB) var col

You'll use:

export(ColorNoAlpha) var col

If you don't need NoAlpha you can simply type the var:

export var col: Color

Same thing for other possible hints.

@YuriSizov
Copy link
Contributor Author

@vnen Thanks for taking your time to reply in detail. I agree, that simplifications of the draft can make some existing cases easier to write and read. Plus, if multiline statements are supported, that can improve comfort even further. I just think in terms of extensibility, but I guess there is no need to overthink it for future proofing.

Going to have to wait for that draft to be formalized and put to public discussion, then.

@vnen
Copy link
Member

vnen commented May 15, 2020

BTW, I intend to use annotations for the export syntax, and I do believe it's much more clear, even than my suggestion here: #828 (comment)

@Calinou
Copy link
Member

Calinou commented Sep 13, 2020

The export syntax has received an overhaul as part of the new GDScript, closing.

@Calinou Calinou closed this as completed Sep 13, 2020
@Calinou Calinou added this to the 4.0 milestone Sep 13, 2020
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

6 participants