18755: Fixes inconsistencies with null handling, primarily around str…

…ing conversions and random number seeding (#46)

src/Amalgam/AssetManager.cpp

@@ -164,7 +164,7 @@ bool AssetManager::StoreResourcePath(EvaluableNode *code, std::string &resource_
 		if(code != nullptr)
 		{
 			for(auto &cn : code->GetOrderedChildNodes())
-				string_map[EvaluableNode::ToString(cn)] = cur_index++;
+				string_map[EvaluableNode::ToStringPreservingOpcodeType(cn)] = cur_index++;
 		}
 
 		//compress and store
@@ -191,7 +191,7 @@ bool AssetManager::StoreResourcePath(EvaluableNode *code, std::string &resource_
 	}
 	else //binary string
 	{
-		std::string s = EvaluableNode::ToString(code);
+		std::string s = EvaluableNode::ToStringPreservingOpcodeType(code);
 		if(StoreFileFromBuffer<std::string>(processed_resource_path, s))
 			return EvaluableNodeReference(enm->AllocNode(ENT_TRUE), true);
 		else
@@ -226,7 +226,7 @@ Entity *AssetManager::LoadEntityFromResourcePath(std::string &resource_path, std
 		{
 			EvaluableNode **seed = metadata->GetMappedChildNode(ENBISI_rand_seed);
 			if(seed != nullptr)
-				default_random_seed = EvaluableNode::ToString(*seed);
+				default_random_seed = EvaluableNode::ToStringPreservingOpcodeType(*seed);
 		}
 	}
 

src/Amalgam/Parser.cpp

@@ -792,7 +792,7 @@ void Parser::Unparse(UnparseData &upd, EvaluableNode *tree, EvaluableNode *paren
 		switch(tree->GetType())
 		{
 		case ENT_NUMBER:
-			upd.result.append(EvaluableNode::ToString(tree));
+			upd.result.append(EvaluableNode::ToStringPreservingOpcodeType(tree));
 			break;
 		case ENT_STRING:
 		{

src/Amalgam/entity/EntityExternalInterface.cpp

@@ -154,7 +154,7 @@ void EntityExternalInterface::AppendToLabel(std::string &handle, std::string &la
 		//wrap the existing and new element in a list
 		//can use local stack instead of heap because the entity will copy anyway
 		EvaluableNode list(ENT_LIST);
-		EvaluableNode initial_value(ENT_STRING, EvaluableNode::ToString(label_val));
+		EvaluableNode initial_value(ENT_STRING, EvaluableNode::ToStringPreservingOpcodeType(label_val));
 		EvaluableNode parsed_input(ENT_STRING, value);
 		list.AppendOrderedChildNode(&initial_value);
 		list.AppendOrderedChildNode(&parsed_input);
@@ -209,7 +209,7 @@ std::string EntityExternalInterface::GetString(std::string &handle, std::string
 	EvaluableNode *label_val = bundle->entity->GetValueAtLabel(label, &bundle->entity->evaluableNodeManager, false);
 
 	//Ensure you grab the return value before releasing resources
-	std::string ret = EvaluableNode::ToString(label_val);
+	std::string ret = EvaluableNode::ToStringPreservingOpcodeType(label_val);
 	return ret;
 }
 
@@ -227,11 +227,11 @@ std::string EntityExternalInterface::GetStringFromList(std::string &handle, std:
 	{
 		auto &children = label_val->GetOrderedChildNodes();
 		if(index < children.size())
-			ret = EvaluableNode::ToString(children[index]);
+			ret = EvaluableNode::ToStringPreservingOpcodeType(children[index]);
 	}
 	else
 	{
-		ret = EvaluableNode::ToString(label_val);
+		ret = EvaluableNode::ToStringPreservingOpcodeType(label_val);
 	}
 
 	return ret;
@@ -476,11 +476,11 @@ void EntityExternalInterface::GetStringList(std::string &handle, std::string &la
 		auto &children = label_val->GetOrderedChildNodes();
 		size_t min = std::min(children.size(), len);
 		for(size_t i = 0; i < min; i++)
-			out_arr[i] = EvaluableNode::ToString(children[i]);
+			out_arr[i] = EvaluableNode::ToStringPreservingOpcodeType(children[i]);
 	}
 	else
 	{
-		out_arr[0] = EvaluableNode::ToString(label_val);
+		out_arr[0] = EvaluableNode::ToStringPreservingOpcodeType(label_val);
 	}
 }
 

src/Amalgam/entity/EntityQueries.h

@@ -91,10 +91,10 @@ class EntityQueryCondition
 	//ENT_QUERY_SELECT's value of the start offset
 	size_t startOffset;
 
-	//if ENT_QUERY_SELECT or ENT_QUERY_SAMPLE has a random stream
+	//whether ENT_QUERY_SELECT or ENT_QUERY_SAMPLE has a random stream; if not, it will use consistent order
 	bool hasRandomStream;
 
-	//ENT_QUERY_SELECT's or ENT_QUERY_SAMPLE's random stream
+	//the random stream for queries that use it
 	RandomStream randomStream;
 
 	//includes zero as a valid difference for ENT_QUERY_MIN_DIFFERENCE

src/Amalgam/entity/EntityQueryBuilder.h

@@ -190,7 +190,8 @@ namespace EntityQueryBuilder
 
 
 	//interpret evaluable node as a distance query
-	inline void BuildDistanceCondition(EvaluableNode *cn, EvaluableNodeType condition_type, std::vector<EntityQueryCondition> &conditions)
+	inline void BuildDistanceCondition(EvaluableNode *cn, EvaluableNodeType condition_type,
+		std::vector<EntityQueryCondition> &conditions, RandomStream &rs)
 	{
 		//cache ordered child nodes so don't need to keep fetching
 		auto &ocn = cn->GetOrderedChildNodes();
@@ -348,10 +349,11 @@ namespace EntityQueryBuilder
 			cur_condition->weightLabel = EvaluableNode::ToStringIDIfExists(ocn[ENTITY_WEIGHT_LABEL_NAME]);
 
 		//set random seed
-		std::string seed = "";
-		if(ocn.size() > RANDOM_SEED)
-			seed = EvaluableNode::ToString(ocn[RANDOM_SEED]);
-		cur_condition->randomStream.SetState(seed);
+		cur_condition->hasRandomStream = (ocn.size() > RANDOM_SEED && !EvaluableNode::IsEmptyNode(ocn[RANDOM_SEED]));
+		if(cur_condition->hasRandomStream)
+			cur_condition->randomStream.SetState(EvaluableNode::ToStringPreservingOpcodeType(ocn[RANDOM_SEED]));
+		else
+			cur_condition->randomStream = rs.CreateOtherStreamViaRand();
 
 		//set radius label
 		if(ocn.size() > RADIUS_LABEL)
@@ -431,7 +433,7 @@ namespace EntityQueryBuilder
 
 	//builds a query condition from cn
 	inline void BuildNonDistanceCondition(EvaluableNode *cn, EvaluableNodeType type,
-		std::vector<EntityQueryCondition> &conditions, EvaluableNodeManager &enm, RandomStream &rs)
+		std::vector<EntityQueryCondition> &conditions, RandomStream &rs)
 	{
 		auto &ocn = cn->GetOrderedChildNodes();
 
@@ -546,7 +548,9 @@ namespace EntityQueryBuilder
 
 				cur_condition->hasRandomStream = (ocn.size() >= 3 && !EvaluableNode::IsEmptyNode(ocn[2]));
 				if(cur_condition->hasRandomStream)
-					cur_condition->randomStream.SetState(EvaluableNode::ToString(ocn[2]));
+					cur_condition->randomStream.SetState(EvaluableNode::ToStringPreservingOpcodeType(ocn[2]));
+				else
+					cur_condition->randomStream = rs.CreateOtherStreamViaRand();
 
 				break;
 			}
@@ -555,7 +559,7 @@ namespace EntityQueryBuilder
 				cur_condition->maxToRetrieve = (ocn.size() > 0) ? EvaluableNode::ToNumber(ocn[0], 0.0) : 1;
 				cur_condition->hasRandomStream = (ocn.size() > 1 && !EvaluableNode::IsEmptyNode(ocn[1]));
 				if(cur_condition->hasRandomStream)
-					cur_condition->randomStream.SetState(EvaluableNode::ToString(ocn[1]));
+					cur_condition->randomStream.SetState(EvaluableNode::ToStringPreservingOpcodeType(ocn[1]));
 				else
 					cur_condition->randomStream = rs.CreateOtherStreamViaRand();
 			    break;
@@ -566,7 +570,7 @@ namespace EntityQueryBuilder
 				cur_condition->maxToRetrieve = (ocn.size() > 1) ? EvaluableNode::ToNumber(ocn[1], 0.0) : 1;
 				cur_condition->hasRandomStream = (ocn.size() > 2 && !EvaluableNode::IsEmptyNode(ocn[2]));
 				if(cur_condition->hasRandomStream)
-					cur_condition->randomStream.SetState(EvaluableNode::ToString(ocn[2]));
+					cur_condition->randomStream.SetState(EvaluableNode::ToStringPreservingOpcodeType(ocn[2]));
 				else
 					cur_condition->randomStream = rs.CreateOtherStreamViaRand();
 				break;

src/Amalgam/evaluablenode/EvaluableNode.cpp

@@ -195,8 +195,8 @@ int EvaluableNode::Compare(EvaluableNode *a, EvaluableNode *b)
 			return 0;
 	}
 
-	std::string a_str = EvaluableNode::ToString(a);
-	std::string b_str = EvaluableNode::ToString(b);
+	std::string a_str = EvaluableNode::ToStringPreservingOpcodeType(a);
+	std::string b_str = EvaluableNode::ToStringPreservingOpcodeType(b);
 	return StringManipulation::StringNaturalCompare(a_str, b_str);
 }
 
@@ -232,7 +232,7 @@ double EvaluableNode::ToNumber(EvaluableNode *e, double value_if_null)
 	}
 }
 
-const std::string EvaluableNode::ToString(EvaluableNode *e)
+const std::string EvaluableNode::ToStringPreservingOpcodeType(EvaluableNode *e)
 {
 	if(e == nullptr)
 		return "null";
@@ -251,6 +251,27 @@ const std::string EvaluableNode::ToString(EvaluableNode *e)
 	}
 }
 
+std::pair<bool, std::string> EvaluableNode::ToString(EvaluableNode *e)
+{
+	if(IsEmptyNode(e))
+		return std::make_pair(false, ".nas");
+
+	switch(e->GetType())
+	{
+	case ENT_NUMBER:
+		return std::make_pair(true, NumberToString(e->GetNumberValueReference()));
+
+	case ENT_STRING:
+	case ENT_SYMBOL:
+		return std::make_pair(true, e->GetStringValue());
+
+	default:
+		return std::make_pair(true, GetStringFromEvaluableNodeType(e->GetType()));
+	}
+
+	return std::make_pair(true, "");
+}
+
 StringInternPool::StringID EvaluableNode::ToStringIDIfExists(EvaluableNode *e)
 {
 	if(e == nullptr)
@@ -264,7 +285,7 @@ StringInternPool::StringID EvaluableNode::ToStringIDIfExists(EvaluableNode *e)
 		return StringInternPool::NOT_A_STRING_ID;
 
 	//see if the string exists even if it is not stored as a StringID
-	const std::string str_value = ToString(e);
+	const std::string str_value = ToStringPreservingOpcodeType(e);
 	//will return empty string if not found
 	return string_intern_pool.GetIDFromString(str_value);
 }
@@ -281,7 +302,7 @@ StringInternPool::StringID EvaluableNode::ToStringIDWithReference(EvaluableNode
 	if(IsNaN(e))
 		return StringInternPool::NOT_A_STRING_ID;
 
-	std::string stringified = ToString(e);
+	std::string stringified = ToStringPreservingOpcodeType(e);
 	return string_intern_pool.CreateStringReference(stringified);
 }
 
@@ -300,7 +321,7 @@ StringInternPool::StringID EvaluableNode::ToStringIDTakingReferenceAndClearing(E
 		return sid_to_return;
 	}
 
-	std::string stringified = ToString(e);
+	std::string stringified = ToStringPreservingOpcodeType(e);
 	return string_intern_pool.CreateStringReference(stringified);
 }
 

src/Amalgam/evaluablenode/EvaluableNode.h

@@ -364,8 +364,11 @@ class EvaluableNode
 		return StringManipulation::NumberToString(value);
 	}
 
-	//Converts the node to a string
-	const static std::string ToString(EvaluableNode *e);
+	//converts the node to a string that represents the opcode
+	const static std::string ToStringPreservingOpcodeType(EvaluableNode *e);
+
+	//converts the node to a string, returning true if valid.  If it doesn't exist, or any form of null/NaN/NaS, it returns false
+	static std::pair<bool, std::string> ToString(EvaluableNode *e);
 
 	//converts node to an existing string. If it doesn't exist, or any form of null/NaN/NaS, it returns NOT_A_STRING_ID
 	static StringInternPool::StringID ToStringIDIfExists(EvaluableNode *e);
@@ -915,7 +918,7 @@ class EvaluableNode
 // concrete values can be stored for the EvaluableNode.  It is intended to
 // group types into the highest specificity that it is worth using to
 // compare two values based on their collective types
-enum EvaluableNodeImmediateValueType
+enum EvaluableNodeImmediateValueType : uint8_t
 {
 	ENIVT_NOT_EXIST,			//there is nothing to even hold the data
 	ENIVT_NULL,					//no data being held
@@ -949,9 +952,10 @@ union EvaluableNodeImmediateValue
 		: code(eniv.code)
 	{	}
 
-	constexpr EvaluableNodeImmediateValue &operator =(const EvaluableNodeImmediateValue &eniv)
+	__forceinline EvaluableNodeImmediateValue &operator =(const EvaluableNodeImmediateValue &eniv)
 	{
-		code = eniv.code;
+		//perform a memcpy because it's a union, to be safe; the compiler should optimize this out
+		std::memcpy(this, &eniv, sizeof(*this));
 		return *this;
 	}
 
@@ -1044,7 +1048,7 @@ class EvaluableNodeImmediateValueWithType
 		: nodeType(enimvwt.nodeType), nodeValue(enimvwt.nodeValue)
 	{	}
 
-	constexpr EvaluableNodeImmediateValueWithType &operator =(const EvaluableNodeImmediateValueWithType &enimvwt)
+	__forceinline EvaluableNodeImmediateValueWithType &operator =(const EvaluableNodeImmediateValueWithType &enimvwt)
 	{
 		nodeType = enimvwt.nodeType;
 		nodeValue = enimvwt.nodeValue;

src/Amalgam/evaluablenode/EvaluableNodeTreeFunctions.cpp

@@ -394,10 +394,13 @@ EvaluableNodeReference AccumulateEvaluableNodeIntoEvaluableNode(EvaluableNodeRef
 		}
 		else if(value_destination_node->IsStringValue())
 		{
-			std::string cur_value = EvaluableNode::ToString(value_destination_node);
-			std::string inc_value = EvaluableNode::ToString(variable_value_node);
+			auto [cur_value_valid, cur_value] = EvaluableNode::ToString(value_destination_node);
+			auto [inc_value_valid, inc_value] = EvaluableNode::ToString(variable_value_node);
 			value_destination_node->SetType(ENT_STRING, enm);
-			value_destination_node->SetStringValue(cur_value.append(inc_value));
+			//string will default to invalid value -- only set if both strings are valid
+			if(cur_value_valid && inc_value_valid)
+				value_destination_node->SetStringValue(cur_value.append(inc_value));
+
 			value_destination_node.unique = true;
 		}
 		else //add ordered child node
@@ -476,9 +479,13 @@ EvaluableNodeReference AccumulateEvaluableNodeIntoEvaluableNode(EvaluableNodeRef
 	}
 	else if(value_destination_node->IsStringValue())
 	{
-		std::string cur_value = EvaluableNode::ToString(value_destination_node);
-		std::string inc_value = EvaluableNode::ToString(variable_value_node);
-		value_destination_node.reference = enm->AllocNode(ENT_STRING, cur_value.append(inc_value));
+		auto [cur_value_valid, cur_value] = EvaluableNode::ToString(value_destination_node);
+		auto [inc_value_valid, inc_value] = EvaluableNode::ToString(variable_value_node);
+		value_destination_node->SetType(ENT_STRING, enm);
+		//string will default to invalid value -- only set if both strings are valid
+		if(cur_value_valid && inc_value_valid)
+			value_destination_node->SetStringValue(cur_value.append(inc_value));
+
 		value_destination_node.unique = true;
 	}
 	else //add ordered child node

src/Amalgam/importexport/FileSupportCSV.cpp

@@ -194,7 +194,7 @@ bool FileSupportCSV::Store(EvaluableNode *code, const std::string &resource_path
 				if(EvaluableNode::IsEmptyNode(column_node))
 					continue;
 
-				std::string original_string = EvaluableNode::ToString(column_node);
+				std::string original_string = EvaluableNode::ToStringPreservingOpcodeType(column_node);
 				std::string escaped_str = EscapeCSVStringIfNeeded(original_string);
 				data_string.append(escaped_str);
 			}

src/Amalgam/interpreter/Interpreter.cpp

@@ -545,7 +545,13 @@ std::pair<bool, std::string> Interpreter::InterpretNodeIntoStringValue(Evaluable
 		return std::make_pair(true, n->GetStringValue());
 
 	auto result = InterpretNodeForImmediateUse(n);
-	std::string result_string = EvaluableNode::ToString(result);
+	if(EvaluableNode::IsEmptyNode(n))
+	{
+		evaluableNodeManager->FreeNodeTreeIfPossible(result);
+		return std::make_pair(false, "");
+	}
+
+	std::string result_string = EvaluableNode::ToStringPreservingOpcodeType(result);
 	evaluableNodeManager->FreeNodeTreeIfPossible(result);
 
 	return std::make_pair(true, result_string);
@@ -565,7 +571,7 @@ std::string Interpreter::InterpretNodeIntoStringValueEmptyNull(EvaluableNode *n)
 	if(EvaluableNode::IsEmptyNode(result))
 		return "";
 
-	std::string result_string = EvaluableNode::ToString(result);
+	std::string result_string = EvaluableNode::ToStringPreservingOpcodeType(result);
 	evaluableNodeManager->FreeNodeTreeIfPossible(result);
 
 	return result_string;

src/Amalgam/interpreter/InterpreterOpcodesDataTypes.cpp

@@ -532,12 +532,12 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_FORMAT(EvaluableNode *en)
 					auto &mcn = from_params->GetMappedChildNodesReference();
 
 					auto found_locale = mcn.find(ENBISI_locale);
-					if(found_locale != end(mcn))
-						locale = EvaluableNode::ToString(found_locale->second);
+					if(found_locale != end(mcn) && !EvaluableNode::IsEmptyNode(found_locale->second))
+						locale = EvaluableNode::ToStringPreservingOpcodeType(found_locale->second);
 
 					auto found_timezone = mcn.find(ENBISI_timezone);
-					if(found_timezone != end(mcn))
-						timezone = EvaluableNode::ToString(found_timezone->second);
+					if(found_timezone != end(mcn) && !EvaluableNode::IsEmptyNode(found_timezone->second))
+						timezone = EvaluableNode::ToStringPreservingOpcodeType(found_timezone->second);
 				}
 
 				use_number = true;
@@ -859,12 +859,12 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_FORMAT(EvaluableNode *en)
 				auto &mcn = to_params->GetMappedChildNodesReference();
 
 				auto found_locale = mcn.find(ENBISI_locale);
-				if(found_locale != end(mcn))
-					locale = EvaluableNode::ToString(found_locale->second);
+				if(found_locale != end(mcn) && !EvaluableNode::IsEmptyNode(found_locale->second))
+					locale = EvaluableNode::ToStringPreservingOpcodeType(found_locale->second);
 
 				auto found_timezone = mcn.find(ENBISI_timezone);
-				if(found_timezone != end(mcn))
-					timezone = EvaluableNode::ToString(found_timezone->second);
+				if(found_timezone != end(mcn) && !EvaluableNode::IsEmptyNode(found_timezone->second))
+					timezone = EvaluableNode::ToStringPreservingOpcodeType(found_timezone->second);
 			}
 
 			double num_secs_from_epoch = 0.0;
@@ -1842,7 +1842,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_PRINT(EvaluableNode *en)
 			else if(DoesEvaluableNodeTypeUseNumberData(cur->GetType()))
 				s = EvaluableNode::NumberToString(cur->GetNumberValueReference());
 			else
-				s = EvaluableNode::ToString(cur);
+				s = EvaluableNode::ToStringPreservingOpcodeType(cur);
 		}
 		else
 		{