GDScript: Appending to an array or removing elements doesn't trigger setter

[ILoveGodot]

Godot version: 3.0.2

Issue description:
Calling append, insert, erase, push_*, pop_*, sort*, clear, resize or remove on an array with a setter doesn't trigger its setter. Calling clear and erase on dictionaries doesn't trigger it either. It works for dictionaries if you add new entries so it should work for arrays too.

Steps to reproduce:
-Add the scripts to nodes

Array:

extends Node

func _ready():
	var a = A.new()
	
	a.b.append(3) #Or the other methods

class A:
	var b = [1,2] setget setter
	
	func setter(value):
		print(value) #Never called

Dictionary:

extends Node

func _ready():
	var a = A.new()
	
	a.b.orange = 4 #erase() and clear() don't trigger it

class A:
	var b = {} setget setter
	
	func setter(value):
		print(value) #Prints '(orange:4)'

[ShyRed]

Actually the setter getting called for modifying a dictionary seems to be a bug?

A setter should only be called, if you set the variable to a different value / object.

If you call append on an object, you are actually calling the getter to get the object and then work on the object. The getter does not care what you do with the object. Its only job is to give you the object.
The setter is never called since you are not setting the variable to a different object. You are just modifying values on the object which is not the same as setting it.

[Warlaan]

If there's a need for functions to be called when an array or a dictionary is modified then the container classes should get signals, so you can connect a function to the change.

I agree with ShyRed, calling a setter or getter when methods of a container are called is too inconsistent with the usual meaning of setters and getters.

[ILoveGodot]

But what should happen now? Should the setter not be called anymore for dictionary modifications? Is there somebody who depends on this behavior? Should somebody open a new issue for container signals? Should I close the issue?

[ShyRed]

Would need a Godot dev / team member to get an official statement on this.

My 2 cents:

1. setter should not be called for dictionary changes anymore as this is not how it is handled in all other languages (to my knowledge)
2. Fixing the bug might break existing projects, so the fix should go into 3.1 and not into a 3.0.x release.
3. Container signals is a feature request that also needs some discussion on if and how to implement, I guess. Best is to open a new issue for this?

[bojidar-bg]

The fact that the setter is called when doing an "indexed set" on the dictionary is because of how GDScript does indexing internally.
Suppose there is a property called transform of type Transform2D.
Some code does transform.o.x = 6. (o is origin in this case)
But, Transform2D is somewhat immutable, as is Vector2, and if the code just naively sets the x of the o, it won't change anything.
So, it would do something like this:

var local_transform = transform
var local_o = transform.o
local_o.x = 6 # Those are vars, so we can change their properties
local_transform.o = local_o
transform = local_transform

Not sure how this should be fixed, since the compiler does not know anything about whether the value is shared (Dictionary, Array, Object) or unshared (Vector2, etc.) at compile time.

Here are some relevant lines in the code:

• gdscript_compiler.cpp:924
• gdscript_compiler.cpp:935
• gdscript_compiler.cpp:977

[Calinou]

Can anyone still reproduce this bug in Godot 3.2.1 or 3.2.2beta4?

Note that GDScript is being rewritten for Godot 4.0.

[bojidar-bg]

Should still be valid on 3.2.x.

[dominiks]

Confirmed on 3.2.1 and 3.2.2beta4 (and also on master).

[strank]

I just ran into this behavior with the current dev version of Godot 4 (v4.0.dev.custom_build.960a26f6c).
At the moment, mutating a dictionary, array or object property with indexed access will call the getter followed by the setter (passing in the same object, change applied). So that includes arrays! Mutating the property with any method call, such as .append or .set will only call the getter.

In a sense, OP's issue is resolved there, but I would argue that the behavior is pretty surprising and inconsistent. My preference would be that gdscript would distinguish between mutable and immutable types so it can keep the behavior that @bojidar-bg mentions but not call setters on indexed set. Or at least treat indexed setting like a method call rather than like an assignment.

In case it helps, here's a little script I wrote to check the current behavior, to be run with godot --headless -s ...:

extends SceneTree

var dict_var := {}: set = set_dict_var, get = get_dict_var
var arr_var := ["bla", ]: set = set_arr_var, get = get_arr_var
var obj_var := Node.new(): set = set_obj_var, get = get_obj_var


func test_dict():
    var initial_dict = dict_var
    prints("initial dict", initial_dict)
    print("----")
    print("setting dictionary value:")
    dict_var["test"] = "setting"
    print("----")
    prints("has dict identity changed?", initial_dict != dict_var)
    initial_dict = dict_var
    print("----")
    prints("getting dictionary value:", dict_var["test"])
    print("----")
    prints("setting the dictionary variable itself")
    dict_var = {}
    print("----")
    prints("has dict identity changed?", initial_dict != dict_var)


func test_arr():
    var initial_arr = arr_var
    prints("initial arr", initial_arr)
    print("----")
    print("setting array value:")
    arr_var[0] = "setting"
    print("----")
    prints("has arr identity changed?", initial_arr != arr_var)
    initial_arr = arr_var
    print("----")
    print("appending array value:")
    arr_var.append("setting")
    print("----")
    print("setting array value:")
    arr_var[0] = "setting"
    print("----")
    prints("getting array value:", arr_var[0])
    print("----")
    prints("setting the array variable itself")
    arr_var = []
    print("----")
    prints("has arr identity changed?", initial_arr != arr_var)


func test_obj():
    var initial_obj = obj_var
    prints("initial obj", initial_obj)
    print("----")
    print("setting object value:")
    obj_var["name"] = "setting"
    print("----")
    prints("has obj identity changed?", initial_obj != obj_var)
    initial_obj = obj_var
    print("----")
    print("setting object value with set:")
    obj_var.set("name", "setting")
    print("----")
    print("setting object value:")
    obj_var["name"] = "setting"
    print("----")
    prints("getting object value:", obj_var["name"])
    print("----")
    prints("setting the object variable itself")
    obj_var = Node.new()
    print("----")
    prints("has obj identity changed?", initial_obj != obj_var)


    
func set_dict_var(value):
    prints("set_dict_var", value)
    dict_var = value
        

func get_dict_var():
    prints("get_dict_var")
    return dict_var
   

func set_arr_var(value):
    prints("set_arr_var", value)
    arr_var = value
        

func get_arr_var():
    prints("get_arr_var")
    return arr_var
    

func set_obj_var(value):
    prints("set_obj_var", value)
    obj_var = value
        

func get_obj_var():
    prints("get_obj_var")
    return obj_var
    

func _init():
    test_dict()
    test_arr()
    test_obj()
    quit()

[vnen]

I'll close this as it's not a bug, but intended behavior. Calling the setter in the other cases is the issue, which #62462 should fix.