ServerEnv: Clean up object lifecycle handling (#6414)

* ServerEnv: Clean up object lifecycle handling

src/content_sao.cpp

@@ -59,7 +59,7 @@ public:
 		m_age += dtime;
 		if(m_age > 10)
 		{
-			m_removed = true;
+			m_pending_removal = true;
 			return;
 		}
 
@@ -415,7 +415,7 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
 			infostream << "Remove SAO " << m_id << "(" << m_init_name
 				<< "), outside of limits" << std::endl;
 			m_pending_deactivation = true;
-			m_removed = true;
+			m_pending_removal = true;
 			return;
 		}
 	}
@@ -555,9 +555,9 @@ int LuaEntitySAO::punch(v3f dir,
 		ServerActiveObject *puncher,
 		float time_from_last_punch)
 {
-	if (!m_registered){
+	if (!m_registered) {
 		// Delete unknown LuaEntities when punched
-		m_removed = true;
+		m_pending_removal = true;
 		return 0;
 	}
 
@@ -601,7 +601,7 @@ int LuaEntitySAO::punch(v3f dir,
 	}
 
 	if (getHP() == 0)
-		m_removed = true;
+		m_pending_removal = true;
 
 
 
@@ -1361,11 +1361,10 @@ void PlayerSAO::setWieldIndex(int i)
 	}
 }
 
-// Erase the peer id and make the object for removal
 void PlayerSAO::disconnected()
 {
 	m_peer_id = 0;
-	m_removed = true;
+	m_pending_removal = true;
 }
 
 void PlayerSAO::unlinkPlayerSessionAndSave()

src/network/serverpackethandler.cpp

@@ -1441,8 +1441,8 @@ void Server::handleCommand_Interact(NetworkPacket* pkt)
 			playersao->noCheatDigStart(p_under);
 		}
 		else if (pointed.type == POINTEDTHING_OBJECT) {
-			// Skip if object has been removed
-			if (pointed_object->m_removed)
+			// Skip if object can't be interacted with anymore
+			if (pointed_object->isGone())
 				return;
 
 			actionstream<<player->getName()<<" punches object "
@@ -1600,8 +1600,8 @@ void Server::handleCommand_Interact(NetworkPacket* pkt)
 		if (pointed.type == POINTEDTHING_OBJECT) {
 			// Right click object
 
-			// Skip if object has been removed
-			if (pointed_object->m_removed)
+			// Skip if object can't be interacted with anymore
+			if (pointed_object->isGone())
 				return;
 
 			actionstream << player->getName() << " right-clicks object "

src/script/cpp_api/s_base.cpp

@@ -322,6 +322,10 @@ void ScriptApiBase::objectrefGetOrCreate(lua_State *L,
 		ObjectRef::create(L, cobj);
 	} else {
 		push_objectRef(L, cobj->getId());
+		if (cobj->isGone())
+			warningstream << "ScriptApiBase::objectrefGetOrCreate(): "
+					<< "Pushing ObjectRef to removed/deactivated object"
+					<< ", this is probably a bug." << std::endl;
 	}
 }
 

src/script/lua_api/l_env.cpp

@@ -537,9 +537,11 @@ int ModApiEnvMod::l_get_objects_inside_radius(lua_State *L)
 	std::vector<u16>::const_iterator iter = ids.begin();
 	for(u32 i = 0; iter != ids.end(); ++iter) {
 		ServerActiveObject *obj = env->getActiveObject(*iter);
-		// Insert object reference into table
-		script->objectrefGetOrCreate(L, obj);
-		lua_rawseti(L, -2, ++i);
+		if (!obj->isGone()) {
+			// Insert object reference into table
+			script->objectrefGetOrCreate(L, obj);
+			lua_rawseti(L, -2, ++i);
+		}
 	}
 	return 1;
 }

src/script/lua_api/l_object.cpp

@@ -137,16 +137,15 @@ int ObjectRef::l_remove(lua_State *L)
 	if (co->getType() == ACTIVEOBJECT_TYPE_PLAYER)
 		return 0;
 
-	const UNORDERED_SET<int> &child_ids = co->getAttachmentChildIds();
-	UNORDERED_SET<int>::const_iterator it;
-	for (it = child_ids.begin(); it != child_ids.end(); ++it) {
+	const std::unordered_set<int> &child_ids = co->getAttachmentChildIds();
+	for (int child_id : child_ids) {
 		// Child can be NULL if it was deleted earlier
-		if (ServerActiveObject *child = env->getActiveObject(*it))
+		if (ServerActiveObject *child = env->getActiveObject(child_id))
 			child->setAttachment(0, "", v3f(0, 0, 0), v3f(0, 0, 0));
 	}
 
-	verbosestream<<"ObjectRef::l_remove(): id="<<co->getId()<<std::endl;
-	co->m_removed = true;
+	verbosestream << "ObjectRef::l_remove(): id=" << co->getId() << std::endl;
+	co->m_pending_removal = true;
 	return 0;
 }
 

src/serverenvironment.cpp

@@ -1018,26 +1018,18 @@ void ServerEnvironment::clearObjects(ClearObjectsMode mode)
 	infostream << "ServerEnvironment::clearObjects(): "
 		<< "Removing all active objects" << std::endl;
 	std::vector<u16> objects_to_remove;
-	for (ActiveObjectMap::iterator i = m_active_objects.begin();
-		i != m_active_objects.end(); ++i) {
-		ServerActiveObject* obj = i->second;
+	for (auto &it : m_active_objects) {
+		u16 id = it.first;
+		ServerActiveObject* obj = it.second;
 		if (obj->getType() == ACTIVEOBJECT_TYPE_PLAYER)
 			continue;
-		u16 id = i->first;
+
 		// Delete static object if block is loaded
-		if (obj->m_static_exists) {
-			MapBlock *block = m_map->getBlockNoCreateNoEx(obj->m_static_block);
-			if (block) {
-				block->m_static_objects.remove(id);
-				block->raiseModified(MOD_STATE_WRITE_NEEDED,
-					MOD_REASON_CLEAR_ALL_OBJECTS);
-				obj->m_static_exists = false;
-			}
-		}
+		deleteStaticFromBlock(obj, id, MOD_REASON_CLEAR_ALL_OBJECTS, true);
+
 		// If known by some client, don't delete immediately
 		if (obj->m_known_by_count > 0) {
-			obj->m_pending_deactivation = true;
-			obj->m_removed = true;
+			obj->m_pending_removal = true;
 			continue;
 		}
 
@@ -1054,9 +1046,8 @@ void ServerEnvironment::clearObjects(ClearObjectsMode mode)
 	}
 
 	// Remove references from m_active_objects
-	for (std::vector<u16>::iterator i = objects_to_remove.begin();
-		i != objects_to_remove.end(); ++i) {
-		m_active_objects.erase(*i);
+	for (u16 i : objects_to_remove) {
+		m_active_objects.erase(i);
 	}
 
 	// Get list of loaded blocks
@@ -1399,16 +1390,14 @@ void ServerEnvironment::step(float dtime)
 		for(ActiveObjectMap::iterator i = m_active_objects.begin();
 			i != m_active_objects.end(); ++i) {
 			ServerActiveObject* obj = i->second;
-			// Don't step if is to be removed or stored statically
-			if(obj->m_removed || obj->m_pending_deactivation)
+			if (obj->isGone())
 				continue;
+
 			// Step object
 			obj->step(dtime, send_recommended);
 			// Read messages from object
-			while(!obj->m_messages_out.empty())
-			{
-				m_active_object_messages.push(
-					obj->m_messages_out.front());
+			while (!obj->m_messages_out.empty()) {
+				m_active_object_messages.push(obj->m_messages_out.front());
 				obj->m_messages_out.pop();
 			}
 		}
@@ -1420,9 +1409,6 @@ void ServerEnvironment::step(float dtime)
 	if(m_object_management_interval.step(dtime, 0.5))
 	{
 		ScopeProfiler sp(g_profiler, "SEnv: remove removed objs avg /.5s", SPT_AVG);
-		/*
-			Remove objects that satisfy (m_removed && m_known_by_count==0)
-		*/
 		removeRemovedObjects();
 	}
 
@@ -1542,7 +1528,7 @@ void ServerEnvironment::getAddedActiveObjects(PlayerSAO *playersao, s16 radius,
 		player_radius_f = 0;
 	/*
 		Go through the object list,
-		- discard m_removed objects,
+		- discard removed/deactivated objects,
 		- discard objects that are too far away,
 		- discard objects that are found in current_objects.
 		- add remaining objects to added_objects
@@ -1556,8 +1542,7 @@ void ServerEnvironment::getAddedActiveObjects(PlayerSAO *playersao, s16 radius,
 		if (object == NULL)
 			continue;
 
-		// Discard if removed or deactivating
-		if(object->m_removed || object->m_pending_deactivation)
+		if (object->isGone())
 			continue;
 
 		f32 distance_f = object->getBasePosition().
@@ -1615,7 +1600,7 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
 			continue;
 		}
 
-		if (object->m_removed || object->m_pending_deactivation) {
+		if (object->isGone()) {
 			removed_objects.push(id);
 			continue;
 		}
@@ -1757,7 +1742,7 @@ u16 ServerEnvironment::addActiveObjectRaw(ServerActiveObject *object,
 }
 
 /*
-	Remove objects that satisfy (m_removed && m_known_by_count==0)
+	Remove objects that satisfy (isGone() && m_known_by_count==0)
 */
 void ServerEnvironment::removeRemovedObjects()
 {
@@ -1766,62 +1751,54 @@ void ServerEnvironment::removeRemovedObjects()
 		i != m_active_objects.end(); ++i) {
 		u16 id = i->first;
 		ServerActiveObject* obj = i->second;
+
 		// This shouldn't happen but check it
-		if(obj == NULL)
-		{
-			infostream<<"NULL object found in ServerEnvironment"
-				<<" while finding removed objects. id="<<id<<std::endl;
-			// Id to be removed from m_active_objects
+		if (!obj) {
+			errorstream << "ServerEnvironment::removeRemovedObjects(): "
+					<< "NULL object found. id=" << id << std::endl;
 			objects_to_remove.push_back(id);
 			continue;
 		}
 
 		/*
-			We will delete objects that are marked as removed or thatare
-			waiting for deletion after deactivation
+			We will handle objects marked for removal or deactivation
 		*/
-		if (!obj->m_removed && !obj->m_pending_deactivation)
+		if (!obj->isGone())
 			continue;
 
 		/*
-			Delete static data from block if is marked as removed
+			Delete static data from block if removed
 		*/
-		if(obj->m_static_exists && obj->m_removed)
-		{
-			MapBlock *block = m_map->emergeBlock(obj->m_static_block, false);
-			if (block) {
-				block->m_static_objects.remove(id);
-				block->raiseModified(MOD_STATE_WRITE_NEEDED,
-					MOD_REASON_REMOVE_OBJECTS_REMOVE);
-				obj->m_static_exists = false;
-			} else {
-				infostream<<"Failed to emerge block from which an object to "
-					<<"be removed was loaded from. id="<<id<<std::endl;
-			}
-		}
+		if (obj->m_pending_removal)
+			deleteStaticFromBlock(obj, id, MOD_REASON_REMOVE_OBJECTS_REMOVE, false);
 
-		// If m_known_by_count > 0, don't actually remove. On some future
+		// If still known by clients, don't actually remove. On some future
 		// invocation this will be 0, which is when removal will continue.
 		if(obj->m_known_by_count > 0)
 			continue;
 
 		/*
-			Move static data from active to stored if not marked as removed
+			Move static data from active to stored if deactivated
 		*/
-		if(obj->m_static_exists && !obj->m_removed){
+		if (!obj->m_pending_removal && obj->m_static_exists) {
 			MapBlock *block = m_map->emergeBlock(obj->m_static_block, false);
 			if (block) {
 				std::map<u16, StaticObject>::iterator i =
 					block->m_static_objects.m_active.find(id);
-				if(i != block->m_static_objects.m_active.end()){
+				if (i != block->m_static_objects.m_active.end()) {
 					block->m_static_objects.m_stored.push_back(i->second);
 					block->m_static_objects.m_active.erase(id);
 					block->raiseModified(MOD_STATE_WRITE_NEEDED,
 						MOD_REASON_REMOVE_OBJECTS_DEACTIVATE);
+				} else {
+					warningstream << "ServerEnvironment::removeRemovedObjects(): "
+							<< "id=" << id << " m_static_exists=true but "
+							<< "static data doesn't actually exist in "
+							<< PP(obj->m_static_block) << std::endl;
 				}
 			} else {
-				infostream<<"Failed to emerge block from which an object to "
-					<<"be deactivated was loaded from. id="<<id<<std::endl;
+				infostream << "Failed to emerge block from which an object to "
+						<< "be deactivated was loaded from. id=" << id << std::endl;
 			}
 		}
 
@@ -1834,13 +1811,11 @@ void ServerEnvironment::removeRemovedObjects()
 		if(obj->environmentDeletes())
 			delete obj;
 
-		// Id to be removed from m_active_objects
 		objects_to_remove.push_back(id);
 	}
 	// Remove references from m_active_objects
-	for(std::vector<u16>::iterator i = objects_to_remove.begin();
-		i != objects_to_remove.end(); ++i) {
-		m_active_objects.erase(*i);
+	for (u16 i : objects_to_remove) {
+		m_active_objects.erase(i);
 	}
 }
 
@@ -1934,6 +1909,7 @@ void ServerEnvironment::activateObjects(MapBlock *block, u32 dtime_s)
 		// This will also add the object to the active static list
 		addActiveObjectRaw(obj, false, dtime_s);
 	}
+
 	// Clear stored list
 	block->m_static_objects.m_stored.clear();
 	// Add leftover failed stuff to stored list
@@ -1944,18 +1920,6 @@ void ServerEnvironment::activateObjects(MapBlock *block, u32 dtime_s)
 		block->m_static_objects.m_stored.push_back(s_obj);
 	}
 
-	// Turn the active counterparts of activated objects not pending for
-	// deactivation
-	for(std::map<u16, StaticObject>::iterator
-		i = block->m_static_objects.m_active.begin();
-		i != block->m_static_objects.m_active.end(); ++i)
-	{
-		u16 id = i->first;
-		ServerActiveObject *object = getActiveObject(id);
-		assert(object);
-		object->m_pending_deactivation = false;
-	}
-
 	/*
 		Note: Block hasn't really been modified here.
 		The objects have just been activated and moved from the stored
@@ -1992,8 +1956,8 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 		if(!force_delete && !obj->isStaticAllowed())
 			continue;
 
-		// If pending deactivation, let removeRemovedObjects() do it
-		if(!force_delete && obj->m_pending_deactivation)
+		// removeRemovedObjects() is responsible for these
+		if(!force_delete && obj->isGone())
 			continue;
 
 		u16 id = i->first;
@@ -2010,47 +1974,25 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 			!m_active_blocks.contains(obj->m_static_block) &&
 			m_active_blocks.contains(blockpos_o))
 		{
-			v3s16 old_static_block = obj->m_static_block;
+			// Delete from block where object was located
+			deleteStaticFromBlock(obj, id, MOD_REASON_STATIC_DATA_REMOVED, false);
 
-			// Save to block where object is located
-			MapBlock *block = m_map->emergeBlock(blockpos_o, false);
-			if(!block){
-				errorstream<<"ServerEnvironment::deactivateFarObjects(): "
-					<<"Could not save object id="<<id
-					<<" to it's current block "<<PP(blockpos_o)
-					<<std::endl;
-				continue;
-			}
 			std::string staticdata_new = "";
 			obj->getStaticData(&staticdata_new);
 			StaticObject s_obj(obj->getType(), objectpos, staticdata_new);
-			block->m_static_objects.insert(id, s_obj);
-			obj->m_static_block = blockpos_o;
-			block->raiseModified(MOD_STATE_WRITE_NEEDED,
-				MOD_REASON_STATIC_DATA_ADDED);
+			// Save to block where object is located
+			saveStaticToBlock(blockpos_o, id, obj, s_obj, MOD_REASON_STATIC_DATA_ADDED);
 
-			// Delete from block where object was located
-			block = m_map->emergeBlock(old_static_block, false);
-			if(!block){
-				errorstream<<"ServerEnvironment::deactivateFarObjects(): "
-					<<"Could not delete object id="<<id
-					<<" from it's previous block "<<PP(old_static_block)
-					<<std::endl;
-				continue;
-			}
-			block->m_static_objects.remove(id);
-			block->raiseModified(MOD_STATE_WRITE_NEEDED,
-				MOD_REASON_STATIC_DATA_REMOVED);
 			continue;
 		}
 
-		// If block is active, don't remove
+		// If block is still active, don't remove
 		if(!force_delete && m_active_blocks.contains(blockpos_o))
 			continue;
 
-		verbosestream<<"ServerEnvironment::deactivateFarObjects(): "
-			<<"deactivating object id="<<id<<" on inactive block "
-			<<PP(blockpos_o)<<std::endl;
+		verbosestream << "ServerEnvironment::deactivateFarObjects(): "
+				<< "deactivating object id=" << id << " on inactive block "
+				<< PP(blockpos_o) << std::endl;
 
 		// If known by some client, don't immediately delete.
 		bool pending_delete = (obj->m_known_by_count > 0 && !force_delete);
@@ -2058,7 +2000,6 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 		/*
 			Update the static data
 		*/
-
 		if(obj->isStaticAllowed())
 		{
 			// Create new static object
@@ -2069,6 +2010,7 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 			bool stays_in_same_block = false;
 			bool data_changed = true;
 
+			// Check if static data has changed considerably
 			if (obj->m_static_exists) {
 				if (obj->m_static_block == blockpos_o)
 					stays_in_same_block = true;
@@ -2087,88 +2029,29 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 							(static_old.pos - objectpos).getLength() < save_movem)
 							data_changed = false;
 					} else {
-						errorstream<<"ServerEnvironment::deactivateFarObjects(): "
-							<<"id="<<id<<" m_static_exists=true but "
-							<<"static data doesn't actually exist in "
-							<<PP(obj->m_static_block)<<std::endl;
+						warningstream << "ServerEnvironment::deactivateFarObjects(): "
+								<< "id=" << id << " m_static_exists=true but "
+								<< "static data doesn't actually exist in "
+								<< PP(obj->m_static_block) << std::endl;
 					}
 				}
 			}
 
+			/*
+				While changes are always saved, blocks are only marked as modified
+				if the object has moved or different staticdata. (see above)
+			*/
 			bool shall_be_written = (!stays_in_same_block || data_changed);
+			u32 reason = shall_be_written ? MOD_REASON_STATIC_DATA_CHANGED : MOD_REASON_UNKNOWN;
 
 			// Delete old static object
-			if(obj->m_static_exists)
-			{
-				MapBlock *block = m_map->emergeBlock(obj->m_static_block, false);
-				if(block)
-				{
-					block->m_static_objects.remove(id);
-					obj->m_static_exists = false;
-					// Only mark block as modified if data changed considerably
-					if(shall_be_written)
-						block->raiseModified(MOD_STATE_WRITE_NEEDED,
-							MOD_REASON_STATIC_DATA_CHANGED);
-				}
-			}
+			deleteStaticFromBlock(obj, id, reason, false);
 
 			// Add to the block where the object is located in
 			v3s16 blockpos = getNodeBlockPos(floatToInt(objectpos, BS));
-			// Get or generate the block
-			MapBlock *block = NULL;
-			try{
-				block = m_map->emergeBlock(blockpos);
-			} catch(InvalidPositionException &e){
-				// Handled via NULL pointer
-				// NOTE: emergeBlock's failure is usually determined by it
-				//       actually returning NULL
-			}
-
-			if(block)
-			{
-				if (block->m_static_objects.m_stored.size() >= g_settings->getU16("max_objects_per_block")) {
-					warningstream << "ServerEnv: Trying to store id = " << obj->getId()
-						<< " statically but block " << PP(blockpos)
-						<< " already contains "
-						<< block->m_static_objects.m_stored.size()
-						<< " objects."
-						<< " Forcing delete." << std::endl;
-					force_delete = true;
-				} else {
-					// If static counterpart already exists in target block,
-					// remove it first.
-					// This shouldn't happen because the object is removed from
-					// the previous block before this according to
-					// obj->m_static_block, but happens rarely for some unknown
-					// reason. Unsuccessful attempts have been made to find
-					// said reason.
-					if(id && block->m_static_objects.m_active.find(id) != block->m_static_objects.m_active.end()){
-						warningstream<<"ServerEnv: Performing hack #83274"
-							<<std::endl;
-						block->m_static_objects.remove(id);
-					}
-					// Store static data
-					u16 store_id = pending_delete ? id : 0;
-					block->m_static_objects.insert(store_id, s_obj);
-
-					// Only mark block as modified if data changed considerably
-					if(shall_be_written)
-						block->raiseModified(MOD_STATE_WRITE_NEEDED,
-							MOD_REASON_STATIC_DATA_CHANGED);
-
-					obj->m_static_exists = true;
-					obj->m_static_block = block->getPos();
-				}
-			}
-			else{
-				if(!force_delete){
-					v3s16 p = floatToInt(objectpos, BS);
-					errorstream<<"ServerEnv: Could not find or generate "
-						<<"a block for storing id="<<obj->getId()
-						<<" statically (pos="<<PP(p)<<")"<<std::endl;
-					continue;
-				}
-			}
+			u16 store_id = pending_delete ? id : 0;
+			if (!saveStaticToBlock(blockpos, store_id, obj, s_obj, reason))
+				force_delete = true;
 		}
 
 		/*
@@ -2178,17 +2061,16 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 
 		if(pending_delete && !force_delete)
 		{
-			verbosestream<<"ServerEnvironment::deactivateFarObjects(): "
-				<<"object id="<<id<<" is known by clients"
-				<<"; not deleting yet"<<std::endl;
+			verbosestream << "ServerEnvironment::deactivateFarObjects(): "
+					<< "object id=" << id << " is known by clients"
+					<< "; not deleting yet" << std::endl;
 
 			obj->m_pending_deactivation = true;
 			continue;
 		}
-
-		verbosestream<<"ServerEnvironment::deactivateFarObjects(): "
-			<<"object id="<<id<<" is not known by clients"
-			<<"; deleting"<<std::endl;
+		verbosestream << "ServerEnvironment::deactivateFarObjects(): "
+				<< "object id=" << id << " is not known by clients"
+				<< "; deleting" << std::endl;
 
 		// Tell the object about removal
 		obj->removingFromEnvironment();
@@ -2203,12 +2085,74 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 	}
 
 	// Remove references from m_active_objects
-	for(std::vector<u16>::iterator i = objects_to_remove.begin();
-		i != objects_to_remove.end(); ++i) {
-		m_active_objects.erase(*i);
+	for (u16 i : objects_to_remove) {
+		m_active_objects.erase(i);
 	}
 }
 
+void ServerEnvironment::deleteStaticFromBlock(
+		ServerActiveObject *obj, u16 id, u32 mod_reason, bool no_emerge)
+{
+	if (!obj->m_static_exists)
+		return;
+
+	MapBlock *block;
+	if (no_emerge)
+		block = m_map->getBlockNoCreateNoEx(obj->m_static_block);
+	else
+		block = m_map->emergeBlock(obj->m_static_block, false);
+	if (!block) {
+		if (!no_emerge)
+			errorstream << "ServerEnv: Failed to emerge block " << PP(obj->m_static_block)
+					<< " when deleting static data of object from it. id=" << id << std::endl;
+		return;
+	}
+
+	block->m_static_objects.remove(id);
+	if (mod_reason != MOD_REASON_UNKNOWN) // Do not mark as modified if requested
+		block->raiseModified(MOD_STATE_WRITE_NEEDED, mod_reason);
+
+	obj->m_static_exists = false;
+}
+
+bool ServerEnvironment::saveStaticToBlock(
+		v3s16 blockpos, u16 store_id,
+		ServerActiveObject *obj, const StaticObject &s_obj,
+		u32 mod_reason)
+{
+	MapBlock *block = nullptr;
+	try {
+		block = m_map->emergeBlock(blockpos);
+	} catch (InvalidPositionException &e) {
+		// Handled via NULL pointer
+		// NOTE: emergeBlock's failure is usually determined by it
+		//       actually returning NULL
+	}
+
+	if (!block) {
+		errorstream << "ServerEnv: Failed to emerge block " << PP(obj->m_static_block)
+				<< " when saving static data of object to it. id=" << store_id << std::endl;
+		return false;
+	}
+	if (block->m_static_objects.m_stored.size() >= g_settings->getU16("max_objects_per_block")) {
+		warningstream << "ServerEnv: Trying to store id = " << store_id
+				<< " statically but block " << PP(blockpos)
+				<< " already contains "
+				<< block->m_static_objects.m_stored.size()
+				<< " objects." << std::endl;
+		return false;
+	}
+
+	block->m_static_objects.insert(store_id, s_obj);
+	if (mod_reason != MOD_REASON_UNKNOWN) // Do not mark as modified if requested
+		block->raiseModified(MOD_STATE_WRITE_NEEDED, mod_reason);
+
+	obj->m_static_exists = true;
+	obj->m_static_block = blockpos;
+
+	return true;
+}
+
 PlayerDatabase *ServerEnvironment::openPlayerDatabase(const std::string &name,
 		const std::string &savedir, const Settings &conf)
 {

src/serverenvironment.h

@@ -33,6 +33,7 @@ class PlayerDatabase;
 class PlayerSAO;
 class ServerEnvironment;
 class ActiveBlockModifier;
+struct StaticObject;
 class ServerActiveObject;
 class Server;
 class ServerScripting;
@@ -362,7 +363,7 @@ private:
 	u16 addActiveObjectRaw(ServerActiveObject *object, bool set_changed, u32 dtime_s);
 
 	/*
-		Remove all objects that satisfy (m_removed && m_known_by_count==0)
+		Remove all objects that satisfy (isGone() && m_known_by_count==0)
 	*/
 	void removeRemovedObjects();
 
@@ -382,6 +383,14 @@ private:
 	*/
 	void deactivateFarObjects(bool force_delete);
 
+	/*
+		A few helpers used by the three above methods
+	*/
+	void deleteStaticFromBlock(
+			ServerActiveObject *obj, u16 id, u32 mod_reason, bool no_emerge);
+	bool saveStaticToBlock(v3s16 blockpos, u16 store_id,
+			ServerActiveObject *obj, const StaticObject &s_obj, u32 mod_reason);
+
 	/*
 		Member variables
 	*/

src/serverobject.cpp

@@ -25,7 +25,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 ServerActiveObject::ServerActiveObject(ServerEnvironment *env, v3f pos):
 	ActiveObject(0),
 	m_known_by_count(0),
-	m_removed(false),
+	m_pending_removal(false),
 	m_pending_deactivation(false),
 	m_static_exists(false),
 	m_static_block(1337,1337,1337),

src/serverobject.h

@@ -210,22 +210,26 @@ public:
 		  it anymore.
 		- Removal is delayed to preserve the id for the time during which
 		  it could be confused to some other object by some client.
-		- This is set to true by the step() method when the object wants
-		  to be deleted.
-		- This can be set to true by anything else too.
+		- This is usually set to true by the step() method when the object wants
+		  to be deleted but can be set by anything else too.
 	*/
-	bool m_removed;
+	bool m_pending_removal = false;
 
 	/*
-		This is set to true when an object should be removed from the active
-		object list but couldn't be removed because the id has to be
-		reserved for some client.
+		Same purpose as m_pending_removal but for deactivation.
+		deactvation = save static data in block, remove active object
 
-		The environment checks this periodically. If this is true and also
-		m_known_by_count is true, object is deleted from the active object
-		list.
+		If this is set alongside with m_pending_removal, removal takes
+		priority.
 	*/
-	bool m_pending_deactivation;
+	bool m_pending_deactivation = false;
+
+	/*
+		A getter that unifies the above to answer the question:
+		"Can the environment still interact with this object?"
+	*/
+	inline bool isGone() const
+	{ return m_pending_removal || m_pending_deactivation; }
 
 	/*
 		Whether the object's static data has been stored to a block