Merge pull request #70176 from clayjohn/mesh-index

Switch surface indexing to start at 0 so string name matches integer index

4 files changed, 28 insertions, 14 deletions

diff --git a/scene/3d/mesh_instance_3d.cpp b/scene/3d/mesh_instance_3d.cpp
index 10f3ab5c79..16de95bfb1 100644
--- a/scene/3d/mesh_instance_3d.cpp
+++ b/scene/3d/mesh_instance_3d.cpp
@@ -53,21 +53,22 @@ bool MeshInstance3D::_set(const StringName &p_name, const Variant &p_value) {
 	if (p_name.operator String().begins_with("surface_material_override/")) {
 		int idx = p_name.operator String().get_slicec('/', 1).to_int();
 
-		// This is a bit of a hack to ensure compatibility with older material
-		// overrides that start indexing at 0.
+		// This is a bit of a hack to ensure compatibility with material
+		// overrides that start indexing at 1.
 		// We assume that idx 0 is always read first, if its not, this won't work.
 		if (idx == 0) {
-			old_surface_index = true;
+			surface_index_0 = true;
 		}
-		if (old_surface_index) {
-			idx++;
+		if (!surface_index_0) {
+			// This means the file was created when the indexing started at 1, so decrease by one.
+			idx--;
 		}
 
 		if (idx > surface_override_materials.size() || idx < 0) {
 			return false;
 		}
 
-		set_surface_override_material(idx - 1, p_value);
+		set_surface_override_material(idx, p_value);
 		return true;
 	}
 
@@ -86,7 +87,7 @@ bool MeshInstance3D::_get(const StringName &p_name, Variant &r_ret) const {
 	}
 
 	if (p_name.operator String().begins_with("surface_material_override/")) {
-		int idx = p_name.operator String().get_slicec('/', 1).to_int() - 1;
+		int idx = p_name.operator String().get_slicec('/', 1).to_int();
 		if (idx >= surface_override_materials.size() || idx < 0) {
 			return false;
 		}
@@ -109,7 +110,7 @@ void MeshInstance3D::_get_property_list(List<PropertyInfo> *p_list) const {
 	}
 
 	if (mesh.is_valid()) {
-		for (int i = 1; i <= mesh->get_surface_count(); i++) {
+		for (int i = 0; i < mesh->get_surface_count(); i++) {
 			p_list->push_back(PropertyInfo(Variant::OBJECT, vformat("%s/%d", PNAME("surface_material_override"), i), PROPERTY_HINT_RESOURCE_TYPE, "BaseMaterial3D,ShaderMaterial", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_DEFERRED_SET_RESOURCE));
 		}
 	}
diff --git a/scene/3d/mesh_instance_3d.h b/scene/3d/mesh_instance_3d.h
index 9e6dbaf160..e1cc5272ea 100644
--- a/scene/3d/mesh_instance_3d.h
+++ b/scene/3d/mesh_instance_3d.h
@@ -57,7 +57,7 @@ protected:
 	bool _set(const StringName &p_name, const Variant &p_value);
 	bool _get(const StringName &p_name, Variant &r_ret) const;
 	void _get_property_list(List<PropertyInfo> *p_list) const;
-	bool old_surface_index = false;
+	bool surface_index_0 = false;
 
 	void _notification(int p_what);
 	static void _bind_methods();
diff --git a/scene/resources/mesh.cpp b/scene/resources/mesh.cpp
index a5e7602c8b..af770ddede 100644
--- a/scene/resources/mesh.cpp
+++ b/scene/resources/mesh.cpp
@@ -1118,7 +1118,19 @@ bool ArrayMesh::_set(const StringName &p_name, const Variant &p_value) {
 		if (sl == -1) {
 			return false;
 		}
-		int idx = sname.substr(8, sl - 8).to_int() - 1;
+		int idx = sname.substr(8, sl - 8).to_int();
+
+		// This is a bit of a hack to ensure compatibility with older material
+		// overrides that start indexing at 1.
+		// We assume that idx 0 is always read first, if its not, this won't work.
+		if (idx == 0) {
+			surface_index_0 = true;
+		}
+		if (!surface_index_0) {
+			// This means the file was created when the indexing started at 1, so decrease by one.
+			idx--;
+		}
+
 		String what = sname.get_slicec('/', 1);
 		if (what == "material") {
 			surface_set_material(idx, p_value);
@@ -1491,7 +1503,7 @@ bool ArrayMesh::_get(const StringName &p_name, Variant &r_ret) const {
 		if (sl == -1) {
 			return false;
 		}
-		int idx = sname.substr(8, sl - 8).to_int() - 1;
+		int idx = sname.substr(8, sl - 8).to_int();
 		String what = sname.get_slicec('/', 1);
 		if (what == "material") {
 			r_ret = surface_get_material(idx);
@@ -1519,11 +1531,11 @@ void ArrayMesh::_get_property_list(List<PropertyInfo> *p_list) const {
 	}
 
 	for (int i = 0; i < surfaces.size(); i++) {
-		p_list->push_back(PropertyInfo(Variant::STRING, "surface_" + itos(i + 1) + "/name", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_EDITOR));
+		p_list->push_back(PropertyInfo(Variant::STRING, "surface_" + itos(i) + "/name", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_EDITOR));
 		if (surfaces[i].is_2d) {
-			p_list->push_back(PropertyInfo(Variant::OBJECT, "surface_" + itos(i + 1) + "/material", PROPERTY_HINT_RESOURCE_TYPE, "CanvasItemMaterial,ShaderMaterial", PROPERTY_USAGE_EDITOR));
+			p_list->push_back(PropertyInfo(Variant::OBJECT, "surface_" + itos(i) + "/material", PROPERTY_HINT_RESOURCE_TYPE, "CanvasItemMaterial,ShaderMaterial", PROPERTY_USAGE_EDITOR));
 		} else {
-			p_list->push_back(PropertyInfo(Variant::OBJECT, "surface_" + itos(i + 1) + "/material", PROPERTY_HINT_RESOURCE_TYPE, "BaseMaterial3D,ShaderMaterial", PROPERTY_USAGE_EDITOR));
+			p_list->push_back(PropertyInfo(Variant::OBJECT, "surface_" + itos(i) + "/material", PROPERTY_HINT_RESOURCE_TYPE, "BaseMaterial3D,ShaderMaterial", PROPERTY_USAGE_EDITOR));
 		}
 	}
 }
diff --git a/scene/resources/mesh.h b/scene/resources/mesh.h
index 6f995280e8..fabc09a42c 100644
--- a/scene/resources/mesh.h
+++ b/scene/resources/mesh.h
@@ -262,6 +262,7 @@ protected:
 	bool _set(const StringName &p_name, const Variant &p_value);
 	bool _get(const StringName &p_name, Variant &r_ret) const;
 	void _get_property_list(List<PropertyInfo> *p_list) const;
+	bool surface_index_0 = false;
 
 	virtual void reset_state() override;