Godot: Export variable triggers setter

[razcore-art]

Operating system or device - Godot version:
Ubuntu 16.10, Godot v2.1.official.stable

export var a = 10 setget set_a triggers setter, but var a = 10 setget doesn't. This might be logical from the implementation point of view but from a developer point of view it doesn't. An example why I don't like this. I'm trying to keep track of the player health in some singleton or with Globals when changing scenes so I do:

export var hp = 100 setget set_hp

func _ready():
    if Globals.has('player_hp'):
        self.hp = Globals.get('player_hp')

func set_hp(x):
    Globals.set('player_hp', x)
    hp = x

This doesn't work when using export like in the above example because the setter gets triggered when the node enters the scene so the if in _ready will always get triggered and resets the player hp to the default value. This is unwanted and if I'm not using export it works as expected.

So there's an inconsistency on how the setter is triggered between using and not using export.

[vnen]

There's no "inconsistency" per se, it just works the way it's intended: an external object changing the property will trigger the setter, in this case the scene being constructed sets the value.

I'm don't think the export should avoid calling the setter. I expect it to call and used it like this in the past.

I guess both usages are valid and there could be a way to make it optional.

[razcore-art]

True, I get why it is this way, but I don't agree with it. What's the purpose of having the setter trigger specifically when using export? The value from the inspector should really be viewed just as being the default value of the variables, just like when you use var a = 10 somewhere. If you want to trigger the setter it can be done by doing:

export(int) var a setget set_a

func _ready():
    self.a = 5

func set_a():
    # setter

which would work the same as not using export.

I mean let's put it this way. If I don't use export I can still trigger the setter manually, but if I use it there's no way not to trigger it, it's being forced on me so I think the default way of doing it would be not to trigger since we have the ability to trigger it ourselves

[Zylann]

I would have done your Globals thing differently, which is to not update it in the setter directly because most of the time you don't care. In fact, you only care when you change the scene, so it gets remembered at the moment it's switching, otherwise I would completely ignore the fact this singleton exists.

That said, I'm a bit confused too by the way export works on scene loading here. I would expect that the values saved in a TSCN would be an actual dump of the state to be restored later, not something seen as "external" properties set one by one.
Another example, if your class has 20 properties that all update calculations when used by code (and some eventually being the same), triggering the setters for every property sounds very ineficient compared to what a single update could do in the constructor, after the values are initialized by the scene loader. You could design your class not to do logic in the setters of course, that's fine. Then you loose the advantage of properties.

One workaround I would use here (and I used it in my Terrain Plugin, actually), is to do this:

export(int) terrain_size = 0 setget set_terrain_size
export terrain_data = [] # This is hidden through flags because it's pointless to edit in the inspector

# Simplified example, but the real-world one roughly works the same
func set_terrain_size(new_size):
    if new_size != terrain_size:
        # Size changed, I should add 256x256 more terrain data!
        # But... not if that has been set by the scene loader. In that case,
        # terrain data will be set later in the process anyways!
        # But it isn't right now... we'll check in _ready(),
        # so we'll not update the data twice.
        if is_inside_tree():
            _on_terrain_size_changed(new_size)

[razcore-art]

@Zylann yeah, exactly, the way export works with setget isn't intuitive at all. I would have expected to not be any difference between using it and not using it. It should just export the variables to the inspector without triggering anything.

But what you're saying yeah... I agree, it also seems like wasted cycles if it's triggered for every property like that.

[Zylann]

I think part of this problem also comes from the fact export is mixing two things together:

• Make a variable editable
• Make it serialized in the scene

What you edit is not always exactly the same as what you save.

Separating them is a way to get what you want. One meant to be edited, another meant to represent the serialized state. The editable one can be based on the other one.

[Sslaxx]

Honestly, I'd rather see the syntax made more explicit, i.e. something like:

export var a set set_a, get get_a

Requiring one or the other or both to be defined.

[Zylann]

@Sslaxx that's not really part of the problem, it's more syntax but the behaviour is still the same.

[razcore-art]

@Sslaxx make a separate issue for that :P, I personally prefer the current style using setget

[Warlaan]

I kind of agree, but I think the issue is not that the setter is invoked when changing the value in the inspector. I think the issue is that the getter is not invoked for the default value in the inspector.

I mean let's look at a typical use case for a setter / getter. Let's say you have a variable in your object that represents some size in meters. All the other values are given in meters, so it's only natural to use meters in that spot as well. But later you notice that it would make more sense to calculate in millimeters internally. So you want to change the value of your variable, but you don't want to change the values external code sees of your variable, and that includes the editor, since an inspector value of 1 should still stand for 1 meter, not for 0.001 meters.
That's why I think in order to fix the unintuitive part you would have to change it so that the inspector doesn't show the raw value but the result of calling the get-method for the value. Right now if you write

export(int) size = 1000 setget set_size_in_meters, get_size_in_meters

func set_size_in_meters(value):
    size = value * 1000

func get_size_in_meters():
    return size * 0.001

the inspector shows a starting value of 1000, which would be the internal default value, but since that value is set at the start of the game using the setter it would in turn lead to an internal value of 1000000.

[razcore-art]

@warlaan, it's a valid point... So we should probably agree to either use both setters&getters in the inspector, or none at all, cause otherwise it wouldn't be consistent...

[Zylann]

I like the fact the inspector is able to call setters. It's incredibly handy. Why would we remove that feature?

[Warlaan]

I agree that it's a good thing that the setter is called, but I also see the inconsistency razvanc-r complained about, since the getter is not used for the value in the inspector.

For me personally it's a minor issue since the inspector value is set just once in the beginning, but I could imagine that it might be a problem in some situations.

[Warlaan]

@razvanc-r Maybe you should change the name of the issue to "export triggers setter, doesn't trigger getter" or something to describe the issue better.

[Zylann]

Hmm it looks like two issues were discussed about here:
1) Getter not being used by the inspector
2) Setter being called by the scene loader (nothing to do with inspector)

[bojidar-bg]

Does the issue persist when you just... remove the default value?

[ghost]

First of all thank you for your report and sorry for the delay.

We released Godot 3.0 in January 2018 after 18 months of work, fixing many old issues either directly, or by obsoleting/replacing the features they were referring to.

We still have hundreds of issues whose relevance/reproducibility needs to be checked against the current stable version, and that's where you can help us.
Could you check if the issue that you described initially is still relevant/reproducible in Godot 3.0 or any newer version, and comment about it here?

For bug reports, please also make sure that the issue contains detailed steps to reproduce the bug and, if possible, a zipped project that can be used to reproduce it right away. This greatly speeds up debugging and bugfixing tasks for our contributors.

Our Bugsquad will review this issue more in-depth in 15 days, and potentially close it if its relevance could not be confirmed.

Thanks in advance.

Note: This message is being copy-pasted to many "stale" issues (90+ days without activity). It might happen that it is not meaningful for this specific issue or appears oblivious of the issue's context, if so please comment to notify the Bugsquad about it.

[jwittenbach]

@Noshyaar I can confirm that the issue of the setter for exported vars being called on scene load persists in 3.0.

Adding to the discussion, I agree that this is less than ideal. Aside from performance issues, it can make using tool tricky -- e.g. when the setter references a child node that has not yet entered the tree at the time when scene loading causes the setter to be called on the parent.

[Zireael07]

I can also confirm that setter is called on scene load in 3.0.2.

[vnen]

when the setter references a child node that has not yet entered the tree

Well, TBH there's no guarantee that the setter will be called only after everything is in the tree. Most of the time you have control over your code and can be sure it's not called before _ready(), but inherently it's not a requirement.

In any case, I see there's a problem but I have no idea what the solution would be. The thing I said back then still stands: an external entity setting a script variable will trigger the setter. To avoid that we would have to make special tweaks in the scene loader and script API to go around it. I still think that triggering the setter is the natural way and working around it is much simpler then the other way: if you wanted the loader to use the setter, it would be quite hard to force it to be used.

Now if the Inspector could use the getter, it would solve the inconsistency. For that it would need to keep a dummy instance of the script, but would bring all the state (such as the _init() function being called) which is probably not wanted. And if you're relying on the tree in your getter, then it wouldn't solve the problem anyway.

[Zylann]

As I said above, a big issue about deserialization triggering all setters one by one in an unpredictable order is that it may trigger operations quite expensively. The scene loader is not just calling setters of an entity externally, it triggers ALL of them, and for a particular reason: building the object. The whole thing is a workaround for parameterless constructors (and I'm not even talking about setters calling order!).
To me a better hypothetic alternative to such a system is to set values directly without triggering setters, and then call a function to notify the end of deserialization (which internally is just asking the object to be built by providing all initialization params at once in a single call). After which, setters will be able to work normally (is that what _init does though? It would be, if it was called after the setters).

Note: this also applies to resources, which don't have _ready or _enter_tree so workarounds for them are more limited.

[jwittenbach]

@vnen I did find a work-around for my particular problem. In the setter, I can check whether or not the children exist -- if they do not, then I know the setter is being called because the scene is being loaded, and they have not entered the tree yet. It's a hack, but it works. This seems to be a reliable indicator.

I agree with @Zylann that it feels unnatural to consider object initialization as an 'external source" modifying the variable -- the variable doesn't even exist yet! I recognize that it's a subjective matter, but initialization doesn't feel like modification to me, so I was very surprised to see the setter being called in this case. That said, I admit to having no idea how hard it would be to change something like that in the engine.

[vnen]

Well, I was mostly describing how it is, not what the ideal situation would be, just so we could think of a solution. Haven't looked at the code, but I'm pretty sure it simply calls Object::set() and the subtypes decide where this call goes, which means setter functions in GDScript.

The solution is probably to provide an alternative to set() that is called on initialization, so subtypes can decide to avoid the setter in this case. For GDScript, it would have to happen after _init(), because that's called right away when the instance is created, so another virtual function would be needed to notify when everything was set.

[kakoeimon]

After reading the comments here I believe that the actual problem is that the behavor (or limitations?) of the exported values are not documented.
The same is true about how the inspector is going to get the values from vars that use setget.
Slightly out of topic, but valuable for tool creators, that to change the values of var inside the editor with gdscript the var must be exported.

[Calinou]

Note that in Godot 4.0, internal variable usage will also call the setter/getter.

---

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!