Setter function not called for default value

[RebelliousX]

Tested versions

Reproducible in 4.3 Dev 1

System information

Irrelevant

Issue description

Exported variable with a setter function and a default value for that variable is never called.

If the value changed from the editor, the setter will be called.

extends Node2D

@export_enum("Default", "Hello", "World") var string: String = "Default":
	set(new_value):
		string = new_value
		print(string)

Steps to reproduce

• create a scene and attach a script to it
• use the script above for example
• save the scene, lets say as string.tscn
• on the main scene, instantiate string.tscn three times
• Name the first Default, the second Hello and the third World
• leave the first Default untouched, the second change the exported variable
  from the inspector to the value Hello and the third scene to the value World.
• run the program, only Hello and World are printed. Default not printed.

Minimal reproduction project (MRP)

Godot - Export Setter.zip

[RebelliousX]

This bug, costs me an hour or two to figure what is going on with my large project with hundreds of nodes. Some their default values changed and some are not. These nodes had custom hand made collision detection.. To my surprise, the nodes with the default value were never called and weird monkey behavior was happening.

For now as a workaround for this, I had to reassign the variable to itself in the _ready() function, like:

func _ready() -> void:
	string = string

This will force call the setter.
But the problem would be that the setter function for the variables with non-default values set will be called twice. The output will be

Hello
World
Default
Hello
World

[dalexeev]

To me this is expected behavior: you set a default value, so you know it's happening and what value you're assigning. As far as I remember, I relied on this behavior in my scripts, and the alternative behavior would create a problem that cannot be worked around. While your problem is quite easy to work around by assigning a value in _init() or in _ready().

This is as logical as the fact that the setter is not called inside the setter. The initializer expression is "inside" the variable definition, so writing occurs directly, bypassing the setter.

When we implemented static variables, we initially introduced a bug with an unnecessary call to the initializer expression (see #77098 p.1), but then fixed it to be consistent with non-static variables. This also indirectly confirms that the current behavior is correct.

Also the issue raises a question whether the setter for the implicit default value should be called:

var a
var b = null
var c: int
var d: int = 0
var e: int = 1

The compiler assigns variable values in multiple passes, and calling a setter on default values would be redundant in most cases in my opinion.

• See also GDScript: Document variable initialization order godot-docs#7507.

[RebelliousX]

No, this is not the expected behavior, you just got used to it being wrong. And with Godot 4.0 release and the new setter functionality changes, any assignment to variable should call the setter. Before in Godot 3, we used to do self.variable = value if we wanted to call the setter, (from same script ) but according to 4 release, they are called from anywhere including same script.

The workaround I mentioned is not optimal. I do lots of nodes instantiation and destruction in the setter functions. Making an assignment in the ready function will cause double calls to the setter functions whose variables are not set to default. In my case I get measurable performance hit on startup.

The behavior should be consistent, any assignment should call the setter function. Nobody likes hidden surprises.

[Mickeon]

I'm partially used to it, but even with my experience I cannot possibly agree that this behavior is "expected" from an user perspective. There are some pros to it, but a lot of times it needs to be worked around just as described above. I feel like there is a flaw here.

[RebelliousX]

#62739, #83078, #83375, #81972, #54595, #36496

And possibly more.. all complain about the same thing, not expected behavior. Mysteriously, all were closed without proper solution to the issue. I didn't see those, since I was searching "opened" issues only. Clearly, this is not the "expected" behavior. And if this is working as intended, then this intention is completely wrong and inconsistent.

[dalexeev]

Clearly, this is not the "expected" behavior. And if this is working as intended, then this intention is completely wrong and inconsistent.

In programming languages, many things are unintuitive and non-obvious, so they require reading documentation and/or learning the ropes. Solutions that seem appropriate in some situations may be completely unacceptable in other situations.

Different people expect different behavior, different programming languages implement the same things differently. When making decisions, we should look at a problem from many angles, rather than focusing on one particular case. Universal solutions are not always equally good at everything, it is normal that in some cases you have to deal with "unnecessary" code.

We also strive not to break compatibility without good reason. This behavior is not an obvious bug, so we must be especially careful about the consequences.

I suggest first comparing the current behavior with other programming languages, the behavior of setters in 3.x, the general behavior of the engine in similar situations (are setters/callbacks called, are signals emitted for default values). It's also worth considering whether there's a different way to solve your problem without changing the setters' behavior: maybe we're dealing with XY problem and the real problem isn't the setters' behavior.

Please let's be constructive to find the best solution. Of course, this will take some time, we shouldn't rush. Thank you for your patience.

[RebelliousX]

@dalexeev Fair enough. no, thank you for your patience. Okay, let's see what we can do about this.

• Currently the only viable solution is to do this in the _ready(), to only call setter if the value is the default as follows:

	if (variable == default_value):
		variable = default_value

It works. but it really doesn't make any sense, just by reading it. Albeit the order of the initialization for the variables will be different (some at _ready(), and others at _init()). In most programming languages like C++ self assignment IIRC doesn't invoke any constructors. Or at least, many years ago, some books I read about C++ they were striving to emphasize and prevent self-assigning the variable to itself with the same value.

We can't use this in the _init() function since the result will be the same as the next solution. (double setter function calls)

• If we tried a different solution, by not using default values and to set the value in the editor. Save as detached scene and instantiate it on different scene like the steps showed in the first post.

extends Node2D

@export_enum("Default", "Hello", "World") var string: String:
	set(new_value):
		string = new_value
		print(string)

The output will be:

Default
Default
Hello
Default
World

Which is worse that the first solution. I thought this would work since we are not initializing the variable the moment it was created. I was (hoping or) relying on the editor to change the value and call the setter that way. But no.

Now _init() initializes all instances of that node to the default whether that node has a default value or a modified value. If it has a different value in the inspector, the setter function will be called again for the modified value. So basically, we get double calls, once for default, another for the modified value.

[dalexeev]

Sorry, I didn't read the issue carefully enough. What exactly do you think is the problem: 1. the behavior of setters (affects all variables) or 2. the fact that the default values of exported variables are not saved to the scene file and are not assigned, so of course the setter is not called (affects only properties with PROPERTY_USAGE_STORAGE)? The last one is the core area, not GDScript.

For inherited scenes there is Pin Value option. I haven't tested this, but it's likely that in this case the default value will still be assigned, which will call the setter.

Default property values are not saved in scene files, this has both pros and cons. Pros: reduced file size, faster initialization, smaller diffs in VCS, more convenient if you want to change defaults. Cons: less reliable if you want to freeze the value in case the default value changes, inconsistent initialization (but if you look at it the other way, there's no point in assigning the same value). Perhaps non-inherited scenes should support Pin Value as well.

[KoBeWi]

If a value is saved in the scene, the setter will be called regardless if it's default or not. Currently saving default is possible using a) property pinning b) _property_can_revert() (I think?) and c) bug. For me c) caused lots of problems, because the setters were called inconsistently, but all known cases so far were fixed.

I think the simplest workaround for your problem is calling the setter manually only if the value is default, i.e.

func _ready():
	if string == "Default":
		string = string

Other solution is setting up the object so that the setter does not need to be called for the default value, as already mentioned above.

While the current behavior can't be changed, we could introduce a new @export annotation for always calling the setter. That needs a proposal though.

[RebelliousX]

@dalexeev Thanks for mentioning the Pin value. Doing so indeed calls the setter function. But when I save the scene as detached and instantiate it from another scene, I get the same output as the 2nd solution I mentioned:

Default
Default
Hello
Default
World

Which is weird, I have 3 nodes, 3 print statements and I get 5 print statements..
My guess is 3 x initialized with "Default" when the object is created, and the two working "Hello" and "World", but a sixth "Default" is missing here. But this is worse. I can't think of any use for Pin Value.

What exactly do you think is the problem: 1. the behavior of setters (affects all variables) or 
2. the fact that the default values of exported variables are not saved to the scene file and are not assigned

The first,, but my specific problem is with the @export. But about number 2, in the 2nd bad solution attempt, I thought that the initializing of the variable without a default value and thus calling the setter will happen at a "later stage" if I set it through the editor, not the code when using export annotation. Which doesn't seem the case.

@KoBeWi I already figured that solution might the simplest choice for now, but it really is not a good solution in my opinion. And not so obvious one too.

I am sure, like me and others from the list of reported issues I found and mentioned above, we will get others opening new issues about this again.

If anything, I would like to keep this issue open for anyone in the future that might stumble with the same issue. Github doesn't make it easy when you search for issues, by default, you only search for opened issues, not closed ones. And all issues related to this are closed.

[dalexeev]

What exactly do you think is the problem: 1. the behavior of setters (affects all variables) or 
2. the fact that the default values of exported variables are not saved to the scene file and are not assigned

The first.

Sorry, I still think this is a bad idea for the reasons I mentioned above (especially compatibility). In my opinion, the initializer expression is intended to be written directly to the variable, just like assignments in the body of the setter. As a workaround, you can assign a value in _init() or _ready() instead of the initializer, in which case the setter will be called.

Details

extends Node

var a = 1:
    set(value):
        prints("a", value)

var b:
    set(value):
        prints("b", value)

@onready var c = 3:
    set(value):
        prints("c", value)

var d:
    set(value):
        prints("d", value)

func _init():
    b = 2

func _ready():
    d = 4

Prints:

b 2
d 4

These callbacks may not be appropriate if child nodes haven't been added yet (see also Early initialization of instantiated scene), but what you suggest (the first, GDScript side) still won't solve the problem, since the setter call would still occur at the wrong time. You can achieve the same behavior without introducing incompatible changes, as shown above. In my opinion, if there is a problem, it is on the core side, not GDScript.

[dalexeev]

Note that the behavior of setters in GDScript is already documented:

[hunterloftis]

This was surprising behavior to me as well. In C#, my workaround has been to assign default values and simultaneously export the property, which seems to get Godot to consistently call its setter.

[AThousandShips]

Please see the explanations above, this is consistent with how data is processed, it might be unexpected to those who are not familiar with the way this works, but it's consistent, it also follows the logic of doing nothing unnecessarily

As I see it, proper design of a class accounts for default values and ensures it works correctly with those default values if nothing is changed, essentially, if you're writing good code the class should always work when just constructed, without providing any explicit values, that way things are as efficient and as robust as possible, that way you don't get bugs if something fails to be assigned its default value, relying on everything being assigned is unsafe and inefficient