GDScript: Improve usability of setter chains

- Consider PackedArrays non-shared since they are copied on C++/script
  boundaries.
- Add error messages in the analyzer when assigning to read-only
  properties.
- Add specific error message at runtime when assignment fails because
  the property is read-only.
This commit is contained in:
George Marques 2023-01-30 14:50:08 -03:00
parent 315d3c4d21
commit 5fc7918594
No known key found for this signature in database
GPG key ID: 046BD46A3201E43D
13 changed files with 92 additions and 19 deletions

View file

@ -3601,15 +3601,6 @@ bool Variant::is_type_shared(Variant::Type p_type) {
case OBJECT:
case ARRAY:
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: {
}

View file

@ -2240,6 +2240,28 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
if (assignee_type.is_constant || (p_assignment->assignee->type == GDScriptParser::Node::SUBSCRIPT && static_cast<GDScriptParser::SubscriptNode *>(p_assignment->assignee)->base->is_constant)) {
push_error("Cannot assign a new value to a constant.", p_assignment->assignee);
return;
} else if (assignee_type.is_read_only) {
push_error("Cannot assign a new value to a read-only property.", p_assignment->assignee);
return;
} else if (p_assignment->assignee->type == GDScriptParser::Node::SUBSCRIPT) {
GDScriptParser::SubscriptNode *sub = static_cast<GDScriptParser::SubscriptNode *>(p_assignment->assignee);
while (sub) {
const GDScriptParser::DataType &base_type = sub->base->datatype;
if (base_type.is_hard_type() && base_type.is_read_only) {
if (base_type.kind == GDScriptParser::DataType::BUILTIN && !Variant::is_type_shared(base_type.builtin_type)) {
push_error("Cannot assign a new value to a read-only property.", p_assignment->assignee);
return;
}
} else {
break;
}
if (sub->base->type == GDScriptParser::Node::SUBSCRIPT) {
sub = static_cast<GDScriptParser::SubscriptNode *>(sub->base);
} else {
sub = nullptr;
}
}
}
// Check if assigned value is an array literal, so we can make it a typed array too if appropriate.
@ -3329,7 +3351,8 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
StringName getter_name = ClassDB::get_property_getter(native, name);
MethodBind *getter = ClassDB::get_method(native, getter_name);
if (getter != nullptr) {
p_identifier->set_datatype(type_from_property(getter->get_return_info()));
bool has_setter = ClassDB::get_property_setter(native, name) != StringName();
p_identifier->set_datatype(type_from_property(getter->get_return_info(), false, !has_setter));
p_identifier->source = GDScriptParser::IdentifierNode::INHERITED_VARIABLE;
}
return;
@ -4268,8 +4291,9 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_metatype(const GDScriptPars
return result;
}
GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo &p_property, bool p_is_arg) const {
GDScriptParser::DataType GDScriptAnalyzer::type_from_property(const PropertyInfo &p_property, bool p_is_arg, bool p_is_readonly) const {
GDScriptParser::DataType result;
result.is_read_only = p_is_readonly;
result.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
if (p_property.type == Variant::NIL && (p_is_arg || (p_property.usage & PROPERTY_USAGE_NIL_IS_VARIANT))) {
// Variant

View file

@ -115,7 +115,7 @@ class GDScriptAnalyzer {
Array make_array_from_element_datatype(const GDScriptParser::DataType &p_element_datatype, const GDScriptParser::Node *p_source_node = nullptr);
GDScriptParser::DataType type_from_variant(const Variant &p_value, const GDScriptParser::Node *p_source);
static GDScriptParser::DataType type_from_metatype(const GDScriptParser::DataType &p_meta_type);
GDScriptParser::DataType type_from_property(const PropertyInfo &p_property, bool p_is_arg = false) const;
GDScriptParser::DataType type_from_property(const PropertyInfo &p_property, bool p_is_arg = false, bool p_is_readonly = false) const;
GDScriptParser::DataType make_global_class_meta_type(const StringName &p_class_name, const GDScriptParser::Node *p_source);
bool get_function_signature(GDScriptParser::Node *p_source, bool p_is_constructor, GDScriptParser::DataType base_type, const StringName &p_function, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg);
bool function_signature_from_info(const MethodInfo &p_info, GDScriptParser::DataType &r_return_type, List<GDScriptParser::DataType> &r_par_types, int &r_default_arg_count, bool &r_static, bool &r_vararg);

View file

@ -122,6 +122,7 @@ public:
TypeSource type_source = UNDETECTED;
bool is_constant = false;
bool is_read_only = false;
bool is_meta_type = false;
bool is_coroutine = false; // For function calls.
@ -206,6 +207,7 @@ public:
void operator=(const DataType &p_other) {
kind = p_other.kind;
type_source = p_other.type_source;
is_read_only = p_other.is_read_only;
is_constant = p_other.is_constant;
is_meta_type = p_other.is_meta_type;
is_coroutine = p_other.is_coroutine;

View file

@ -811,13 +811,22 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
#ifdef DEBUG_ENABLED
if (!valid) {
Object *obj = dst->get_validated_object();
String v = index->operator String();
if (!v.is_empty()) {
v = "'" + v + "'";
} else {
v = "of type '" + _get_var_type(index) + "'";
bool read_only_property = false;
if (obj) {
read_only_property = ClassDB::has_property(obj->get_class_name(), v) && (ClassDB::get_property_setter(obj->get_class_name(), v) == StringName());
}
if (read_only_property) {
err_text = vformat(R"(Cannot set value into property "%s" (on base "%s") because it is read-only.)", v, _get_var_type(dst));
} else {
if (!v.is_empty()) {
v = "'" + v + "'";
} else {
v = "of type '" + _get_var_type(index) + "'";
}
err_text = "Invalid set index " + v + " (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'";
}
err_text = "Invalid set index " + v + " (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'";
OPCODE_BREAK;
}
#endif
@ -1003,8 +1012,16 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
#ifdef DEBUG_ENABLED
if (!valid) {
String err_type;
err_text = "Invalid set index '" + String(*index) + "' (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'.";
Object *obj = dst->get_validated_object();
bool read_only_property = false;
if (obj) {
read_only_property = ClassDB::has_property(obj->get_class_name(), *index) && (ClassDB::get_property_setter(obj->get_class_name(), *index) == StringName());
}
if (read_only_property) {
err_text = vformat(R"(Cannot set value into property "%s" (on base "%s") because it is read-only.)", String(*index), _get_var_type(dst));
} else {
err_text = "Invalid set index '" + String(*index) + "' (on base: '" + _get_var_type(dst) + "') with value of type '" + _get_var_type(value) + "'.";
}
OPCODE_BREAK;
}
#endif

View file

@ -0,0 +1,4 @@
func test():
var tree := SceneTree.new()
tree.root = Window.new()
tree.free()

View file

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a new value to a read-only property.

View file

@ -0,0 +1,4 @@
func test():
var state := PhysicsDirectBodyState3DExtension.new()
state.center_of_mass.x += 1.0
state.free()

View file

@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Cannot assign a new value to a read-only property.

View file

@ -0,0 +1,7 @@
func test():
var state = PhysicsDirectBodyState3DExtension.new()
assign(state)
state.free()
func assign(state):
state.center_of_mass.x -= 1.0

View file

@ -0,0 +1,6 @@
GDTEST_RUNTIME_ERROR
>> SCRIPT ERROR
>> on function: assign()
>> runtime/assign_to_read_only_property.gd
>> 7
>> Cannot set value into property "center_of_mass" (on base "PhysicsDirectBodyState3DExtension") because it is read-only.

View file

@ -0,0 +1,8 @@
func test():
var state = PhysicsDirectBodyState3DExtension.new()
var prop = &"center_of_mass"
assign(state, prop)
state.free()
func assign(state, prop):
state[prop].x = 1.0

View file

@ -0,0 +1,6 @@
GDTEST_RUNTIME_ERROR
>> SCRIPT ERROR
>> on function: assign()
>> runtime/assign_to_read_only_property_with_variable_index.gd
>> 8
>> Cannot set value into property "center_of_mass" (on base "PhysicsDirectBodyState3DExtension") because it is read-only.