Merge pull request #62462 from vnen/gdscript-setter-chaining

GDScript: Fix setter being called in chains for shared types

core/variant/variant.cpp

@@ -3327,13 +3327,20 @@ Vector<Variant> varray(const Variant &p_arg1, const Variant &p_arg2, const Varia
 void Variant::static_assign(const Variant &p_variant) {
 }
 
-bool Variant::is_shared() const {
-	switch (type) {
+bool Variant::is_type_shared(Variant::Type p_type) {
+	switch (p_type) {
 		case OBJECT:
-			return true;
 		case ARRAY:
-			return true;
 		case DICTIONARY:
+		case PACKED_BYTE_ARRAY:
+		case PACKED_INT32_ARRAY:
+		case PACKED_INT64_ARRAY:
+		case PACKED_FLOAT32_ARRAY:
+		case PACKED_FLOAT64_ARRAY:
+		case PACKED_STRING_ARRAY:
+		case PACKED_VECTOR2_ARRAY:
+		case PACKED_VECTOR3_ARRAY:
+		case PACKED_COLOR_ARRAY:
 			return true;
 		default: {
 		}
@@ -3342,6 +3349,10 @@ bool Variant::is_shared() const {
 	return false;
 }
 
+bool Variant::is_shared() const {
+	return is_type_shared(type);
+}
+
 void Variant::_variant_call_error(const String &p_method, Callable::CallError &error) {
 	switch (error.error) {
 		case Callable::CallError::CALL_ERROR_INVALID_ARGUMENT: {

core/variant/variant.h

@@ -297,6 +297,7 @@ public:
 	static String get_type_name(Variant::Type p_type);
 	static bool can_convert(Type p_type_from, Type p_type_to);
 	static bool can_convert_strict(Type p_type_from, Type p_type_to);
+	static bool is_type_shared(Variant::Type p_type);
 
 	bool is_ref_counted() const;
 	_FORCE_INLINE_ bool is_num() const {

modules/gdscript/gdscript_byte_codegen.cpp

@@ -1336,6 +1336,18 @@ void GDScriptByteCodeGenerator::write_endif() {
 	if_jmp_addrs.pop_back();
 }
 
+void GDScriptByteCodeGenerator::write_jump_if_shared(const Address &p_value) {
+	append(GDScriptFunction::OPCODE_JUMP_IF_SHARED, 1);
+	append(p_value);
+	if_jmp_addrs.push_back(opcodes.size());
+	append(0); // Jump destination, will be patched.
+}
+
+void GDScriptByteCodeGenerator::write_end_jump_if_shared() {
+	patch_jump(if_jmp_addrs.back()->get());
+	if_jmp_addrs.pop_back();
+}
+
 void GDScriptByteCodeGenerator::start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) {
 	Address counter(Address::LOCAL_VARIABLE, add_local("@counter_pos", p_iterator_type), p_iterator_type);
 	Address container(Address::LOCAL_VARIABLE, add_local("@container_pos", p_list_type), p_list_type);

modules/gdscript/gdscript_byte_codegen.h

@@ -479,6 +479,8 @@ public:
 	virtual void write_if(const Address &p_condition) override;
 	virtual void write_else() override;
 	virtual void write_endif() override;
+	virtual void write_jump_if_shared(const Address &p_value) override;
+	virtual void write_end_jump_if_shared() override;
 	virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) override;
 	virtual void write_for_assignment(const Address &p_variable, const Address &p_list) override;
 	virtual void write_for() override;

modules/gdscript/gdscript_codegen.h

@@ -140,6 +140,8 @@ public:
 	virtual void write_if(const Address &p_condition) = 0;
 	virtual void write_else() = 0;
 	virtual void write_endif() = 0;
+	virtual void write_jump_if_shared(const Address &p_value) = 0;
+	virtual void write_end_jump_if_shared() = 0;
 	virtual void start_for(const GDScriptDataType &p_iterator_type, const GDScriptDataType &p_list_type) = 0;
 	virtual void write_for_assignment(const Address &p_variable, const Address &p_list) = 0;
 	virtual void write_for() = 0;

modules/gdscript/gdscript_compiler.cpp

@@ -1056,13 +1056,25 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 
 				// Set back the values into their bases.
 				for (const ChainInfo &info : set_chain) {
-					if (!info.is_named) {
-						gen->write_set(info.base, info.key, assigned);
-						if (info.key.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
-							gen->pop_temporary();
+					bool known_type = assigned.type.has_type;
+					bool is_shared = Variant::is_type_shared(assigned.type.builtin_type);
+
+					if (!known_type) {
+						// Jump shared values since they are already updated in-place.
+						gen->write_jump_if_shared(assigned);
+					}
+					if (known_type && !is_shared) {
+						if (!info.is_named) {
+							gen->write_set(info.base, info.key, assigned);
+							if (info.key.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
+								gen->pop_temporary();
+							}
+						} else {
+							gen->write_set_named(info.base, info.name, assigned);
 						}
-					} else {
-						gen->write_set_named(info.base, info.name, assigned);
+					}
+					if (!known_type) {
+						gen->write_end_jump_if_shared();
 					}
 					if (assigned.mode == GDScriptCodeGenerator::Address::TEMPORARY) {
 						gen->pop_temporary();
@@ -1070,19 +1082,35 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
 					assigned = info.base;
 				}
 
-				// If this is a class member property, also assign to it.
-				// This allow things like: position.x += 2.0
-				if (assign_class_member_property != StringName()) {
-					gen->write_set_member(assigned, assign_class_member_property);
-				}
-				// Same as above but for members
-				if (is_member_property) {
-					if (member_property_has_setter && !member_property_is_in_setter) {
-						Vector<GDScriptCodeGenerator::Address> args;
-						args.push_back(assigned);
-						gen->write_call(GDScriptCodeGenerator::Address(), GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::SELF), member_property_setter_function, args);
-					} else {
-						gen->write_assign(target_member_property, assigned);
+				bool known_type = assigned.type.has_type;
+				bool is_shared = Variant::is_type_shared(assigned.type.builtin_type);
+
+				if (!known_type || !is_shared) {
+					// If this is a class member property, also assign to it.
+					// This allow things like: position.x += 2.0
+					if (assign_class_member_property != StringName()) {
+						if (!known_type) {
+							gen->write_jump_if_shared(assigned);
+						}
+						gen->write_set_member(assigned, assign_class_member_property);
+						if (!known_type) {
+							gen->write_end_jump_if_shared();
+						}
+					} else if (is_member_property) {
+						// Same as above but for script members.
+						if (!known_type) {
+							gen->write_jump_if_shared(assigned);
+						}
+						if (member_property_has_setter && !member_property_is_in_setter) {
+							Vector<GDScriptCodeGenerator::Address> args;
+							args.push_back(assigned);
+							gen->write_call(GDScriptCodeGenerator::Address(), GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::SELF), member_property_setter_function, args);
+						} else {
+							gen->write_assign(target_member_property, assigned);
+						}
+						if (!known_type) {
+							gen->write_end_jump_if_shared();
+						}
 					}
 				}
 

modules/gdscript/gdscript_disassembler.cpp

@@ -838,6 +838,14 @@ void GDScriptFunction::disassemble(const Vector<String> &p_code_lines) const {
 
 				incr = 1;
 			} break;
+			case OPCODE_JUMP_IF_SHARED: {
+				text += "jump-if-shared ";
+				text += DADDR(1);
+				text += " to ";
+				text += itos(_code_ptr[ip + 2]);
+
+				incr = 3;
+			} break;
 			case OPCODE_RETURN: {
 				text += "return ";
 				text += DADDR(1);

modules/gdscript/gdscript_function.h

@@ -304,6 +304,7 @@ public:
 		OPCODE_JUMP_IF,
 		OPCODE_JUMP_IF_NOT,
 		OPCODE_JUMP_TO_DEF_ARGUMENT,
+		OPCODE_JUMP_IF_SHARED,
 		OPCODE_RETURN,
 		OPCODE_RETURN_TYPED_BUILTIN,
 		OPCODE_RETURN_TYPED_ARRAY,

modules/gdscript/gdscript_vm.cpp

@@ -311,6 +311,7 @@ void (*type_init_function_table[])(Variant *) = {
 		&&OPCODE_JUMP_IF,                            \
 		&&OPCODE_JUMP_IF_NOT,                        \
 		&&OPCODE_JUMP_TO_DEF_ARGUMENT,               \
+		&&OPCODE_JUMP_IF_SHARED,                     \
 		&&OPCODE_RETURN,                             \
 		&&OPCODE_RETURN_TYPED_BUILTIN,               \
 		&&OPCODE_RETURN_TYPED_ARRAY,                 \
@@ -2361,6 +2362,21 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 			}
 			DISPATCH_OPCODE;
 
+			OPCODE(OPCODE_JUMP_IF_SHARED) {
+				CHECK_SPACE(3);
+
+				GET_INSTRUCTION_ARG(val, 0);
+
+				if (val->is_shared()) {
+					int to = _code_ptr[ip + 2];
+					GD_ERR_BREAK(to < 0 || to > _code_size);
+					ip = to;
+				} else {
+					ip += 3;
+				}
+			}
+			DISPATCH_OPCODE;
+
 			OPCODE(OPCODE_RETURN) {
 				CHECK_SPACE(2);
 				GET_INSTRUCTION_ARG(r, 0);