-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Properties in GDScript #844
Comments
I should disagree. This is absolutely not a problem. It allows keep code in separate blocks: first variables and then functions. Which is great as it keeps code clean and as readable as possible. |
Note that I mean specifically the getters and setters. If you see a With the syntax proposed here, I would split the blocks as: variables, properties, then functions. |
Why not use annotations for that ? I think I would prefer something like: @getter(get_myproperty)
@setter(set_myproperty)
var myproperty |
I mentioned annotation as an alternative, but it doesn't solve points (3) and (4). It also would make difficult to solve (5) since the setter/getter needs to access the variable without the setter/getter, but if it's a regular function then the conditions for direct access get more complex. |
For me, (3) is not really a problem as in most cases, the setter and getter will have very name very close to the variable name, so I don't think it is that confusing. For point (4), I am not sure there is way to solve this problem, unless by adding more explicit keywords maybe, but that would be weird. Regarding point (5), I think we can call the getter/setter using |
Yes, it is possible to use I've dealt with many Godot beginners who were baffled by this behavior and found it confusing. So no matter what syntax we pick, I believe (5) must be resolved. |
For annotation or other syntax, it's important to keep in mind the whole context when evaluating. Of course two/three lines of notation may seem cleaner but you need to take in account that those are defined somewhere. So an annotation syntax would look more like this: var milliseconds: int = 0
@getter(get_seconds)
@setter(set_seconds)
var seconds: int
# [...] a bunch of other code here
func get_seconds():
return milliseconds / 1000
func set_seconds(value):
milliseconds = value * 1000 Which in my view requires more boilerplate code than it needs, by exposing two functions that most likely won't be used, because why would you need to use them if using the property is simpler? A similar happen with the engine native getters and setters when properties were introduced (though they still work, they're not exposed in the documentation). This example code also can illustrate the issue (5). Anywhere in the class if you use And you also now have a member variable that you don't ever use (which could be stripped out by a property syntax, recognizing it's not used). |
If that is a read-only property then how to define a variable with custom getter but not setter, that allows other scripts to do a direct assignment like it is currently possible?
|
Why have |
if setget is seperated into
it would also be nice to beable to switch order of set and get
|
As for me I always name argument in set functions in different ways. Not always
Yes, I agree with it. |
Because
What C# does should not matter for GDScript design given it was added for people who didn't want to learn/use GDScript. I'm for explicit value since that's what Python does. |
That's something to discuss. We can either make it read-only if there's no setter, or use direct assignment (then use a @readonly annotation for this).
Because I despise implicit identifiers in scope. Say your class has a property named var value:
set:
self.value = value In this case, Also, naming it something else can make your code clearer to read.
I think this saying is overused. But I also think that removing magic is better for understanding.
This is a different idea. In my proposal you won't need to define the functions elsewhere, so this is notation is not really useful. I'm not "splitting If we went that route I'd prefer the annotation syntax. Otherwise it wouldn't solve point (2) in my original post.
As said, this notation won't work, but in my proposal order doesn't matter. |
@vnen I really don't know about this.
If this has to do with people calling the getter and setter functions then I think some sort of equivalent of the private keyword should be used. |
With this change, if I want to directly set a variable inside the class that defined the variable (without calling its setter function), would I still be able to do that? If not, it would cause a stack overflow in situations where 2 variables have setters which set the values of each other. For example, say I have a turn-based game.
If not, maybe a new function/annotation or something would be needed, like: |
Simple, you disallow having a property named |
I agree they shouldn't be used for everything (and they aren't) but in this case I find much more confusing and less readable to have all in the same line. Longer lines are always harder to read. And annotations can go in the same line if you so prefer.
Well, with my proposal it's not impossible to do it: var my_prop:
get: return get_my_prop()
set(value): set_my_prop(value) The only thing is that in this you would need a backing field for use inside the setter and getter, to avoid an infinite loop. So, I guess we can provide a special syntax if you still want a separate function: var my_prop:
get = get_my_prop, set = set_my_prop Order doesn't matter, and both are optional. With this we can detect the pattern and allow direct access inside the functions (though I still don't like providing direct access far from the declaration, but it's a compromise I could accept). |
No. That's the problem (5) that I mentioned in the original post and it is a problem I intend to fix. If you need direct access in this case, you can use a backing field.
I guess we could have a way to edit the field directly, but I'm not convinced yet that it's really needed. |
That is a super bad name to disallow. Is very unlikely you'll name something What we could do, to settle on a compromise, is make the setter parameter optional and assume it's |
I'd really prefer it if you don't introduce two ways of doing the same thing unless really necessary, if they want the C#-ite semantics just use C#, being explicit is good and building in exceptions already seems like a poor idea. Since it functionally fulfills everyones requirements with the original (explicit, simple) proposal and people can just call their function in the setter/getter if they want I don't see the problem. (i think $ in gdscript already confuses people enough though, so i may be extreme) ... Just my two cents anyways 👀 |
@dalexeev honestly I would remove that from the docs. I would even make those methods inaccessible from scripting. They are not needed, exactly because you can simply set those as properties. I don't see one reason to use the method instead of the property. |
Non-obvious use cases are possible. For example, using var action = funcref(npc, "set_position") |
Well, you can create your function if you really need this. Especially when lambdas are added it will likely be simple to do it in the same line. |
(5) isn't an issue and should not be touched, instead, tutorials should notify that
Yeah, you have to be conscious of what you're typing, there are two ways of accessing, and you should know which one of those two you want to use. People need to understand that Besides that, I like this change 👍, easy to parse and looks like a very useful shortcut for typing
I totally agree with this, there's no point in separating variables from it's |
@vnen
should be..
also i like the idea of doing something like...
or something similar. |
With all the respect, this is a very bad idea, click on this comment it already has 6 dislikes. There are three main reasons for this reaction:
|
In order to expand on "C# does it" I did a quick research (please correct me where i'm wrong) about setters in other languages. I've checked languages that were considered to use for scripting in Godot plus Haxe. My results: Explicit setter's value: Lua, Python, Squirrel, JavaScript, ActionScript, Haxe Personally i consider implicitness of setter's value a small issue which does not justify reserving a word. |
implicit is better for this case because again there is only one thing your passing a value. |
This proposal basically works the same as ES6 getter/setters, except in Javascript there is no such thing as "direct access" to the property (inside of the getter/setter function, or anything, like the current proposal), essentially if a property is defined as a getter/setter, it can no longer contain a value. The property is an alias to two functions at that point. I've worked with Javascript for quite a while now, and have never been a fan of having a 2nd backing variable unless the getter/setter is truly a combination of multiple existing variables, as opposed to a way to run some extra logic before setting a variable's value. This discussion isn't novel or a problem unique to GDScript. Essentially it becomes a problem of coming up with a different variable name for your internal data vs external interface, and a lot of times you want that name to be the same. Finding alternate names that make sense for variables when you could've had the option to just reuse the same name would save a lot of time. The alternative, like suggested, is just use So basically, I've been dealing with this issue for a long time now in other languages, and my solution is to come up with an alternate name for any variable that needs a different external interface. That's a lot of extra cognitive load to writing a program that really doesn't need to be there if the language just supports direct access from inside of the class. |
I would argue that many properties don't need direct access to a backing variable (and also most members in general can just be fields with no getter/setter logic), so no, you don't need this for literally every property. Also, even if you have a backing variable, you shouldn't use it from outside the script it's declared in, because the point is for the property to be the public interface. Therefore it's not important to memorize which properties have backing variables. |
As I said, in this proposal you do have direct access inside the getter/setter. So you don't need a backing field unless you need direct access somewhere else. |
Yeah, that is exactly what I was stating here:
The purpose of that sentence was to point out how this is different from Javascript. I understand how it works.
You misunderstand me bro. Nowhere did I say the backing variable would be used outside of the class. I'm talking about organization of the class itself. If I had a variable "x" that needed backing but "y" did not, it makes no sense to be calling "_x" then "y" in the code to set both. This kind of thing would be an anti-pattern and a naming convention nightmare. Basically, this backing variable idea is dooming every variable used internally in the class to be prefixed with |
Basically, if I'm going to prefix variables with |
The |
Sure, but you're rewriting the language, and what happens when #641 goes in? |
GDScript doesn't have access modifiers. That one is a community proposal we don't know yet if that's gonna be done or not. I still prefer a difference between |
It's just weird, man. Because at least Javascript has a set design philosophy, you cannot store a value in a getter/setter variable, it must be stored in a second variable. Whereas the GDScript proposal does allow you to store the value with the getter/setter, but only in certain use cases. Honestly if it worked like Javascript where you couldn't store the value and were forced to use a second variable all of the time I wouldn't have as much a problem with it, right now I'm getting mixed messages from this design. |
I agree properties should only be getters and setters they should not hold data of any kind just refrence it |
@vnen if the "default backing field", the one with the same name as the property, is never accessed in the setter*, is it planned to avoid initializing it. So there is no unused, potentially large, object allocated? * (or in the getter, I guess, but accessing it only there would be a bug, I'd say, and should maybe be flagged with a warning.) |
Yes, the space is allocated only if you use the variable. |
Its the same for python, C#. I think you really can't solve problem 5 while having these weird "maybe" generated variables. Because you would just invert the confusion as long as there are ways to use This is the only real "consistency" that you can have. I strongly disagree when @vnen calls it consistent behaviour when its sometimes direct access and sometimes property access, even if its "consistently" sometimes this and sometimes that. You still have to remember this which is quite hard if you had exposure to properties in any other programming language. I think just explaining these two different variants to anyone who has never heard anything about properties is a nightmare, more so than explaining the direct access case to someone who is "only" familiar with properties in every other language. Further, when I had just stumbled upon properties in GDScript while reading code and seeing both ways of doing it -- assuming that I would actually be able to infer the behaviour -- I would probably have assumed that the way with backing variables is a hacky misuse and that there is now an allocated but unused variable. I would probably refrain from using it with backing variables because I don't like hacky misuses. I think this is why all the other languages force @Giwayume to come up with different internal/external names. Its just way simpler to understand and keep track of. And if you at one point really want to change just the variable directly just use The first syntax (in the first two blocks of @vnen 's initial post) is the only one that makes sense to me in this context. But my taste is that it should look more like a function, because it is syntax sugar for functions. So I would prefer dead simple:
But this is just my personal taste. Edit: Forget my taste. This syntax would allow you to do weird stuff like
I don't know if its necessary to allow this. Probably just @vnen ' s proposal minus the awkward generated variables (;-)) is the better solution. |
After godotengine/godot#40598 is |
@Shatur95 In your particular case, I see that you have both |
@Shatur95 The proposed solution here is this:
I think their personal coding problem is off topic here, Though I realize this thread is very long already. |
Thanks the tip about backing field. The properties syntax should improve the readability of the code, but for me personally, the option with a backing field looks less readable. Off topic
Oh, yes, thanks. I just forgot to add |
We discussed this in a meeting with the core devs and since this is already implemented with godotengine/godot#40598, then it can be closed. Note that the implementation took in consideration the discussion here, by finding a compromise with differing opinions. Discussion can still happen here. Any idea for a big change in the syntax/behavior should be made as a new proposal. |
Is there docs somewhere for how it was implemented? Before I get into concerns I want to say that I do like the direction this feature is going. I see some potential confusion/issues with initialization. In most languages with properties, the backing field and property have separate initializers so you can choose whether to go through the setter or not. Example: var _foo: int = 0 # < Initializing backing field. Setter not called
var foo: int:
... #property stuff
var _foo: int
var foo: int = 0: # < Initializing property. Setter is called
... #property stuff Usually, having the backing field and property separate don't matter. With gdscript, however, we have The second use-case from above is fine, but the first use case from above gets confusing. var _foo: int = 0 # < Initializing backing field. Setter not called... the first time
@export var foo: int: # < Can't tell default val. Val always saved, thus setter always runs after first save.
... #property stuff I'm not sure if there are other implicit issues with breaking the tie. Also, I know this isn't a super big issue. I just could see folk's assumptions about other languages leading to confusion as to why the setter is getting called, which leads to issues being opened and stuff. Getting ahead of it by making sure the docs are good would be... good. |
Are there any examples of how to use this? Doesn't look the docs have been updated. |
Okay I figured it out. I was trying to get it to work using export annotation:
|
Describe the project you are working on:
The GDScript compiler.
Describe the problem or limitation you are having in your project:
The current way to define setters and getters are to use the
setget
syntax and name the functions to use as setter and getter.There are a few problems with such syntax:
var x setget ,get_x
export
and `onready will move to annotations which improves this but not completely).set_x()
function or just set thex
field with the automatic setter.Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Implement a syntax for actual properties instead of
setget
.Moving
setget
to annotations would solve (2) but not the others (although (5) is independent of syntax, we could fix it while keeping the keyword).With properties we can keep a cohese block with variable declaration together with setter and getter, without exposing a extra functions.
Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
This is one possible way of declaring a property:
So the getter and setter are included in the variable declaration as extra blocks, so no function declaration needed. Type specifiers are not needed also since the variable is typed and the setter/getter will enforce the same type.
Alternatively, we could use a new
property
keyword, but I don't feel it's needed. It's visually clear that the blocks belong to the variable.You can also set only the getter for a read-only property:
To avoid creating external variables for this, you can use a generated member variable inside the setter and getter:
Edit: I think the proposal I made in #844 (comment) could be a compromise for people who still wants actual functions as setter/getter:
So, I guess we can provide a special syntax if you still want a separate function:
If this enhancement will not be used often, can it be worked around with a few lines of script?:
It will be used often and the current way makes this more complex to write and maintain.
Is there a reason why this should be core and not an add-on in the asset library?:
It cannot be implemented as an addon.
The text was updated successfully, but these errors were encountered: