Make sure generated blocks have their timestamp set

behavior change: newly generated blocks are no longer momentarily activated.
this shouldn't matter for anyone and did not consistently apply to all blocks anyway

addresses issue from #15902 for new maps(!)

doc/lua_api.md

@@ -9479,12 +9479,17 @@ Used by `core.register_lbm`.
 
 A loading block modifier (LBM) is used to define a function that is called for
 specific nodes (defined by `nodenames`) when a mapblock which contains such nodes
-gets activated (not loaded!).
+gets activated (**not loaded!**).
 
-Note: LBMs operate on a "snapshot" of node positions taken once before they are triggered.
+*Note*: LBMs operate on a "snapshot" of node positions taken once before they are triggered.
 That means if an LBM callback adds a node, it won't be taken into account.
-However the engine guarantees that when the callback is called that all given position(s)
+However the engine guarantees that at the point in time when the callback is called
-contain a matching node.
+that all given positions contain a matching node.
+
+*Note*: For maps generated in 5.11.0 or older, many newly generated blocks did not
+get a timestamp set. This means LBMs introduced between generation time and
+time of first activation will never run.
+Currently the only workaround is to use `run_at_every_load`.
 
 ```lua
 {
@@ -9508,7 +9513,7 @@ contain a matching node.
     action = function(pos, node, dtime_s) end,
     -- Function triggered for each qualifying node.
     -- `dtime_s` is the in-game time (in seconds) elapsed since the block
-    -- was last active.
+    -- was last active (available since 5.7.0).
 
     bulk_action = function(pos_list, dtime_s) end,
     -- Function triggered with a list of all applicable node positions at once.

src/emerge.cpp

@@ -581,7 +581,8 @@ MapBlock *EmergeThread::finishGen(v3s16 pos, BlockMakeData *bmdata,
 		Perform post-processing on blocks (invalidate lighting, queue liquid
 		transforms, etc.) to finish block make
 	*/
-	m_map->finishBlockMake(bmdata, modified_blocks);
+	m_map->finishBlockMake(bmdata, modified_blocks,
+		m_server->m_env->getGameTime());
 
 	MapBlock *block = m_map->getBlockNoCreateNoEx(pos);
 	if (!block) {
@@ -619,11 +620,6 @@ MapBlock *EmergeThread::finishGen(v3s16 pos, BlockMakeData *bmdata,
 	m_mapgen->gennotify.clearEvents();
 	m_mapgen->vm = nullptr;
 
-	/*
-		Activate the block
-	*/
-	m_server->m_env->activateBlock(block, 0);
-
 	return block;
 }
 

src/serverenvironment.cpp

@@ -394,10 +394,8 @@ void ActiveBlockList::update(std::vector<PlayerSAO*> &active_players,
 	*/
 	std::set<v3s16> newlist = m_forceloaded_list;
 	std::set<v3s16> extralist;
-	m_abm_list = m_forceloaded_list;
 	for (const PlayerSAO *playersao : active_players) {
 		v3s16 pos = getNodeBlockPos(floatToInt(playersao->getBasePosition(), BS));
-		fillRadiusBlock(pos, active_block_range, m_abm_list);
 		fillRadiusBlock(pos, active_block_range, newlist);
 
 		s16 player_ao_range = std::min(active_object_range, playersao->getWantedRange());
@@ -417,6 +415,8 @@ void ActiveBlockList::update(std::vector<PlayerSAO*> &active_players,
 		}
 	}
 
+	m_abm_list = newlist;
+
 	/*
 		Find out which blocks on the new list are not on the old list
 	*/
@@ -448,6 +448,7 @@ void ActiveBlockList::update(std::vector<PlayerSAO*> &active_players,
 		Do some least-effort sanity checks to hopefully catch code bugs.
 	*/
 	assert(newlist.size() >= extralist.size());
+	assert(newlist.size() >= m_abm_list.size());
 	assert(blocks_removed.size() <= m_list.size());
 	if (!blocks_added.empty()) {
 		assert(newlist.count(*blocks_added.begin()) > 0);
@@ -1076,6 +1077,14 @@ public:
 	}
 };
 
+
+void ServerEnvironment::forceActivateBlock(MapBlock *block)
+{
+	assert(block);
+	if (m_active_blocks.add(block->getPos()))
+		activateBlock(block);
+}
+
 void ServerEnvironment::activateBlock(MapBlock *block, u32 additional_dtime)
 {
 	// Reset usage timer immediately, otherwise a block that becomes active

src/serverenvironment.h

@@ -124,6 +124,7 @@ public:
 	// Don't call this after loadIntroductionTimes() ran.
 	void addLBMDef(LoadingBlockModifierDef *lbm_def);
 
+	/// @param now current game time
 	void loadIntroductionTimes(const std::string &times,
 		IGameDef *gamedef, u32 now);
 
@@ -150,6 +151,7 @@ private:
 	// The key of the map is the LBM def's first introduction time.
 	lbm_lookup_map m_lbm_lookup;
 
+	/// @return map of LBM name -> timestamp
 	static std::unordered_map<std::string, u32>
 	parseIntroductionTimesString(const std::string &times);
 
@@ -186,12 +188,24 @@ public:
 		m_list.clear();
 	}
 
+	/// @return true if block was newly added
+	bool add(v3s16 p) {
+		if (m_list.insert(p).second) {
+			m_abm_list.insert(p);
+			return true;
+		}
+		return false;
+	}
+
 	void remove(v3s16 p) {
 		m_list.erase(p);
 		m_abm_list.erase(p);
 	}
 
+	// list of all active blocks
 	std::set<v3s16> m_list;
+	// list of blocks for ABM processing
+	// subset of `m_list` that does not contain view cone affected blocks
 	std::set<v3s16> m_abm_list;
 	// list of blocks that are always active, not modified by this class
 	std::set<v3s16> m_forceloaded_list;
@@ -312,10 +326,10 @@ public:
 	);
 
 	/*
-		Activate objects and dynamically modify for the dtime determined
+		Force a block to become active. It will probably be deactivated
-		from timestamp and additional_dtime
+		the next time active blocks are re-calculated.
 	*/
-	void activateBlock(MapBlock *block, u32 additional_dtime=0);
+	void forceActivateBlock(MapBlock *block);
 
 	/*
 		{Active,Loading}BlockModifiers
@@ -404,6 +418,9 @@ private:
 			const std::string &savedir, const Settings &conf);
 	static AuthDatabase *openAuthDatabase(const std::string &name,
 			const std::string &savedir, const Settings &conf);
+
+	void activateBlock(MapBlock *block, u32 additional_dtime=0);
+
 	/*
 		Internal ActiveObject interface
 		-------------------------------------------

src/servermap.cpp

@@ -199,6 +199,7 @@ bool ServerMap::blockpos_over_mapgen_limit(v3s16 p)
 
 bool ServerMap::initBlockMake(v3s16 blockpos, BlockMakeData *data)
 {
+	assert(data);
 	s16 csize = getMapgenParams()->chunksize;
 	v3s16 bpmin = EmergeManager::getContainingChunk(blockpos, csize);
 	v3s16 bpmax = bpmin + v3s16(1, 1, 1) * (csize - 1);
@@ -263,8 +264,10 @@ bool ServerMap::initBlockMake(v3s16 blockpos, BlockMakeData *data)
 }
 
 void ServerMap::finishBlockMake(BlockMakeData *data,
-	std::map<v3s16, MapBlock*> *changed_blocks)
+	std::map<v3s16, MapBlock*> *changed_blocks, u32 now)
 {
+	assert(data);
+	assert(changed_blocks);
 	v3s16 bpmin = data->blockpos_min;
 	v3s16 bpmax = data->blockpos_max;
 
@@ -283,7 +286,7 @@ void ServerMap::finishBlockMake(BlockMakeData *data,
 	/*
 		Copy transforming liquid information
 	*/
-	while (data->transforming_liquid.size()) {
+	while (!data->transforming_liquid.empty()) {
 		m_transforming_liquid.push_back(data->transforming_liquid.front());
 		data->transforming_liquid.pop_front();
 	}
@@ -297,15 +300,13 @@ void ServerMap::finishBlockMake(BlockMakeData *data,
 		*/
 		block->expireIsAirCache();
 		/*
-			Set block as modified
+			Set block as modified (if it isn't already)
 		*/
 		block->raiseModified(MOD_STATE_WRITE_NEEDED,
 			MOD_REASON_EXPIRE_IS_AIR);
 	}
 
-	/*
+	// Note: this does not apply to the extra border area
-		Set central blocks as generated
-	*/
 	for (s16 x = bpmin.X; x <= bpmax.X; x++)
 	for (s16 z = bpmin.Z; z <= bpmax.Z; z++)
 	for (s16 y = bpmin.Y; y <= bpmax.Y; y++) {
@@ -314,13 +315,10 @@ void ServerMap::finishBlockMake(BlockMakeData *data,
 			continue;
 
 		block->setGenerated(true);
+		// Set timestamp to ensure correct application of LBMs and other stuff
+		block->setTimestampNoChangedFlag(now);
 	}
 
-	/*
-		Save changed parts of map
-		NOTE: Will be saved later.
-	*/
-	//save(MOD_STATE_WRITE_AT_UNLOAD);
 	m_chunks_in_progress.erase(bpmin);
 }
 

src/servermap.h

@@ -61,9 +61,13 @@ public:
 		Blocks are generated by using these and makeBlock().
 	*/
 	bool blockpos_over_mapgen_limit(v3s16 p);
+	/// @brief copy data from map to prepare for mapgen
+	/// @return true if mapgen should actually happen
 	bool initBlockMake(v3s16 blockpos, BlockMakeData *data);
+	/// @brief write data back to map after mapgen
+	/// @param now current game time
 	void finishBlockMake(BlockMakeData *data,
-		std::map<v3s16, MapBlock*> *changed_blocks);
+		std::map<v3s16, MapBlock*> *changed_blocks, u32 now);
 
 	/*
 		Get a block from somewhere.

src/unittest/test_sao.cpp

@@ -197,8 +197,8 @@ void TestSAO::testActivate(ServerEnvironment *env)
 	UASSERT(block);
 	block->m_static_objects.insert(0, s_obj);
 
-	// Activating the block will convert it to active.
+	// this will convert it to an active object
-	env->activateBlock(block);
+	env->forceActivateBlock(block);
 
 	const u16 obj_id = assert_active_in_block(block);
 	auto *obj = env->getActiveObject(obj_id);
@@ -239,7 +239,7 @@ void TestSAO::testStaticToFalse(ServerEnvironment *env)
 	UASSERT(block);
 	block->m_static_objects.insert(0, s_obj);
 
-	env->activateBlock(block);
+	env->forceActivateBlock(block);
 
 	const u16 obj_id = assert_active_in_block(block);
 	auto *obj = env->getActiveObject(obj_id);

src/util/container.h

@@ -53,11 +53,16 @@ public:
 		return m_queue.front();
 	}
 
-	u32 size() const
+	size_t size() const
 	{
 		return m_queue.size();
 	}
 
+	bool empty() const
+	{
+		return m_queue.empty();
+	}
+
 private:
 	std::set<Value> m_set;
 	std::queue<Value> m_queue;