Remove current entry without invoking Next on iter

This is a more predictable behavior; end-users can call remove and break out early and it will be removed properly.

scripting/include/more_adt.inc

@@ -60,8 +60,9 @@ methodmap StringMultiMapIterator < Handle {
 	public native void SetArray(const any[] array, int num_items);
 
 	/**
-	 * Marks the element at the current iterator position as removed.
-	 * The entry will be removed on the next invocation of Next().
+	 * Removes the entry at the current iterator position.  The iterator is not advanced.
+	 * Attempts to call getters / setters on a removed entry will throw an error.
+	 * Subsequent calls to `Remove()` on the same entry will not throw any error.
 	 */
 	public native void Remove();
 }

smsdk_config.h

@@ -40,7 +40,7 @@
 /* Basic information exposed publicly */
 #define SMEXT_CONF_NAME			"More ADTs"
 #define SMEXT_CONF_DESCRIPTION	"Provides additional data structures to SourceMod."
-#define SMEXT_CONF_VERSION		"0.1.5"
+#define SMEXT_CONF_VERSION		"0.1.6"
 #define SMEXT_CONF_AUTHOR		"nosoop"
 #define SMEXT_CONF_URL			"http://github.com/nosoop/SMExt-MoreADTs"
 #define SMEXT_CONF_LOGTAG		"mADT"

types/iterator_container.h

@@ -7,20 +7,28 @@ template <typename T>
 class IteratorContainer {
 	public:
 	IteratorContainer(T* source, typename T::iterator begin, typename T::iterator end):
-			_struct(source), _it(begin), _end(end), _removed(false) {};
+			_struct(source), _it(begin), _end(end), _current_removed(false) {};
 
 	bool Next() {
-		// TODO implement a way to mark the current entry as deleted
-		if (_removed) {
+		if (_current != _end) {
+			// _current is assigned before _it advances
+			_current = _it++;
+			_current_removed = false;
+
+			return _current != _end;
+		}
+		return false;
+	}
+
+	void Remove() {
+		if (!_current_removed) {
+			_current_removed = true;
 			_struct->erase(_current);
-			_removed = false;
 		}
-		_current = _it;
-		return _it++ != _end;
 	}
 
-	void MarkRemoved() {
-		_removed = true;
+	bool IsRemoved() {
+		return _current_removed;
 	}
 
 	typename T::iterator Current() {
@@ -30,5 +38,5 @@ class IteratorContainer {
 	private:
 	T* _struct;
 	typename T::iterator _current, _it, _end;
-	bool _removed;
+	bool _current_removed;
 };

types/string_multimap.cpp

@@ -284,6 +284,10 @@ cell_t sm_GetStringMultiMapIteratorKey(IPluginContext *pContext, const cell_t *p
 		return pContext->ThrowNativeError("Invalid StringMultiMapIterator handle %x (error %d)", hndl, err);
 	}
 
+	if (pMultiMapIter->IsRemoved()) {
+		return pContext->ThrowNativeError("Can't use getter on removed entry in StringMultiMapIterator handle %x", hndl);
+	}
+
 	pContext->StringToLocal(params[2], params[3], pMultiMapIter->Current()->first.c_str());
 
 	return 0;
@@ -299,6 +303,10 @@ cell_t sm_GetStringMultiMapIteratorString(IPluginContext *pContext, const cell_t
 		return pContext->ThrowNativeError("Invalid StringMultiMapIterator handle %x (error %d)", hndl, err);
 	}
 
+	if (pMultiMapIter->IsRemoved()) {
+		return pContext->ThrowNativeError("Can't use getter on removed entry in StringMultiMapIterator handle %x", hndl);
+	}
+
 	if (auto pval = mpark::get_if<std::string>(&pMultiMapIter->Current()->second)) {
 		pContext->StringToLocal(params[2], params[3], pval->c_str());
 		return true;
@@ -317,6 +325,10 @@ cell_t sm_SetStringMultiMapIteratorString(IPluginContext *pContext, const cell_t
 		return pContext->ThrowNativeError("Invalid StringMultiMapIterator handle %x (error %d)", hndl, err);
 	}
 
+	if (pMultiMapIter->IsRemoved()) {
+		return pContext->ThrowNativeError("Can't use getter on removed entry in StringMultiMapIterator handle %x", hndl);
+	}
+
 	char* val;
 	pContext->LocalToString(params[2], &val);
 
@@ -335,6 +347,10 @@ cell_t sm_GetStringMultiMapIteratorValue(IPluginContext *pContext, const cell_t
 		return pContext->ThrowNativeError("Invalid StringMultiMapIterator handle %x (error %d)", hndl, err);
 	}
 
+	if (pMultiMapIter->IsRemoved()) {
+		return pContext->ThrowNativeError("Can't use getter on removed entry in StringMultiMapIterator handle %x", hndl);
+	}
+
 	if (auto pval = mpark::get_if<cell_t>(&pMultiMapIter->Current()->second)) {
 		cell_t* val;
 		pContext->LocalToPhysAddr(params[2], &val);
@@ -356,6 +372,10 @@ cell_t sm_SetStringMultiMapIteratorValue(IPluginContext *pContext, const cell_t
 		return pContext->ThrowNativeError("Invalid StringMultiMapIterator handle %x (error %d)", hndl, err);
 	}
 
+	if (pMultiMapIter->IsRemoved()) {
+		return pContext->ThrowNativeError("Can't use setter on removed entry in StringMultiMapIterator handle %x", hndl);
+	}
+
 	cell_t val = params[2];
 
 	pMultiMapIter->Current()->second = val;
@@ -373,6 +393,10 @@ cell_t sm_GetStringMultiMapIteratorArray(IPluginContext *pContext, const cell_t
 		return pContext->ThrowNativeError("Invalid StringMultiMapIterator handle %x (error %d)", hndl, err);
 	}
 
+	if (pMultiMapIter->IsRemoved()) {
+		return pContext->ThrowNativeError("Can't use getter on removed entry in StringMultiMapIterator handle %x", hndl);
+	}
+
 	if (auto pval = mpark::get_if<std::vector<cell_t>>(&pMultiMapIter->Current()->second)) {
 		cell_t* array;
 		pContext->LocalToPhysAddr(params[2], &array);
@@ -404,6 +428,10 @@ cell_t sm_SetStringMultiMapIteratorArray(IPluginContext *pContext, const cell_t
 		return pContext->ThrowNativeError("Invalid StringMultiMapIterator handle %x (error %d)", hndl, err);
 	}
 
+	if (pMultiMapIter->IsRemoved()) {
+		return pContext->ThrowNativeError("Can't use setter on removed entry in StringMultiMapIterator handle %x", hndl);
+	}
+
 	cell_t* array;
 	pContext->LocalToPhysAddr(params[2], &array);
 
@@ -429,7 +457,7 @@ cell_t sm_RemoveOnStringMultiMapIterator(IPluginContext *pContext, const cell_t
 		return pContext->ThrowNativeError("Invalid StringMultiMapIterator handle %x (error %d)", hndl, err);
 	}
 
-	pMultiMapIter->MarkRemoved();
+	pMultiMapIter->Remove();
 
 	return 0;
 }