Refactored code in Arrays.cpp and fixed save_array() script function.

* related to "corrupted" savegames in #265.

artifacts/scripting/arrays.txt

@@ -212,9 +212,11 @@ use macros sort_array, sort_array_reverse, reverse_array, shuffle_array from sfa
 - always returns 0
 
 > void save_array(mixed key, int arrayID):
+- makes the array saveable; it will be saved in sfallgv.sav file when saving the game
 - arrayID is associated with given "key"
 - array becomes permanent (if it was temporary) and "saved"
 - key can be of any type (int, float or string)
+- if you specify 0 as the key for the array ID, it will make the array "unsaved"
 
 > int load_array(mixed key):
 - load array from savegame data by the same key provided in "save_array"

sfall/Modules/ScriptExtender.cpp

@@ -653,7 +653,7 @@ void SaveGlobals(HANDLE h) {
 static void ClearGlobals() {
 	globalVars.clear();
 	for (array_itr it = arrays.begin(); it != arrays.end(); ++it) {
-		it->second.clear();
+		it->second.clearArrayVar();
 	}
 	arrays.clear();
 	savedArrays.clear();

sfall/Modules/Scripting/Arrays.cpp

@@ -42,9 +42,9 @@ sArrayElement::sArrayElement(const long& other)
 	set(other);
 }
 
-void sArrayElement::clear()
+void sArrayElement::clearData()
 {
-	if (strVal && type == DataType::STR) delete[] strVal;
+	if (type == DataType::STR) delete[] strVal;
 }
 
 void sArrayElement::setByType( DWORD val, DataType dataType )
@@ -64,21 +64,21 @@ void sArrayElement::setByType( DWORD val, DataType dataType )
 
 void sArrayElement::set( long val )
 {
-	clear();
+	clearData();
 	type = DataType::INT;
 	intVal = val;
 }
 
 void sArrayElement::set( float val )
 {
-	clear();
+	clearData();
 	type = DataType::FLOAT;
 	floatVal = val;
 }
 
 void sArrayElement::set( const char* val, int sLen /*= -1*/ )
 {
-	clear();
+	clearData();
 	type = DataType::STR;
 	if (sLen == -1) sLen = strlen(val);
 	if (sLen >= ARRAY_MAX_STRING) sLen = ARRAY_MAX_STRING - 1; // memory safety
@@ -90,7 +90,7 @@ void sArrayElement::set( const char* val, int sLen /*= -1*/ )
 
 void sArrayElement::unset()
 {
-	clear();
+	clearData();
 	type = DataType::NONE;
 	len = intVal = 0;
 }
@@ -99,11 +99,11 @@ DWORD sArrayElement::getHashStatic(DWORD value, DataType type) {
 	switch (type) {
 	case DataType::STR: {
 		const char* str = (const char*)value;
-		DWORD res = 0;
-		for (int i = 0; str[i] != '\0'; i++) {
-			res = ((res << 5) + res) + str[i];
+		DWORD hash = 0;
+		while (*str) {
+			hash = ((hash << 5) + hash) + *str++;
 		}
-		return res;
+		return hash;
 	}
 	case DataType::INT:
 	case DataType::FLOAT:
@@ -150,7 +150,14 @@ void sArrayVar::clearRange( int from, int to /*= -1*/ )
 	if (to == -1) to = val.size();
 	std::vector<sArrayElement>::iterator it, itTo = val.begin() + to;
 	for (it = val.begin() + from; it < itTo; ++it) {
-		it->clear();
+		it->clearData();
+	}
+}
+
+void sArrayVar::clearAll()
+{
+	for (auto it = val.begin(); it != val.end(); ++it) {
+		it->clearData();
 	}
 }
 
@@ -179,6 +186,7 @@ bool LoadArrayElement(sArrayElement* el, HANDLE h)
 			el->strVal = nullptr;
 	} else {
 		ReadFile(h, &el->intVal, 4, &unused, 0);
+		el->len = 0;
 	}
 	return (el->len) ? (unused != el->len) : (unused != 4);
 }
@@ -202,8 +210,8 @@ static long LoadArraysOld(HANDLE h) {
 		var.types = new DWORD[var.len];
 		var.data = new char[var.len * var.datalen];
 
-		ReadFile(h, var.types, (4 * var.len), &unused, 0);
-		ReadFile(h, var.data, (var.len * var.datalen), &unused, 0);
+		ReadFile(h, &var.types, (4 * var.len), &unused, 0);
+		ReadFile(h, &var.data, (var.len * var.datalen), &unused, 0);
 
 		varN.flags = 0;
 		varN.val.resize(var.len);
@@ -250,7 +258,6 @@ long LoadArrays(HANDLE h) {
 			arrayVar.key.intVal = static_cast<long>(arrayVar.key.type);
 			arrayVar.key.type = DataType::INT;
 		}
-
 		ReadFile(h, &arrayVar.flags, 4, &unused, 0);
 		ReadFile(h, &elCount, 4, &unused, 0); // actual number of elements: keys+values
 		if (unused != 4) return -1;
@@ -268,6 +275,7 @@ long LoadArrays(HANDLE h) {
 
 		arrays.insert(array_pair(nextArrayID, arrayVar));
 		savedArrays[arrayVar.key] = nextArrayID++;
+
 		arrayVar.keyHash.clear();
 	}
 	return 0;
@@ -277,29 +285,29 @@ void SaveArrays(HANDLE h) {
 	DWORD elCount, unused, count = 0;
 	WriteFile(h, &count, 4, &unused, 0); // this is for backward compatibility with 3.3-
 
-	array_itr arIt;
+	array_itr arrayIt;
 	ArrayKeysMap::iterator it = savedArrays.begin();
 	while (it != savedArrays.end()) {
-		arIt = arrays.find(it->second);
-		if (arIt == arrays.end()) {
-			savedArrays.erase(it++);
+		arrayIt = arrays.find(it->second); // check the existence of an array
+		if (arrayIt == arrays.end()) {
+			it = savedArrays.erase(it); // array is not found, delete it [fix to C++11 https://en.cppreference.com/w/cpp/container/unordered_map/erase]
 		} else {
 			++count;
 			++it;
 		}
 	}
-	WriteFile(h, &count, 4, &unused, 0);
-
+	WriteFile(h, &count, 4, &unused, 0); // count saved arrays
+	if (count == 0) return;
 	for (it = savedArrays.begin(); it != savedArrays.end(); ++it) {
-		arIt = arrays.find(it->second);
-		if (arIt != arrays.end()) {
+		arrayIt = arrays.find(it->second);
+		if (arrayIt != arrays.end()) {
 			sArrayVar &var = arrays[it->second];
-			SaveArrayElement(&var.key, h);
+			SaveArrayElement(&var.key, h); // type, key/length of key and string of key
 			WriteFile(h, &var.flags, 4, &unused, 0);
 			elCount = var.val.size();
-			WriteFile(h, &elCount, 4, &unused, 0);
-			for (std::vector<sArrayElement>::iterator it = var.val.begin(); it != var.val.end(); ++it) {
-				SaveArrayElement(&(*it), h);
+			WriteFile(h, &elCount, 4, &unused, 0); // number of elements in saved array
+			for (std::vector<sArrayElement>::iterator vIt = var.val.begin(); vIt != var.val.end(); ++vIt) {
+				SaveArrayElement(&(*vIt), h);
 			}
 		}
 	}
@@ -371,7 +379,7 @@ void DESetArray(int id, const DWORD* types, const char* data) {
 	TODO: move somewhere else
 */
 
-DWORD _stdcall CreateArray(int len, DWORD flags) {
+DWORD CreateArray(int len, DWORD flags) {
 	sArrayVar var;
 	var.flags = (flags & 0xFFFFFFFE); // reset 1 bit
 	if (len < 0) {
@@ -395,17 +403,17 @@ DWORD _stdcall CreateArray(int len, DWORD flags) {
 	return nextArrayID++;
 }
 
-DWORD _stdcall TempArray(DWORD len, DWORD flags) {
+DWORD TempArray(DWORD len, DWORD flags) {
 	DWORD id = CreateArray(len, flags);
 	tempArrays.insert(id);
 	return id;
 }
 
-void _stdcall FreeArray(DWORD id) {
+void FreeArray(DWORD id) {
 	array_itr it = arrays.find(id);
 	if (it != arrays.end()) {
-		savedArrays.erase(it->second.key);
-		it->second.clear();
+		if (it->second.key.intVal) savedArrays.erase(it->second.key);
+		it->second.clearArrayVar();
 		arrays.erase(id);
 	}
 }
@@ -419,7 +427,7 @@ void DeleteAllTempArrays() {
 	}
 }
 
-ScriptValue _stdcall GetArrayKey(DWORD id, int index) {
+ScriptValue GetArrayKey(DWORD id, int index) {
 	if (arrays.find(id) == arrays.end() || index < -1 || index > arrays[id].size()) {
 		return ScriptValue(0);
 	}
@@ -446,7 +454,7 @@ ScriptValue _stdcall GetArrayKey(DWORD id, int index) {
 	}
 }
 
-ScriptValue _stdcall GetArray(DWORD id, const ScriptValue& key) {
+ScriptValue GetArray(DWORD id, const ScriptValue& key) {
 	if (arrays.find(id) == arrays.end()) {
 		return 0;
 	}
@@ -478,7 +486,7 @@ ScriptValue _stdcall GetArray(DWORD id, const ScriptValue& key) {
 	return ScriptValue(0);
 }
 
-void _stdcall SetArray(DWORD id, const ScriptValue& key, const ScriptValue& val, bool allowUnset) {
+void SetArray(DWORD id, const ScriptValue& key, const ScriptValue& val, bool allowUnset) {
 	if (arrays.find(id) == arrays.end()) {
 		return;
 	}
@@ -529,7 +537,7 @@ void _stdcall SetArray(DWORD id, const ScriptValue& key, const ScriptValue& val,
 	}
 }
 
-int _stdcall LenArray(DWORD id) {
+int LenArray(DWORD id) {
 	if (arrays.find(id) == arrays.end()) return -1;
 	return arrays[id].size();
 }
@@ -590,12 +598,16 @@ static void MapSort(sArrayVar& arr, int type) {
 	}
 }
 
-long _stdcall ResizeArray(DWORD id, int newlen) {
-	if (newlen == -1 || arrays.find(id) == arrays.end() || arrays[id].size() == newlen) return 0;
+long ResizeArray(DWORD id, int newlen) {
+	if (newlen == -1 || arrays.find(id) == arrays.end()) return 0;
+
 	sArrayVar &arr = arrays[id];
+	int arrSize = arr.size();
+	if (arrSize == newlen) return 0;
+
 	if (arr.isAssoc()) {
 		// only allow to reduce number of elements (adding range of elements is meaningless for maps)
-		if (newlen >= 0 && newlen < arrays[id].size()) {
+		if (newlen >= 0 && newlen < arrSize) {
 			ArrayKeysMap::iterator itHash;
 			std::vector<sArrayElement>::iterator itVal;
 			int actualLen = newlen * 2;
@@ -603,7 +615,10 @@ long _stdcall ResizeArray(DWORD id, int newlen) {
 				if ((itHash = arr.keyHash.find(*itVal)) != arr.keyHash.end())
 					arr.keyHash.erase(itHash);
 			}
-			arr.clearRange(actualLen);
+			if (actualLen == 0)
+				arr.clearAll();
+			else
+				arr.clearRange(actualLen);
 			arr.val.resize(actualLen);
 		} else if (newlen < 0) {
 			if (newlen < (ARRAY_ACTION_SHUFFLE - 2)) return -1;
@@ -612,10 +627,12 @@ long _stdcall ResizeArray(DWORD id, int newlen) {
 		return 0;
 	}
 	if (newlen >= 0) { // actual resize
-		if (newlen > ARRAY_MAX_SIZE) // safety
-			newlen = ARRAY_MAX_SIZE;
-		if (newlen < arr.size())
-			arr.clearRange(newlen);
+		if (newlen == 0) {
+			arr.clearAll();
+		} else {
+			if (newlen > ARRAY_MAX_SIZE) newlen = ARRAY_MAX_SIZE; // safety
+			if (newlen < arrSize) arr.clearRange(newlen);
+		}
 		arr.val.resize(newlen);
 	} else { // special functions for lists...
 		if (newlen < ARRAY_ACTION_SHUFFLE) return -1;
@@ -624,11 +641,11 @@ long _stdcall ResizeArray(DWORD id, int newlen) {
 	return 0;
 }
 
-void _stdcall FixArray(DWORD id) {
+void FixArray(DWORD id) {
 	tempArrays.erase(id);
 }
 
-ScriptValue _stdcall ScanArray(DWORD id, const ScriptValue& val) {
+ScriptValue ScanArray(DWORD id, const ScriptValue& val) {
 	if (arrays.find(id) == arrays.end()) {
 		return ScriptValue(-1);
 	}
@@ -652,7 +669,7 @@ ScriptValue _stdcall ScanArray(DWORD id, const ScriptValue& val) {
 	return ScriptValue(-1);
 }
 
-DWORD _stdcall LoadArray(const ScriptValue& key) {
+DWORD LoadArray(const ScriptValue& key) {
 	if (!key.isInt() || key.asInt() != 0) { // returns arrayId by it's key (ignoring int(0) because it is used to "unsave" array)
 		sArrayElement keyEl = sArrayElement(key.rawValue(), key.type());
 
@@ -678,22 +695,29 @@ DWORD _stdcall LoadArray(const ScriptValue& key) {
 	return 0; // not found
 }
 
-void _stdcall SaveArray(const ScriptValue& key, DWORD id) {
-	array_itr it = arrays.find(id), it2;
-	if (it != arrays.end()) {
+void SaveArray(const ScriptValue& key, DWORD id) {
+	array_itr it, itArray = arrays.find(id); // arrayId => arrayVar
+	if (itArray != arrays.end()) {
 		if (!key.isInt() || key.asInt() != 0) {
 			// make array permanent
-			FixArray(it->first);
+			FixArray(itArray->first);
 			// if another array is saved under the same key, clear it
-			ArrayKeysMap::iterator sIt = savedArrays.find(sArrayElement(key.rawValue(), key.type()));
-			if (sIt != savedArrays.end() && sIt->second != id && (it2 = arrays.find(sIt->second)) != arrays.end()) {
-				it2->second.key.unset();
+			ArrayKeysMap::iterator savedIt = savedArrays.find(sArrayElement(key.rawValue(), key.type()));
+			if (savedIt != savedArrays.end()) {
+				if (savedIt->second == id) return; // exit, array is already saveable
+				// arrays have different ID, search the ID of the "saved" array in collection of arrays
+				if ((it = arrays.find(savedIt->second)) != arrays.end()) {
+					// array exists, delete key value
+					savedArrays.erase(savedIt); /* added for fix */
+					it->second.key.unset();
+				}
 			}
-			it->second.key.setByType(key.rawValue(), key.type());
-			savedArrays[it->second.key] = id;
-		} else { // int(0) is used to "unsave" array without destroying it
-			int num = savedArrays.erase(it->second.key);
-			it->second.key.unset();
+			// make "saved" array
+			itArray->second.key.setByType(key.rawValue(), key.type());
+			savedArrays.emplace(itArray->second.key, id); // savedArrays[itArray->second.key] = id;
+		} else { // key of int(0) is used to "unsave" array without destroying it
+			savedArrays.erase(itArray->second.key);
+			itArray->second.key.unset();
 		}
 	}
 }
@@ -708,7 +732,7 @@ void _stdcall SaveArray(const ScriptValue& key, DWORD id) {
 
 	Should always return 0!
 */
-DWORD _stdcall StackArray(const ScriptValue& key, const ScriptValue& val) {
+long StackArray(const ScriptValue& key, const ScriptValue& val) {
 	DWORD id = stackArrayId;
 	if (id == 0 || arrays.find(id) == arrays.end()) {
 		return 0;

sfall/Modules/Scripting/Arrays.h

@@ -40,7 +40,7 @@ public:
 	sArrayElement(const long&);
 
 	// free string resource from memory, has to be called manually when deleting element
-	void clear();
+	void clearData();
 
 	// set* methods will actually COPY strings, use this when acquiring data from the scripting engine
 	void setByType(DWORD val, DataType dataType);
@@ -107,15 +107,16 @@ public:
 	ArrayKeysMap keyHash; // key element => element index, for faster lookup
 	std::vector<sArrayElement> val; // list of values or key=>value pairs (even - keys, odd - values)
 
+	sArrayVar() : flags(0), key() {}
+
 	bool isAssoc() const {
 		return (flags & ARRAYFLAG_ASSOC);
 	}
 
 	// logical array size (number of elements for normal arrays; number of key=>value pairs for associative)
 	int size() const {
-		return isAssoc()
-			? val.size() / 2
-			: val.size();
+		size_t sz = val.size();
+		return isAssoc() ? sz / 2 : sz;
 	}
 
 	// usefull when filling array from within sfall code (normal lists only)
@@ -125,15 +126,14 @@ public:
 		}
 	}
 
-	sArrayVar() : flags(0), key() {}
-
 	// free memory used by strings
-	void clear() {
-		clearRange(0);
-		key.clear();
+	void clearArrayVar() {
+		clearAll();
+		key.clearData();
 	}
 
 	void clearRange(int from, int to = -1);
+	void clearAll();
 };
 
 // arrays map: arrayId => arrayVar
@@ -160,13 +160,13 @@ void DEGetArray(int id, DWORD* types, char* data);
 void DESetArray(int id, const DWORD* types, const char* data);
 
 // creates new normal (persistent) array. len == -1 specifies associative array (map)
-DWORD _stdcall CreateArray(int len, DWORD flags);
+DWORD CreateArray(int len, DWORD flags);
 
 // same as CreateArray, but creates temporary array instead (will die at the end of the frame)
-DWORD _stdcall TempArray(DWORD len, DWORD flags);
+DWORD TempArray(DWORD len, DWORD flags);
 
 // destroys array
-void _stdcall FreeArray(DWORD id);
+void FreeArray(DWORD id);
 
 // destroy all temp arrays
 void DeleteAllTempArrays();
@@ -174,34 +174,34 @@ void DeleteAllTempArrays();
 /*
 	op_get_array_key can be used to iterate over all keys in associative array
 */
-ScriptValue _stdcall GetArrayKey(DWORD id, int index);
+ScriptValue GetArrayKey(DWORD id, int index);
 
 // get array element by index (list) or key (map)
-ScriptValue _stdcall GetArray(DWORD id, const ScriptValue& key);
+ScriptValue GetArray(DWORD id, const ScriptValue& key);
 
 // set array element by index or key
-void _stdcall SetArray(DWORD id, const ScriptValue& key, const ScriptValue& val, bool allowUnset);
+void SetArray(DWORD id, const ScriptValue& key, const ScriptValue& val, bool allowUnset);
 
 // number of elements in list or pairs in map
-int _stdcall LenArray(DWORD id);
+int LenArray(DWORD id);
 
 // change array size (only works with list)
-long _stdcall ResizeArray(DWORD id, int newlen);
+long ResizeArray(DWORD id, int newlen);
 
 // make temporary array persistent
-void _stdcall FixArray(DWORD id);
+void FixArray(DWORD id);
 
 // searches for a given element in array and returns it's index (for list) or key (for map) or int(-1) if not found
-ScriptValue _stdcall ScanArray(DWORD id, const ScriptValue& val);
+ScriptValue ScanArray(DWORD id, const ScriptValue& val);
 
 // get saved array by it's key
-DWORD _stdcall LoadArray(const ScriptValue& key);
+DWORD LoadArray(const ScriptValue& key);
 
 // make array saved into the savegame with associated key
-void _stdcall SaveArray(const ScriptValue& key, DWORD id);
+void SaveArray(const ScriptValue& key, DWORD id);
 
 // special function that powers array expressions
-DWORD _stdcall StackArray(const ScriptValue& key, const ScriptValue& val);
+long StackArray(const ScriptValue& key, const ScriptValue& val);
 
 }
 }

sfall/Modules/Scripting/Handlers/Arrays.cpp

@@ -30,13 +30,13 @@ namespace script
 {
 
 void sf_create_array(OpcodeContext& ctx) {
-	auto arrayId = CreateArray(ctx.arg(0).asInt(), ctx.arg(1).asInt());
+	auto arrayId = CreateArray(ctx.arg(0).rawValue(), ctx.arg(1).rawValue());
 	ctx.setReturn(arrayId, DataType::INT);
 }
 
 void sf_set_array(OpcodeContext& ctx) {
 	SetArray(
-		ctx.arg(0).asInt(),
+		ctx.arg(0).rawValue(),
 		ctx.arg(1),
 		ctx.arg(2),
 		true
@@ -65,55 +65,55 @@ void sf_get_array(OpcodeContext& ctx) {
 }
 
 void sf_free_array(OpcodeContext& ctx) {
-	FreeArray(ctx.arg(0).asInt());
+	FreeArray(ctx.arg(0).rawValue());
 }
 
 void sf_len_array(OpcodeContext& ctx) {
 	ctx.setReturn(
-		LenArray(ctx.arg(0).asInt())
+		LenArray(ctx.arg(0).rawValue())
 	);
 }
 
 void sf_resize_array(OpcodeContext& ctx) {
-	if (ResizeArray(ctx.arg(0).asInt(), ctx.arg(1).asInt())) {
+	if (ResizeArray(ctx.arg(0).rawValue(), ctx.arg(1).rawValue())) {
 		ctx.printOpcodeError("%s() - array sorting error.", ctx.getOpcodeName());
 	}
 }
 
 void sf_temp_array(OpcodeContext& ctx) {
-	auto arrayId = TempArray(ctx.arg(0).asInt(), ctx.arg(1).asInt());
+	auto arrayId = TempArray(ctx.arg(0).rawValue(), ctx.arg(1).rawValue());
 	ctx.setReturn(arrayId, DataType::INT);
 }
 
 void sf_fix_array(OpcodeContext& ctx) {
-	FixArray(ctx.arg(0).asInt());
+	FixArray(ctx.arg(0).rawValue());
 }
 
 void sf_scan_array(OpcodeContext& ctx) {
 	ctx.setReturn(
-		ScanArray(ctx.arg(0).asInt(), ctx.arg(1))
+		ScanArray(ctx.arg(0).rawValue(), ctx.arg(1))
 	);
 }
 
 void sf_save_array(OpcodeContext& ctx) {
-	SaveArray(ctx.arg(0), ctx.arg(1).asInt());
+	SaveArray(ctx.arg(0), ctx.arg(1).rawValue());
 }
 
 void sf_load_array(OpcodeContext& ctx) {
 	ctx.setReturn(
-		static_cast<int>(LoadArray(ctx.arg(0)))
+		LoadArray(ctx.arg(0))
 	);
 }
 
 void sf_get_array_key(OpcodeContext& ctx) {
 	ctx.setReturn(
-		GetArrayKey(ctx.arg(0).asInt(), ctx.arg(1).asInt())
+		GetArrayKey(ctx.arg(0).rawValue(), ctx.arg(1).rawValue())
 	);
 }
 
 void sf_stack_array(OpcodeContext& ctx) {
 	ctx.setReturn(
-		static_cast<int>(StackArray(ctx.arg(0), ctx.arg(1)))
+		StackArray(ctx.arg(0), ctx.arg(1))
 	);
 }