Fix terrain rendering bug

remove operator overloading for clarity, reintroduce old location
adition as legacy_sum and similar functions, change new location
addition to vector_sum. The old functions are used where the old
operators were used.

src/builder.cpp

@@ -517,7 +517,7 @@ terrain_builder::building_rule terrain_builder::rotate_rule(const terrain_builde
 
 	for(cons2 = tmp_cons.begin(); cons2 != tmp_cons.end(); ++cons2) {
 		// Adjusts positions
-		cons2->second.loc += gamemap::location(-minx, -((miny-1)/2));
+		cons2->second.loc.legacy_sum_assign(gamemap::location(-minx, -((miny-1)/2)));
 		ret.constraints[cons2->second.loc] = cons2->second;
 	}
 
@@ -868,7 +868,7 @@ bool terrain_builder::rule_matches(const terrain_builder::building_rule &rule,
 			cons != rule.constraints.end(); ++cons) {
 
 		// Translated location
-		const gamemap::location tloc = loc + cons->second.loc;
+		const gamemap::location tloc = loc.legacy_sum(cons->second.loc);
 
 		if(!tile_map_.on_map(tloc)) {
 			return false;
@@ -910,7 +910,7 @@ void terrain_builder::apply_rule(const terrain_builder::building_rule &rule, con
 			constraint != rule.constraints.end(); ++constraint) {
 
 		rule_imagelist::const_iterator img;
-		const gamemap::location tloc = loc + constraint->second.loc;
+		const gamemap::location tloc = loc.legacy_sum(constraint->second.loc);
 		if(!tile_map_.on_map(tloc)) {
 			return;
 		}
@@ -1003,7 +1003,7 @@ void terrain_builder::build_terrains()
 
 			for(std::vector<gamemap::location>::const_iterator itor = locations->begin();
 					itor != locations->end(); ++itor) {
-				const gamemap::location loc = *itor - min_constraint->second.loc;
+				const gamemap::location loc = itor->legacy_difference(min_constraint->second.loc);
 
 				if(rule_matches(rule, loc, rule_index, min_constraint)) {
 					apply_rule(rule, loc);

src/editor/editor.cpp

@@ -687,7 +687,7 @@ void map_editor::edit_rotate_selection()
 		center = gamemap::location(0,0);
 		std::set<gamemap::location>::const_iterator it;
 		for(it = selected_hexes_.begin(); it != selected_hexes_.end(); it++) {
-			center = center + *it;
+			center.legacy_sum_assign(*it);
 		}
 		center.x = center.x / selected_hexes_.size();
 		center.y = center.y / selected_hexes_.size();
@@ -822,7 +822,7 @@ void map_editor::copy_buffer(map_buffer& buffer, const std::set<gamemap::locatio
 	std::set<gamemap::location>::const_iterator it;
 	for (it = locs.begin(); it != locs.end(); it++) {
 		t_translation::t_terrain terrain = map_.get_terrain(*it);
-		buffer.push_back(buffer_item(*it-origin, terrain, starting_side_at(map_, *it)));
+		buffer.push_back(buffer_item(it->legacy_difference(origin), terrain, starting_side_at(map_, *it)));
 	}
 }
 
@@ -832,7 +832,7 @@ void map_editor::paste_buffer(const map_buffer &buffer, const gamemap::location
 	std::vector<buffer_item>::const_iterator it;
 	for (it = buffer.begin(); it != buffer.end(); it++) {
 		//the addition of locations is not commutative !
-		gamemap::location target = it->offset + loc;
+		gamemap::location target = it->offset.legacy_sum(loc);
 
 		if (map_.on_board_with_border(target)) {
 			undo_action.add_terrain(map_.get_terrain(target), it->terrain, target);
@@ -1204,7 +1204,7 @@ void map_editor::left_button_down(const int mousex, const int mousey) {
 		gui_.clear_highlighted_locs();
 		std::set<gamemap::location>::const_iterator it;
 		for (it = selected_hexes_.begin(); it != selected_hexes_.end(); it++) {
-			const gamemap::location hl_loc = (*it-selection_move_start_) + hex;
+			const gamemap::location hl_loc = it->legacy_difference(selection_move_start_).legacy_sum(hex);
 			if (map_.on_board_with_border(hl_loc)) {
 				gui_.add_highlighted_loc(hl_loc);
 			}

src/editor2/brush.cpp

@@ -55,7 +55,7 @@ std::set<gamemap::location> brush::project(const gamemap::location& hotspot) con
 {
 	std::set<gamemap::location> result;
 	foreach (const gamemap::location& relative, relative_tiles_) {
-		result.insert(relative + hotspot);
+		result.insert(relative.vector_sum(hotspot));
 	}
 	return result;
 }

src/editor2/map_fragment.cpp

@@ -48,7 +48,7 @@ std::set<gamemap::location> map_fragment::get_area() const
 void map_fragment::paste_into(gamemap& map, const gamemap::location& loc) const
 {
 	foreach (const tile_info& i, items_) {
-		map.set_terrain(i.offset + loc, i.terrain);
+		map.set_terrain(i.offset.vector_sum(loc), i.terrain);
 	}
 }
 

src/map.cpp

@@ -202,21 +202,50 @@ void gamemap::location::write(config& cfg) const
 	cfg["y"] = buf;
 }
 
-gamemap::location gamemap::location::operator-() const
+gamemap::location gamemap::location::legacy_negation() const
 {
-	location ret(-x, -y);
-	ret.y -= x & 1; //subtract one if we're on an odd x coordinate
-	return ret;
+	return location(-x, -y);
 }
 
-gamemap::location gamemap::location::operator+(const gamemap::location& a) const
+gamemap::location gamemap::location::legacy_sum(const gamemap::location& a) const
 {
-	gamemap::location ret = *this;
-	ret += a;
-	return ret;
+	return location(*this).legacy_sum_assign(a);
 }
 
-gamemap::location& gamemap::location::operator+=(const gamemap::location &a)
+gamemap::location& gamemap::location::legacy_sum_assign(const gamemap::location &a)
+{
+	bool parity = (x & 1) != 0;
+	x += a.x;
+	y += a.y;
+	if((a.x > 0) && (a.x % 2) && parity)
+		y++;
+	if((a.x < 0) && (a.x % 2) && !parity)
+		y--;
+
+	return *this;
+}
+
+gamemap::location gamemap::location::legacy_difference(const gamemap::location &a) const
+{
+	return legacy_sum(a.legacy_negation());
+}
+
+gamemap::location& gamemap::location::legacy_difference_assign(const gamemap::location &a)
+{
+	return legacy_sum_assign(a.legacy_negation());
+}
+
+gamemap::location gamemap::location::vector_negation() const
+{
+	return location(-x, -y - (x & 1)); //subtract one if we're on an odd x coordinate
+}
+
+gamemap::location gamemap::location::vector_sum(const gamemap::location& a) const
+{
+	return location(*this).vector_sum_assign(a);
+}
+
+gamemap::location& gamemap::location::vector_sum_assign(const gamemap::location &a)
 {
 	y += (x & 1) * (a.x & 1); //add one if both x coords are odd
 	x += a.x;
@@ -224,14 +253,14 @@ gamemap::location& gamemap::location::operator+=(const gamemap::location &a)
 	return *this;
 }
 
-gamemap::location gamemap::location::operator-(const gamemap::location &a) const
+gamemap::location gamemap::location::vector_difference(const gamemap::location &a) const
 {
-	return operator+(-a);
+	return vector_sum(a.vector_negation());
 }
 
-gamemap::location& gamemap::location::operator-=(const gamemap::location &a)
+gamemap::location& gamemap::location::vector_difference_assign(const gamemap::location &a)
 {
-	return operator+=(-a);
+	return vector_sum_assign(a.vector_negation());
 }
 
 gamemap::location gamemap::location::get_direction(
@@ -255,7 +284,7 @@ gamemap::location gamemap::location::get_direction(
 }
 
 gamemap::location::DIRECTION gamemap::location::get_relative_dir(gamemap::location loc) const {
-	location diff = loc -*this;
+	location diff = loc.legacy_difference(*this);
 	if(diff == location(0,0)) return NDIRECTIONS;
 	if( diff.y < 0 && diff.x >= 0 && abs(diff.x) >= abs(diff.y)) return NORTH_EAST;
 	if( diff.y < 0 && diff.x <  0 && abs(diff.x) >= abs(diff.y)) return NORTH_WEST;

src/map.hpp

@@ -96,11 +96,25 @@ public:
 		bool operator!=(const location& a) const { return !operator==(a); }
 
 		// Adds an absolute location to a "delta" location
-		location operator-() const;
-		location operator+(const location &a) const;
-		location &operator+=(const location &a);
-		location operator-(const location &a) const;
-		location &operator-=(const location &a);
+		// This is not the mathematically correct bahviur, it is neither
+		// commutative nor associative. Negative coordinates may give strange
+		// results. It is retained because terain builder code relies in this
+		// broken behaviour. Best avoid.
+		location legacy_negation() const;
+		location legacy_sum(const location &a) const;
+		location& legacy_sum_assign(const location &a);
+		location legacy_difference(const location &a) const;
+		location &legacy_difference_assign(const location &a);
+
+		// Location arithmetic operations treating the locations as vectors in
+		// a hex-based space. These operations form an abelian group, i.e. 
+		// everything works as you would expect addition and substraction to 
+		// work, with associativity and commutativity.
+		location vector_negation() const;
+		location vector_sum(const location &a) const;
+		location& vector_sum_assign(const location &a);
+		location vector_difference(const location &a) const;
+		location &vector_difference_assign(const location &a);
 
 		// Do n step in the direction d 
 		location get_direction(DIRECTION d, int n = 1) const;