From eb4dd7f8968be5bca5a44f5ec1a676553a9470c2 Mon Sep 17 00:00:00 2001
From: Jelle Raaijmakers <jelle@gmta.nl>
Date: Mon, 2 Oct 2023 22:22:11 +0200
Subject: [PATCH] Maps: Fix spiraling tile iterator end

The `operator++` of the spiraling tile iterator was repeating the first
coordinates (`0, 0`) instead of moving to the next tile on the first
iteration. Swapping the move and check ensures we get to the end of the
iterator, fixing gray tiles that would sometimes pop up in the lower
right.

Since we never return from `operator++` without setting a valid
position, we can drop `current_x` and `current_y` and just use the
`Gfx::Point<T>` directly.
---
 Userland/Applications/Maps/MapWidget.cpp | 32 ++++++++++--------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/Userland/Applications/Maps/MapWidget.cpp b/Userland/Applications/Maps/MapWidget.cpp
index 5027cbfd29..cda31d44c6 100644
--- a/Userland/Applications/Maps/MapWidget.cpp
+++ b/Userland/Applications/Maps/MapWidget.cpp
@@ -379,10 +379,8 @@ public:
         T height;
 
         T index { 0 };
-        T current_x { 0 };
-        T current_y { 0 };
 
-        Gfx::Point<T> last_valid_position { current_x, current_y };
+        Gfx::Point<T> position { 0, 0 };
 
         constexpr Iterator(T width, T height)
             : width(move(width))
@@ -401,28 +399,24 @@ public:
         constexpr bool operator==(Iterator const& other) const { return index == other.index; }
         constexpr bool operator!=(Iterator const& other) const { return index != other.index; }
 
-        constexpr Gfx::Point<T> operator*() const { return last_valid_position; }
+        constexpr Gfx::Point<T> operator*() const { return position; }
 
         constexpr Iterator operator++()
         {
-            auto found_valid_position = false;
-
-            while (!found_valid_position && !is_end()) {
-                // Translating the coordinates makes the range check simpler:
-                T xp = current_x + width / 2;
-                T yp = current_y + height / 2;
-                if (xp >= 0 && xp <= width && yp >= 0 && yp <= height) {
-                    last_valid_position = { current_x, current_y };
-                    found_valid_position = true;
-                }
-
+            while (!is_end()) {
                 // Figure out in which of the four squares we are.
-                if (AK::abs(current_x) <= AK::abs(current_y) && (current_x != current_y || current_x >= 0))
-                    current_x += ((current_y >= 0) ? 1 : -1);
+                if (AK::abs(position.x()) <= AK::abs(position.y()) && (position.x() != position.y() || position.x() >= 0))
+                    position.translate_by(position.y() >= 0 ? 1 : -1, 0);
                 else
-                    current_y += ((current_x >= 0) ? -1 : 1);
-
+                    position.translate_by(0, position.x() >= 0 ? -1 : 1);
                 ++index;
+
+                // Translating the coordinates makes the range check simpler:
+                T xp = position.x() + width / 2;
+                T yp = position.y() + height / 2;
+                if (xp >= 0 && xp <= width && yp >= 0 && yp <= height) {
+                    break;
+                }
             }
 
             return *this;