Skip to content

Commit a9e846b

Browse files
committed
Fix get_region_map_index() checking bounds
1 parent 5def3d3 commit a9e846b

4 files changed

+22
-17
lines changed

src/terrain_3d_editor.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ Ref<Terrain3DRegion> Terrain3DEditor::_operate_region(const Vector2i &p_region_l
3838
}
3939
if (storage->get_region_map_index(p_region_loc) < 0) {
4040
if (can_print) {
41-
LOG(ERROR, "Specified position outside of maximum bounds");
41+
LOG(ERROR, "Location ", p_region_loc, " out of bounds. Max: ",
42+
-Terrain3DStorage::REGION_MAP_SIZE / 2, " to ", Terrain3DStorage::REGION_MAP_SIZE / 2 - 1);
4243
}
4344
return Ref<Terrain3DRegion>();
4445
}

src/terrain_3d_region.cpp

+6-5
Original file line numberDiff line numberDiff line change
@@ -251,12 +251,13 @@ Error Terrain3DRegion::save(const String &p_path, const bool p_16_bit) {
251251
void Terrain3DRegion::set_location(const Vector2i &p_location) {
252252
// In the future anywhere they want to put the location might be fine, but because of region_map
253253
// We have a limitation of 16x16 and eventually 45x45.
254-
if (abs(p_location.x) < Terrain3DStorage::REGION_MAP_SIZE / 2 && abs(p_location.y) < Terrain3DStorage::REGION_MAP_SIZE / 2) {
255-
LOG(INFO, "Set location: ", p_location);
256-
_location = p_location;
257-
} else {
258-
LOG(ERROR, "Location out of bounds: ", p_location);
254+
if (Terrain3DStorage::get_region_map_index(p_location) < 0) {
255+
LOG(ERROR, "Location ", p_location, " out of bounds. Max: ",
256+
-Terrain3DStorage::REGION_MAP_SIZE / 2, " to ", Terrain3DStorage::REGION_MAP_SIZE / 2 - 1);
257+
return;
259258
}
259+
LOG(INFO, "Set location: ", p_location);
260+
_location = p_location;
260261
}
261262

262263
void Terrain3DRegion::set_data(const Dictionary &p_data) {

src/terrain_3d_storage.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,8 @@ Error Terrain3DStorage::add_region(const Ref<Terrain3DRegion> &p_region, const b
199199

200200
// Check bounds and slow report errors
201201
if (get_region_map_index(region_loc) < 0) {
202-
LOG(ERROR, "Specified position outside of maximum region map size: +/-",
203-
real_t((REGION_MAP_SIZE / 2) * _region_size) * _mesh_vertex_spacing);
202+
LOG(ERROR, "Location ", region_loc, " out of bounds. Max: ",
203+
-REGION_MAP_SIZE / 2, " to ", REGION_MAP_SIZE / 2 - 1);
204204
return FAILED;
205205
}
206206

@@ -992,6 +992,7 @@ void Terrain3DStorage::_bind_methods() {
992992
ClassDB::bind_method(D_METHOD("get_regions_active", "copy", "deep"), &Terrain3DStorage::get_regions_active, DEFVAL(false), DEFVAL(false));
993993
ClassDB::bind_method(D_METHOD("get_regions_all"), &Terrain3DStorage::get_regions_all);
994994
ClassDB::bind_method(D_METHOD("get_region_map"), &Terrain3DStorage::get_region_map);
995+
ClassDB::bind_static_method("Terrain3DStorage", D_METHOD("get_region_map_index"), &Terrain3DStorage::get_region_map_index);
995996

996997
ClassDB::bind_method(D_METHOD("has_region", "region_location"), &Terrain3DStorage::has_region);
997998
ClassDB::bind_method(D_METHOD("has_regionp", "global_position"), &Terrain3DStorage::has_regionp);

src/terrain_3d_storage.h

+11-9
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ class Terrain3DStorage : public Object {
104104
TypedArray<Terrain3DRegion> get_regions_active(const bool p_copy = false, const bool p_deep = false) const;
105105
Dictionary get_regions_all() const { return _regions; }
106106
PackedInt32Array get_region_map() const { return _region_map; }
107-
int get_region_map_index(const Vector2i &p_region_loc) const;
107+
static int get_region_map_index(const Vector2i &p_region_loc);
108108

109109
bool has_region(const Vector2i &p_region_loc) const { return get_region_id(p_region_loc) != -1; }
110110
bool has_regionp(const Vector3 &p_global_position) const { return get_region_idp(p_global_position) != -1; }
@@ -197,16 +197,18 @@ VARIANT_ENUM_CAST(Terrain3DStorage::HeightFilter);
197197

198198
/// Inline Region Functions
199199

200-
// This function verifies the location is within the bounds of the _region_map array and
201-
// thus the world. It returns the _region_map index if valid, -1 if not
202-
inline int Terrain3DStorage::get_region_map_index(const Vector2i &p_region_loc) const {
203-
// Offset locations centered on (0,0) to positive only
204-
Vector2i loc = Vector2i(p_region_loc + (REGION_MAP_VSIZE / 2));
205-
int map_index = loc.y * REGION_MAP_SIZE + loc.x;
206-
if (map_index < 0 || map_index >= REGION_MAP_SIZE * REGION_MAP_SIZE) {
200+
// Verifies the location is within the bounds of the _region_map array and
201+
// the world, returning the _region_map index, which contains the region_id.
202+
// Valid region locations are -8, -8 to 7, 7, or when offset: 0, 0 to 15, 15
203+
// If any bits other than 0xF are set, it's out of bounds and returns -1
204+
inline int Terrain3DStorage::get_region_map_index(const Vector2i &p_region_loc) {
205+
// Offset world to positive values only
206+
Vector2i loc = p_region_loc + (REGION_MAP_VSIZE / 2);
207+
// Catch values > 15
208+
if ((uint32_t(loc.x | loc.y) & uint32_t(~0xF)) > 0) {
207209
return -1;
208210
}
209-
return map_index;
211+
return loc.y * REGION_MAP_SIZE + loc.x;
210212
}
211213

212214
// Returns a region location given a global position. No bounds checking nor data access.

0 commit comments

Comments
 (0)