Disentangle map implementations (#12148)

Fixes violation of Liskov substitution principle
Fixes #12144

src/client/clientmap.h

@@ -76,9 +76,9 @@ public:
 
 	virtual ~ClientMap() = default;
 
-	s32 mapType() const
+	bool maySaveBlocks() override
 	{
-		return MAPTYPE_CLIENT;
+		return false;
 	}
 
 	void drop()

src/map.cpp

@@ -246,22 +246,6 @@ void Map::addNodeAndUpdate(v3s16 p, MapNode n,
 		action.setSetNode(p, rollback_oldnode, rollback_newnode);
 		m_gamedef->rollback()->reportAction(action);
 	}
-
-	/*
-		Add neighboring liquid nodes and this node to transform queue.
-		(it's vital for the node itself to get updated last, if it was removed.)
-	 */
-
-	for (const v3s16 &dir : g_7dirs) {
-		v3s16 p2 = p + dir;
-
-		bool is_valid_position;
-		MapNode n2 = getNode(p2, &is_valid_position);
-		if(is_valid_position &&
-				(m_nodedef->get(n2).isLiquid() ||
-				n2.getContent() == CONTENT_AIR))
-			m_transforming_liquid.push_back(p2);
-	}
 }
 
 void Map::removeNodeAndUpdate(v3s16 p,
@@ -342,7 +326,7 @@ struct TimeOrderedMapBlock {
 void Map::timerUpdate(float dtime, float unload_timeout, u32 max_loaded_blocks,
 		std::vector<v3s16> *unloaded_blocks)
 {
-	bool save_before_unloading = (mapType() == MAPTYPE_SERVER);
+	bool save_before_unloading = maySaveBlocks();
 
 	// Profile modified reasons
 	Profiler modprofiler;
@@ -527,11 +511,11 @@ struct NodeNeighbor {
 	{ }
 };
 
-void Map::transforming_liquid_add(v3s16 p) {
+void ServerMap::transforming_liquid_add(v3s16 p) {
         m_transforming_liquid.push_back(p);
 }
 
-void Map::transformLiquids(std::map<v3s16, MapBlock*> &modified_blocks,
+void ServerMap::transformLiquids(std::map<v3s16, MapBlock*> &modified_blocks,
 		ServerEnvironment *env)
 {
 	u32 loopcount = 0;
@@ -1565,6 +1549,29 @@ bool ServerMap::isBlockInQueue(v3s16 pos)
 	return m_emerge && m_emerge->isBlockInQueue(pos);
 }
 
+void ServerMap::addNodeAndUpdate(v3s16 p, MapNode n,
+		std::map<v3s16, MapBlock*> &modified_blocks,
+		bool remove_metadata)
+{
+	Map::addNodeAndUpdate(p, n, modified_blocks, remove_metadata);
+
+	/*
+		Add neighboring liquid nodes and this node to transform queue.
+		(it's vital for the node itself to get updated last, if it was removed.)
+	 */
+
+	for (const v3s16 &dir : g_7dirs) {
+		v3s16 p2 = p + dir;
+
+		bool is_valid_position;
+		MapNode n2 = getNode(p2, &is_valid_position);
+		if(is_valid_position &&
+				(m_nodedef->get(n2).isLiquid() ||
+				n2.getContent() == CONTENT_AIR))
+			m_transforming_liquid.push_back(p2);
+	}
+}
+
 // N.B.  This requires no synchronization, since data will not be modified unless
 // the VoxelManipulator being updated belongs to the same thread.
 void ServerMap::updateVManip(v3s16 pos)

src/map.h

@@ -54,10 +54,6 @@ struct BlockMakeData;
 	MapEditEvent
 */
 
-#define MAPTYPE_BASE 0
-#define MAPTYPE_SERVER 1
-#define MAPTYPE_CLIENT 2
-
 enum MapEditEventType{
 	// Node added (changed from air or something else to something)
 	MEET_ADDNODE,
@@ -127,11 +123,6 @@ public:
 	virtual ~Map();
 	DISABLE_CLASS_COPY(Map);
 
-	virtual s32 mapType() const
-	{
-		return MAPTYPE_BASE;
-	}
-
 	/*
 		Drop (client) or delete (server) the map.
 	*/
@@ -180,7 +171,7 @@ public:
 	/*
 		These handle lighting but not faces.
 	*/
-	void addNodeAndUpdate(v3s16 p, MapNode n,
+	virtual void addNodeAndUpdate(v3s16 p, MapNode n,
 			std::map<v3s16, MapBlock*> &modified_blocks,
 			bool remove_metadata = true);
 	void removeNodeAndUpdate(v3s16 p,
@@ -200,6 +191,11 @@ public:
 
 	virtual void save(ModifiedState save_level) { FATAL_ERROR("FIXME"); }
 
+	/*
+		Return true unless the map definitely cannot save blocks.
+	*/
+	virtual bool maySaveBlocks() { return true; }
+
 	// Server implements these.
 	// Client leaves them as no-op.
 	virtual bool saveBlock(MapBlock *block) { return false; }
@@ -207,14 +203,14 @@ public:
 
 	/*
 		Updates usage timers and unloads unused blocks and sectors.
-		Saves modified blocks before unloading on MAPTYPE_SERVER.
+		Saves modified blocks before unloading if possible.
 	*/
 	void timerUpdate(float dtime, float unload_timeout, u32 max_loaded_blocks,
 			std::vector<v3s16> *unloaded_blocks=NULL);
 
 	/*
 		Unloads all blocks with a zero refCount().
-		Saves modified blocks before unloading on MAPTYPE_SERVER.
+		Saves modified blocks before unloading if possible.
 	*/
 	void unloadUnreferencedBlocks(std::vector<v3s16> *unloaded_blocks=NULL);
 
@@ -226,9 +222,6 @@ public:
 	// For debug printing. Prints "Map: ", "ServerMap: " or "ClientMap: "
 	virtual void PrintInfo(std::ostream &out);
 
-	void transformLiquids(std::map<v3s16, MapBlock*> & modified_blocks,
-			ServerEnvironment *env);
-
 	/*
 		Node metadata
 		These are basically coordinate wrappers to MapBlock
@@ -267,12 +260,8 @@ public:
 		Variables
 	*/
 
-	void transforming_liquid_add(v3s16 p);
-
 	bool isBlockOccluded(MapBlock *block, v3s16 cam_pos_nodes);
 protected:
-	friend class LuaVoxelManip;
-
 	IGameDef *m_gamedef;
 
 	std::set<MapEventReceiver*> m_event_receivers;
@@ -283,9 +272,6 @@ protected:
 	MapSector *m_sector_cache = nullptr;
 	v2s16 m_sector_cache_p;
 
-	// Queued transforming water nodes
-	UniqueQueue<v3s16> m_transforming_liquid;
-
 	// This stores the properties of the nodes on the map.
 	const NodeDefManager *m_nodedef;
 
@@ -294,12 +280,6 @@ protected:
 	bool isOccluded(const v3s16 &pos_camera, const v3s16 &pos_target,
 		float step, float stepfac, float start_offset, float end_offset,
 		u32 needed_count);
-
-private:
-	f32 m_transforming_liquid_loop_count_multiplier = 1.0f;
-	u32 m_unprocessed_count = 0;
-	u64 m_inc_trending_up_start_time = 0; // milliseconds
-	bool m_queue_size_timer_started = false;
 };
 
 /*
@@ -317,11 +297,6 @@ public:
 	ServerMap(const std::string &savedir, IGameDef *gamedef, EmergeManager *emerge, MetricsBackend *mb);
 	~ServerMap();
 
-	s32 mapType() const
-	{
-		return MAPTYPE_SERVER;
-	}
-
 	/*
 		Get a sector from somewhere.
 		- Check memory
@@ -364,6 +339,10 @@ public:
 
 	bool isBlockInQueue(v3s16 pos);
 
+	void addNodeAndUpdate(v3s16 p, MapNode n,
+			std::map<v3s16, MapBlock*> &modified_blocks,
+			bool remove_metadata) override;
+
 	/*
 		Database functions
 	*/
@@ -406,9 +385,16 @@ public:
 	bool repairBlockLight(v3s16 blockpos,
 		std::map<v3s16, MapBlock *> *modified_blocks);
 
+	void transformLiquids(std::map<v3s16, MapBlock*> & modified_blocks,
+			ServerEnvironment *env);
+
+	void transforming_liquid_add(v3s16 p);
+
 	MapSettingsManager settings_mgr;
 
 private:
+	friend class LuaVoxelManip;
+
 	// Emerge manager
 	EmergeManager *m_emerge;
 
@@ -419,6 +405,13 @@ private:
 
 	std::set<v3s16> m_chunks_in_progress;
 
+	// Queued transforming water nodes
+	UniqueQueue<v3s16> m_transforming_liquid;
+	f32 m_transforming_liquid_loop_count_multiplier = 1.0f;
+	u32 m_unprocessed_count = 0;
+	u64 m_inc_trending_up_start_time = 0; // milliseconds
+	bool m_queue_size_timer_started = false;
+
 	/*
 		Metadata is re-written on disk only if this is true.
 		This is reset to false when written on disk.

src/script/lua_api/l_env.cpp

@@ -1386,7 +1386,7 @@ int ModApiEnvMod::l_transforming_liquid_add(lua_State *L)
 	GET_ENV_PTR;
 
 	v3s16 p0 = read_v3s16(L, 1);
-	env->getMap().transforming_liquid_add(p0);
+	env->getServerMap().transforming_liquid_add(p0);
 	return 1;
 }
 

src/script/lua_api/l_vmanip.cpp

@@ -166,7 +166,7 @@ int LuaVoxelManip::l_update_liquids(lua_State *L)
 
 	LuaVoxelManip *o = checkobject(L, 1);
 
-	Map *map = &(env->getMap());
+	ServerMap *map = &(env->getServerMap());
 	const NodeDefManager *ndef = getServer(L)->getNodeDefManager();
 	MMVManip *vm = o->vm;
 

src/server.cpp

@@ -665,7 +665,7 @@ void Server::AsyncRunStep(bool initial_step)
 		ScopeProfiler sp(g_profiler, "Server: liquid transform");
 
 		std::map<v3s16, MapBlock*> modified_blocks;
-		m_env->getMap().transformLiquids(modified_blocks, m_env);
+		m_env->getServerMap().transformLiquids(modified_blocks, m_env);
 
 		/*
 			Set the modified blocks unsent for all the clients