(svn r19293) -Fix [FS#3566]: Some methods of AIAbstractList left invalid iterators.

bin/ai/regression/regression.nut

@@ -785,6 +785,36 @@ function Regression::List()
 
 	list.Clear();
 	print("  IsEmpty():     " + list.IsEmpty());
+
+	for (local i = 0; i < 10; i++) {
+		list.AddItem(i, 5 + i / 2);
+	}
+
+	local it = list.Begin();
+	print("    " + it + " => " + list.GetValue(it) + "  (" + list.HasNext() + ")");
+	list.Sort(list.SORT_BY_VALUE, list.SORT_ASCENDING);
+	it = list.Next();
+	print("    " + it + " => " + list.GetValue(it) + "  (" + list.HasNext() + ")");
+
+	it = list.Begin();
+	print("    " + it + " => " + list.GetValue(it) + "  (" + list.HasNext() + ")");
+
+	list.SetValue(it + 1, -5);
+	it = list.Next();
+	print("    " + it + " => " + list.GetValue(it) + "  (" + list.HasNext() + ")");
+
+	list.RemoveValue(list.GetValue(it) + 1);
+	it = list.Next();
+	print("    " + it + " => " + list.GetValue(it) + "  (" + list.HasNext() + ")");
+
+	list.RemoveAboveValue(list.GetValue(it));
+	it = list.Next();
+	print("    " + it + " => " + list.GetValue(it) + "  (" + list.HasNext() + ")");
+
+	while (list.HasNext()) {
+		it = list.Next();
+		print("    " + it + " => " + list.GetValue(it));
+	}
 }
 
 function Regression::Map()

bin/ai/regression/regression.txt

@@ -569,6 +569,14 @@
   []:
     4000 => 50
   IsEmpty():     true
+    0 => 5  (true)
+ERROR: Next() is invalid as Begin() is never called
+ERROR: HasNext() is invalid as Begin() is never called
+    0 => 5  (false)
+    0 => 5  (true)
+    2 => 6  (true)
+    3 => 6  (true)
+    9 => 0  (false)
 
 --Company--
   SetName():            true

src/ai/api/ai_abstractlist.cpp

@@ -437,8 +437,8 @@ int32 AIAbstractList::Begin()
 int32 AIAbstractList::Next()
 {
 	if (this->initialized == false) {
-		DEBUG(ai, 0, "ERROR: Next() is invalid as Begin() is never called");
-		return false;
+		DEBUG(ai, 0, "Next() is invalid as Begin() is never called");
+		return 0;
 	}
 	return this->sorter->Next();
 }
@@ -451,7 +451,7 @@ bool AIAbstractList::IsEmpty()
 bool AIAbstractList::HasNext()
 {
 	if (this->initialized == false) {
-		DEBUG(ai, 0, "ERROR: HasNext() is invalid as Begin() is never called");
+		DEBUG(ai, 0, "HasNext() is invalid as Begin() is never called");
 		return false;
 	}
 	return this->sorter->HasNext();
@@ -517,6 +517,7 @@ void AIAbstractList::Sort(SorterType sorter, bool ascending)
 	}
 	this->sorter_type    = sorter;
 	this->sort_ascending = ascending;
+	this->initialized    = false;
 }
 
 void AIAbstractList::AddList(AIAbstractList *list)
@@ -534,12 +535,7 @@ void AIAbstractList::RemoveAboveValue(int32 value)
 
 	for (AIAbstractListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
 		next_iter = iter; next_iter++;
-		if ((*iter).second > value) this->items.erase(iter);
-	}
-
-	for (AIAbstractListBucket::iterator next_iter, iter = this->buckets.begin(); iter != this->buckets.end(); iter = next_iter) {
-		next_iter = iter; next_iter++;
-		if ((*iter).first > value) this->buckets.erase(iter);
+		if ((*iter).second > value) this->RemoveItem((*iter).first);
 	}
 }
 
@@ -549,12 +545,7 @@ void AIAbstractList::RemoveBelowValue(int32 value)
 
 	for (AIAbstractListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
 		next_iter = iter; next_iter++;
-		if ((*iter).second < value) this->items.erase(iter);
-	}
-
-	for (AIAbstractListBucket::iterator next_iter, iter = this->buckets.begin(); iter != this->buckets.end(); iter = next_iter) {
-		next_iter = iter; next_iter++;
-		if ((*iter).first < value) this->buckets.erase(iter);
+		if ((*iter).second < value) this->RemoveItem((*iter).first);
 	}
 }
 
@@ -564,12 +555,7 @@ void AIAbstractList::RemoveBetweenValue(int32 start, int32 end)
 
 	for (AIAbstractListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
 		next_iter = iter; next_iter++;
-		if ((*iter).second > start && (*iter).second < end) this->items.erase(iter);
-	}
-
-	for (AIAbstractListBucket::iterator next_iter, iter = this->buckets.begin(); iter != this->buckets.end(); iter = next_iter) {
-		next_iter = iter; next_iter++;
-		if ((*iter).first > start && (*iter).first < end) this->buckets.erase(iter);
+		if ((*iter).second > start && (*iter).second < end) this->RemoveItem((*iter).first);
 	}
 }
 
@@ -579,12 +565,7 @@ void AIAbstractList::RemoveValue(int32 value)
 
 	for (AIAbstractListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
 		next_iter = iter; next_iter++;
-		if ((*iter).second == value) this->items.erase(iter);
-	}
-
-	for (AIAbstractListBucket::iterator next_iter, iter = this->buckets.begin(); iter != this->buckets.end(); iter = next_iter) {
-		next_iter = iter; next_iter++;
-		if ((*iter).first == value) this->buckets.erase(iter);
+		if ((*iter).second == value) this->RemoveItem((*iter).first);
 	}
 }
 
@@ -677,12 +658,7 @@ void AIAbstractList::KeepAboveValue(int32 value)
 
 	for (AIAbstractListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
 		next_iter = iter; next_iter++;
-		if ((*iter).second <= value) this->items.erase(iter);
-	}
-
-	for (AIAbstractListBucket::iterator next_iter, iter = this->buckets.begin(); iter != this->buckets.end(); iter = next_iter) {
-		next_iter = iter; next_iter++;
-		if ((*iter).first <= value) this->buckets.erase(iter);
+		if ((*iter).second <= value) this->RemoveItem((*iter).first);
 	}
 }
 
@@ -692,12 +668,7 @@ void AIAbstractList::KeepBelowValue(int32 value)
 
 	for (AIAbstractListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
 		next_iter = iter; next_iter++;
-		if ((*iter).second >= value) this->items.erase(iter);
-	}
-
-	for (AIAbstractListBucket::iterator next_iter, iter = this->buckets.begin(); iter != this->buckets.end(); iter = next_iter) {
-		next_iter = iter; next_iter++;
-		if ((*iter).first >= value) this->buckets.erase(iter);
+		if ((*iter).second >= value) this->RemoveItem((*iter).first);
 	}
 }
 
@@ -707,12 +678,7 @@ void AIAbstractList::KeepBetweenValue(int32 start, int32 end)
 
 	for (AIAbstractListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
 		next_iter = iter; next_iter++;
-		if ((*iter).second <= start || (*iter).second >= end) this->items.erase(iter);
-	}
-
-	for (AIAbstractListBucket::iterator next_iter, iter = this->buckets.begin(); iter != this->buckets.end(); iter = next_iter) {
-		next_iter = iter; next_iter++;
-		if ((*iter).first <= start || (*iter).first >= end) this->buckets.erase(iter);
+		if ((*iter).second <= start || (*iter).second >= end) this->RemoveItem((*iter).first);
 	}
 }
 
@@ -722,12 +688,7 @@ void AIAbstractList::KeepValue(int32 value)
 
 	for (AIAbstractListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
 		next_iter = iter; next_iter++;
-		if ((*iter).second != value) this->items.erase(iter);
-	}
-
-	for (AIAbstractListBucket::iterator next_iter, iter = this->buckets.begin(); iter != this->buckets.end(); iter = next_iter) {
-		next_iter = iter; next_iter++;
-		if ((*iter).first != value) this->buckets.erase(iter);
+		if ((*iter).second != value) this->RemoveItem((*iter).first);
 	}
 }
 

src/ai/api/ai_abstractlist.hpp

@@ -63,6 +63,7 @@ protected:
 	/**
 	 * Remove a single item from the list.
 	 * @param item the item to remove. If not existing, it is ignored.
+	 * @note Always use this function for removing items. It keeps the iterator valid!
 	 */
 	void RemoveItem(int32 item);