Some more random code cleanups

builtin/settingtypes.txt

@@ -2106,14 +2106,14 @@ max_objects_per_block (Maximum objects per block) int 256 256 65535
 active_block_mgmt_interval (Active block management interval) float 2.0 0.0
 
 #    Length of time between Active Block Modifier (ABM) execution cycles, stated in seconds.
-abm_interval (ABM interval) float 1.0 0.0
+abm_interval (ABM interval) float 1.0 0.1 30.0
 
 #    The time budget allowed for ABMs to execute on each step
 #    (as a fraction of the ABM Interval)
 abm_time_budget (ABM time budget) float 0.2 0.1 0.9
 
 #    Length of time between NodeTimer execution cycles, stated in seconds.
-nodetimer_interval (NodeTimer interval) float 0.2 0.0
+nodetimer_interval (NodeTimer interval) float 0.2 0.1 1.0
 
 #    Max liquids processed per step.
 liquid_loop_max (Liquid loop max) int 100000 1 4294967295

src/environment.cpp

@@ -17,11 +17,6 @@ Environment::Environment(IGameDef *gamedef):
 	m_day_count(0),
 	m_gamedef(gamedef)
 {
-	m_cache_active_block_mgmt_interval = g_settings->getFloat("active_block_mgmt_interval");
-	m_cache_abm_interval = g_settings->getFloat("abm_interval");
-	m_cache_nodetimer_interval = g_settings->getFloat("nodetimer_interval");
-	m_cache_abm_time_budget = g_settings->getFloat("abm_time_budget");
-
 	m_time_of_day = g_settings->getU32("world_start_time");
 	m_time_of_day_f = (float)m_time_of_day / 24000.0f;
 }

src/environment.h

@@ -122,20 +122,6 @@ protected:
 	 * Above: values managed by m_time_lock
 	 */
 
-	/* TODO: Add a callback function so these can be updated when a setting
-	 *       changes.  At this point in time it doesn't matter (e.g. /set
-	 *       is documented to change server settings only)
-	 *
-	 * TODO: Local caching of settings is not optimal and should at some stage
-	 *       be updated to use a global settings object for getting thse values
-	 *       (as opposed to the this local caching). This can be addressed in
-	 *       a later release.
-	 */
-	float m_cache_active_block_mgmt_interval;
-	float m_cache_abm_interval;
-	float m_cache_nodetimer_interval;
-	float m_cache_abm_time_budget;
-
 	IGameDef *m_gamedef;
 
 private:

src/mapblock.cpp

@@ -120,20 +120,19 @@ bool MapBlock::saveStaticObject(u16 id, const StaticObject &obj, u32 reason)
 	return true;
 }
 
-// This method is only for Server, don't call it on client
 void MapBlock::step(float dtime, const std::function<bool(v3s16, MapNode, f32)> &on_timer_cb)
 {
-	// Run script callbacks for elapsed node_timers
+	// Run callbacks for elapsed node_timers
 	std::vector<NodeTimer> elapsed_timers = m_node_timers.step(dtime);
-	if (!elapsed_timers.empty()) {
 	MapNode n;
 	v3s16 p;
-		for (const NodeTimer &elapsed_timer : elapsed_timers) {
+	for (const auto &it : elapsed_timers) {
-			n = getNodeNoEx(elapsed_timer.position);
+		n = getNodeNoEx(it.position);
-			p = elapsed_timer.position + getPosRelative();
+		p = it.position + getPosRelative();
-			if (on_timer_cb(p, n, elapsed_timer.elapsed))
+		if (on_timer_cb(p, n, it.elapsed))
-				setNodeTimer(NodeTimer(elapsed_timer.timeout, 0, elapsed_timer.position));
+			setNodeTimer(NodeTimer(it.timeout, 0, it.position));
-		}
+		if (isOrphan())
+			return;
 	}
 }
 

src/mapblock.h

@@ -313,6 +313,7 @@ public:
 	bool onObjectsActivation();
 	bool saveStaticObject(u16 id, const StaticObject &obj, u32 reason);
 
+	/// @note This method is only for Server, don't call it on client
 	void step(float dtime, const std::function<bool(v3s16, MapNode, f32)> &on_timer_cb);
 
 	////
@@ -337,6 +338,7 @@ public:
 		return m_timestamp;
 	}
 
+	/// @deprecated don't use in new code, unclear semantics.
 	inline u32 getDiskTimestamp()
 	{
 		return m_disk_timestamp;

src/serverenvironment.cpp

@@ -45,6 +45,8 @@
 
 static constexpr s16 ACTIVE_OBJECT_RESAVE_DISTANCE_SQ = sqr(3);
 
+static constexpr u32 BLOCK_RESAVE_TIMESTAMP_DIFF = 60; // in units of game time
+
 /*
 	ABMWithState
 */
@@ -54,9 +56,9 @@ ABMWithState::ABMWithState(ActiveBlockModifier *abm_):
 {
 	// Initialize timer to random value to spread processing
 	float itv = abm->getTriggerInterval();
-	itv = MYMAX(0.001, itv); // No less than 1ms
+	itv = MYMAX(0.001f, itv); // No less than 1ms
-	int minval = MYMAX(-0.51*itv, -60); // Clamp to
+	int minval = MYMAX(-0.51f*itv, -60); // Clamp to
-	int maxval = MYMIN(0.51*itv, 60);   // +-60 seconds
+	int maxval = MYMIN(0.51f*itv, 60);   // +-60 seconds
 	timer = myrand_range(minval, maxval);
 }
 
@@ -494,6 +496,11 @@ ServerEnvironment::ServerEnvironment(std::unique_ptr<ServerMap> map,
 	m_script(server->getScriptIface()),
 	m_server(server)
 {
+	m_cache_active_block_mgmt_interval = g_settings->getFloat("active_block_mgmt_interval");
+	m_cache_abm_interval = rangelim(g_settings->getFloat("abm_interval"), 0.1f, 30);
+	m_cache_nodetimer_interval = rangelim(g_settings->getFloat("nodetimer_interval"), 0.1f, 1);
+	m_cache_abm_time_budget = g_settings->getFloat("abm_time_budget");
+
 	m_step_time_counter = mb->addCounter(
 		"minetest_env_step_time", "Time spent in environment step (in microseconds)");
 
@@ -1083,9 +1090,10 @@ void ServerEnvironment::forceActivateBlock(MapBlock *block)
 	assert(block);
 	if (m_active_blocks.add(block->getPos()))
 		activateBlock(block);
+	m_active_block_gauge->set(m_active_blocks.size());
 }
 
-void ServerEnvironment::activateBlock(MapBlock *block, u32 additional_dtime)
+void ServerEnvironment::activateBlock(MapBlock *block)
 {
 	// Reset usage timer immediately, otherwise a block that becomes active
 	// again at around the same time as it would normally be unloaded will
@@ -1100,7 +1108,6 @@ void ServerEnvironment::activateBlock(MapBlock *block, u32 additional_dtime)
 	u32 stamp = block->getTimestamp();
 	if (m_game_time > stamp && stamp != BLOCK_TIMESTAMP_UNDEFINED)
 		dtime_s = m_game_time - stamp;
-	dtime_s += additional_dtime;
 
 	// Remove stored static objects if clearObjects was called since block's timestamp
 	// Note that non-generated blocks may still have stored static objects
@@ -1124,7 +1131,7 @@ void ServerEnvironment::activateBlock(MapBlock *block, u32 additional_dtime)
 
 	// Run node timers
 	block->step((float)dtime_s, [&](v3s16 p, MapNode n, f32 d) -> bool {
-		return !block->isOrphan() && m_script->node_on_timer(p, n, d);
+		return m_script->node_on_timer(p, n, d);
 	});
 }
 
@@ -1525,7 +1532,10 @@ void ServerEnvironment::step(float dtime)
 	if (m_active_blocks_nodemetadata_interval.step(dtime, m_cache_nodetimer_interval)) {
 		ScopeProfiler sp(g_profiler, "ServerEnv: Run node timers", SPT_AVG);
 
-		float dtime = m_cache_nodetimer_interval;
+		// FIXME: this is not actually correct, because the block may have been
+		// activated just moments ago. In practice the intervnal is very small
+		// so this doesn't really matter.
+		const float dtime = m_cache_nodetimer_interval;
 
 		for (const v3s16 &p: m_active_blocks.m_list) {
 			MapBlock *block = m_map->getBlockNoCreateNoEx(p);
@@ -1537,11 +1547,14 @@ void ServerEnvironment::step(float dtime)
 
 			// Set current time as timestamp
 			block->setTimestampNoChangedFlag(m_game_time);
-			// If time has changed much from the one on disk,
+			// If the block timestamp has changed considerably, mark it to be
-			// set block to be saved when it is unloaded
+			// re-saved. We do this even if there were no actual data changes
-			if(block->getTimestamp() > block->getDiskTimestamp() + 60)
+			// for the sake of LBMs.
+			if (block->getTimestamp() > block->getDiskTimestamp()
+				+ BLOCK_RESAVE_TIMESTAMP_DIFF) {
 				block->raiseModified(MOD_STATE_WRITE_AT_UNLOAD,
 					MOD_REASON_BLOCK_EXPIRED);
+			}
 
 			// Run node timers
 			block->step(dtime, [&](v3s16 p, MapNode n, f32 d) -> bool {
@@ -1558,6 +1571,7 @@ void ServerEnvironment::step(float dtime)
 		std::shuffle(m_abms.begin(), m_abms.end(), MyRandGenerator());
 
 		// Initialize handling of ActiveBlockModifiers
+		// TODO: reinitializing this state every time is probably not efficient?
 		ABMHandler abmhandler(m_abms, m_cache_abm_interval, this, true);
 
 		int blocks_scanned = 0;

src/serverenvironment.h

@@ -419,7 +419,7 @@ private:
 	static AuthDatabase *openAuthDatabase(const std::string &name,
 			const std::string &savedir, const Settings &conf);
 
-	void activateBlock(MapBlock *block, u32 additional_dtime=0);
+	void activateBlock(MapBlock *block);
 
 	/*
 		Internal ActiveObject interface
@@ -514,6 +514,16 @@ private:
 	// Can raise to high values like 15s with eg. map generation mods.
 	float m_max_lag_estimate = 0.1f;
 
+
+	/*
+	 * TODO: Add a callback function so these can be updated when a setting
+	 *       changes.
+	 */
+	float m_cache_active_block_mgmt_interval;
+	float m_cache_abm_interval;
+	float m_cache_nodetimer_interval;
+	float m_cache_abm_time_budget;
+
 	// peer_ids in here should be unique, except that there may be many 0s
 	std::vector<RemotePlayer*> m_players;