21187: Fixes bugs around constraining entity creation and mutating co…

…de (#218)

src/Amalgam/entity/Entity.h

@@ -557,9 +557,10 @@ class Entity
 	//returns the thread_local static variable entity[Read|Write]ReferenceBuffer, so results will be invalidated
 	//by subsequent calls
 	//if include_this_entity is true, it will include the entity in the references
+	//if exclude_entity is not nullptr, it will not include it, for example, if it's already locked
 	template<typename EntityReferenceType>
 	inline EntityReferenceBufferReference<EntityReferenceType> GetAllDeeplyContainedEntityReferencesGroupedByDepth(
-		bool include_this_entity = false)
+		bool include_this_entity = false, Entity *exclude_entity = nullptr)
 	{
 		EntityReferenceBufferReference<EntityReferenceType> erbr;
 		if constexpr(std::is_same<EntityReferenceType, EntityWriteReference>::value)
@@ -571,16 +572,21 @@ class Entity
 
 		if(include_this_entity)
 		{
-			if constexpr(std::is_same<EntityReferenceType, EntityWriteReference>::value)
-				entityWriteReferenceBuffer.emplace_back(this);
-			else
-				entityReadReferenceBuffer.emplace_back(this);
+			//don't put the entity in the buffer if it's excluded,
+			// as it should already have a lock, but include it in the count below
+			if(this != exclude_entity)
+			{
+				if constexpr(std::is_same<EntityReferenceType, EntityWriteReference>::value)
+					entityWriteReferenceBuffer.emplace_back(this);
+				else
+					entityReadReferenceBuffer.emplace_back(this);
+			}
 
 			erbr.maxEntityPathDepth++;
 		}
 
 		size_t max_depth = 0;
-		GetAllDeeplyContainedEntityReferencesGroupedByDepthRecurse<EntityReferenceType>(0, max_depth);
+		GetAllDeeplyContainedEntityReferencesGroupedByDepthRecurse<EntityReferenceType>(0, max_depth, exclude_entity);
 		erbr.maxEntityPathDepth += max_depth;
 		return erbr;
 	}
@@ -590,7 +596,7 @@ class Entity
 	void AppendAllDeeplyContainedEntityReferencesGroupedByDepth(EntityReferenceBufferReference<EntityReferenceType> &erbr)
 	{
 		size_t max_depth = 0;
-		GetAllDeeplyContainedEntityReferencesGroupedByDepthRecurse<EntityReferenceType>(0, max_depth);
+		GetAllDeeplyContainedEntityReferencesGroupedByDepthRecurse<EntityReferenceType>(0, max_depth, nullptr);
 		erbr.maxEntityPathDepth += max_depth;
 	}
 
@@ -757,7 +763,8 @@ class Entity
 
 	//helper function for GetAllDeeplyContainedEntityReadReferencesGroupedByDepth
 	template<typename EntityReferenceType>
-	bool GetAllDeeplyContainedEntityReferencesGroupedByDepthRecurse(size_t cur_depth, size_t &max_depth)
+	bool GetAllDeeplyContainedEntityReferencesGroupedByDepthRecurse(
+		size_t cur_depth, size_t &max_depth, Entity *exclude_entity)
 	{
 		if(cur_depth > max_depth)
 			max_depth = cur_depth;
@@ -774,6 +781,9 @@ class Entity
 		auto &contained_entities = GetContainedEntities();
 		for(Entity *e : contained_entities)
 		{
+			if(e == exclude_entity)
+				continue;
+
 			if constexpr(std::is_same<EntityReferenceType, EntityWriteReference>::value)
 				entityWriteReferenceBuffer.emplace_back(e);
 			else
@@ -782,7 +792,8 @@ class Entity
 
 		for(auto &ce : contained_entities)
 		{
-			if(!ce->GetAllDeeplyContainedEntityReferencesGroupedByDepthRecurse<EntityReferenceType>(cur_depth + 1, max_depth))
+			if(!ce->GetAllDeeplyContainedEntityReferencesGroupedByDepthRecurse<EntityReferenceType>(cur_depth + 1,
+					max_depth, exclude_entity))
 				return false;
 		}
 

src/Amalgam/evaluablenode/EvaluableNodeManagement.h

@@ -858,9 +858,14 @@ class EvaluableNodeManager
 		// however, this should be rarely called on those entities since it's basically clearing them out, so it should not generally be a performance issue
 		auto location = std::find(begin(nodes), begin(nodes) + firstUnusedNodeIndex, new_root);
 
+		if(location == end(nodes))
+		{
+			assert(false);
+			return;
+		}
+
 		//put the new root in the proper place
-		if(location != end(nodes))
-			std::swap(*begin(nodes), *location);
+		std::swap(*begin(nodes), *location);
 	}
 
 	//returns true if any node is referenced other than root, which is an indication if there are

src/Amalgam/evaluablenode/EvaluableNodeTreeManipulation.cpp

@@ -1505,7 +1505,7 @@ void MutateImmediateNode(EvaluableNode *n, RandomStream &rs, std::vector<std::st
 EvaluableNode *EvaluableNodeTreeManipulation::MutateNode(EvaluableNode *n, MutationParameters &mp)
 {
 	if(n == nullptr)
-		return nullptr;
+		n = mp.enm->AllocNode(ENT_NULL);
 
 	//if immediate type (after initial mutation), see if should mutate value
 	bool is_immediate = n->IsImmediate();
@@ -1583,7 +1583,7 @@ EvaluableNode *EvaluableNodeTreeManipulation::MutateNode(EvaluableNode *n, Mutat
 		}
 
 		case ENBISI_swap_elements:
-			if(n->GetOrderedChildNodes().size() > 0)
+			if(n->GetOrderedChildNodes().size() > 1)
 			{
 				size_t num_child_nodes = n->GetOrderedChildNodesReference().size();
 				auto first_index = mp.interpreter->randomStream.RandSize(num_child_nodes);
@@ -1714,13 +1714,14 @@ EvaluableNode *EvaluableNodeTreeManipulation::MutateNode(EvaluableNode *n, Mutat
 
 EvaluableNode *EvaluableNodeTreeManipulation::MutateTree(MutationParameters &mp, EvaluableNode *tree)
 {
-	if(tree == nullptr)
-		return nullptr;
-
-	//if this object has already been copied, then just return the reference to the new copy
-	auto found_copy = mp.references.find(tree);
-	if(found_copy != end(mp.references))
-		return found_copy->second;
+	//if it's nullptr, then move on to making a copy, otherwise see if it's already been copied
+	if(tree != nullptr)
+	{
+		//if this object has already been copied, then just return the reference to the new copy
+		auto found_copy = mp.references.find(tree);
+		if(found_copy != end(mp.references))
+			return found_copy->second;
+	}
 
 	EvaluableNode *copy = mp.enm->AllocNode(tree);
 	auto node_stack = mp.interpreter->CreateInterpreterNodeStackStateSaver(copy);
@@ -1753,10 +1754,7 @@ EvaluableNode *EvaluableNodeTreeManipulation::MutateTree(MutationParameters &mp,
 		//for any mapped children, copy and update
 		for(auto &[_, s] : copy->GetMappedChildNodesReference())
 		{
-			//get current item in list
 			EvaluableNode *n = s;
-			if(n == nullptr)
-				continue;
 
 			//turn into a copy and mutate
 			n = MutateTree(mp, n);
@@ -1773,8 +1771,6 @@ EvaluableNode *EvaluableNodeTreeManipulation::MutateTree(MutationParameters &mp,
 		{
 			//get current item in list
 			EvaluableNode *n = ocn[i];
-			if(n == nullptr)
-				continue;
 
 			//turn into a copy and mutate
 			n = MutateTree(mp, n);

src/Amalgam/interpreter/Interpreter.h

@@ -900,7 +900,9 @@ class Interpreter
 		if(!performanceConstraints->constrainMaxContainedEntities && !performanceConstraints->constrainMaxContainedEntityDepth)
 			return true;
 
-		auto erbr = performanceConstraints->entityToConstrainFrom->GetAllDeeplyContainedEntityReferencesGroupedByDepth<EntityReadReference>();
+		auto erbr
+			= performanceConstraints->entityToConstrainFrom->GetAllDeeplyContainedEntityReferencesGroupedByDepth<
+			EntityReadReference>(true, destination_container);
 
 		if(performanceConstraints->constrainMaxContainedEntities)
 		{

src/Amalgam/interpreter/InterpreterOpcodesEntityControl.cpp

@@ -360,7 +360,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CREATE_ENTITIES(EvaluableN
 			entity_container = EntityWriteReference(curEntity);
 		}
 
-		if(entity_container == nullptr)
+		if(entity_container == nullptr || !CanCreateNewEntityFromConstraints(entity_container, new_entity_id))
 		{
 			new_entity_ids_list->AppendOrderedChildNode(nullptr);
 			continue;
@@ -378,7 +378,7 @@ EvaluableNodeReference Interpreter::InterpretNode_ENT_CREATE_ENTITIES(EvaluableN
 
 		entity_container->AddContainedEntityViaReference(new_entity, new_entity_id, writeListeners);
 
-		if(new_entity_id == StringInternPool::NOT_A_STRING_ID || !CanCreateNewEntityFromConstraints(entity_container, new_entity_id))
+		if(new_entity_id == StringInternPool::NOT_A_STRING_ID)
 		{
 			delete new_entity;
 			new_entity_ids_list->AppendOrderedChildNode(nullptr);