General code refactoring/improvements in server, treegen and connection

src/client/clientmap.h

@@ -57,18 +57,11 @@ public:
 			s32 id
 	);
 
-	virtual ~ClientMap();
-
 	bool maySaveBlocks() override
 	{
 		return false;
 	}
 
-	void drop() override
-	{
-		ISceneNode::drop(); // calls destructor
-	}
-
 	void updateCamera(v3f pos, v3f dir, f32 fov, v3s16 offset, video::SColor light_color);
 
 	/*
@@ -122,6 +115,9 @@ public:
 	void onSettingChanged(const std::string &name);
 
 protected:
+	// use drop() instead
+	virtual ~ClientMap();
+
 	void reportMetrics(u64 save_time_us, u32 saved_blocks, u32 all_blocks) override;
 private:
 	bool isMeshOccluded(MapBlock *mesh_block, u16 mesh_size, v3s16 cam_pos_nodes);

src/collision.cpp

@@ -89,7 +89,7 @@ static inline v3f truncate(const v3f& vec, const f32 factor)
 // The time after which the collision occurs is stored in dtime.
 CollisionAxis axisAlignedCollision(
 		const aabb3f &staticbox, const aabb3f &movingbox,
-		const v3f &speed, f32 *dtime)
+		const v3f speed, f32 *dtime)
 {
 	//TimeTaker tt("axisAlignedCollision");
 

src/collision.h

@@ -78,7 +78,7 @@ collisionMoveResult collisionMoveSimple(Environment *env,IGameDef *gamedef,
 // dtime receives time until first collision, invalid if -1 is returned
 CollisionAxis axisAlignedCollision(
 		const aabb3f &staticbox, const aabb3f &movingbox,
-		const v3f &speed, f32 *dtime);
+		v3f speed, f32 *dtime);
 
 // Helper function:
 // Checks if moving the movingbox up by the given distance would hit a ceiling.

src/database/database-files.cpp

@@ -25,6 +25,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "filesys.h"
 #include "server/player_sao.h"
 #include "util/string.h"
+#include <json/json.h>
 #include <cassert>
 
 // !!! WARNING !!!

src/database/database-files.h

@@ -94,7 +94,6 @@ public:
 
 private:
 	Json::Value *getOrCreateJson(const std::string &modname);
-	bool writeJson(const std::string &modname, const Json::Value &json);
 
 	std::string m_storage_dir;
 	std::unordered_map<std::string, Json::Value> m_mod_storage;

src/emerge.cpp

@@ -646,7 +646,7 @@ void *EmergeThread::run()
 	std::map<v3s16, MapBlock *> modified_blocks;
 
 	m_map    = &m_server->m_env->getServerMap();
-	m_emerge = m_server->m_emerge;
+	m_emerge = m_server->getEmergeManager();
 	m_mapgen = m_emerge->m_mapgens[id];
 	enable_mapgen_debug_info = m_emerge->enable_mapgen_debug_info;
 

src/httpfetch.cpp

@@ -19,8 +19,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "httpfetch.h"
 #include "porting.h" // for sleep_ms(), get_sysinfo(), secure_rand_fill_buf()
-#include <iostream>
-#include <sstream>
 #include <list>
 #include <unordered_map>
 #include <cerrno>
@@ -152,9 +150,9 @@ bool httpfetch_async_get(u64 caller, HTTPFetchResult &fetch_result)
 static size_t httpfetch_writefunction(
 		char *ptr, size_t size, size_t nmemb, void *userdata)
 {
-	std::ostringstream *stream = (std::ostringstream*)userdata;
+	auto *dest = reinterpret_cast<std::string*>(userdata);
 	size_t count = size * nmemb;
-	stream->write(ptr, count);
+	dest->append(ptr, count);
 	return count;
 }
 
@@ -215,7 +213,6 @@ private:
 	CURLM *multi = nullptr;
 	HTTPFetchRequest request;
 	HTTPFetchResult result;
-	std::ostringstream oss;
 	struct curl_slist *http_header = nullptr;
 	curl_mime *multipart_mime = nullptr;
 };
@@ -225,8 +222,7 @@ HTTPFetchOngoing::HTTPFetchOngoing(const HTTPFetchRequest &request_,
 		CurlHandlePool *pool_):
 	pool(pool_),
 	request(request_),
-	result(request_),
-	oss(std::ios::binary)
+	result(request_)
 {
 	curl = pool->alloc();
 	if (!curl)
@@ -277,16 +273,15 @@ HTTPFetchOngoing::HTTPFetchOngoing(const HTTPFetchRequest &request_,
 		curl_easy_setopt(curl, CURLOPT_USERAGENT, request.useragent.c_str());
 
 	// Set up a write callback that writes to the
-	// ostringstream ongoing->oss, unless the data
-	// is to be discarded
+	// result struct, unless the data is to be discarded
 	if (request.caller == HTTPFETCH_DISCARD) {
 		curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
 				httpfetch_discardfunction);
-		curl_easy_setopt(curl, CURLOPT_WRITEDATA, NULL);
+		curl_easy_setopt(curl, CURLOPT_WRITEDATA, nullptr);
 	} else {
 		curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
 				httpfetch_writefunction);
-		curl_easy_setopt(curl, CURLOPT_WRITEDATA, &oss);
+		curl_easy_setopt(curl, CURLOPT_WRITEDATA, &result.data);
 	}
 
 	// Set data from fields or raw_data
@@ -372,7 +367,6 @@ const HTTPFetchResult * HTTPFetchOngoing::complete(CURLcode res)
 {
 	result.succeeded = (res == CURLE_OK);
 	result.timeout = (res == CURLE_OPERATION_TIMEDOUT);
-	result.data = oss.str();
 
 	// Get HTTP/FTP response code
 	result.response_code = 0;

src/map.cpp

@@ -600,7 +600,7 @@ void Map::removeNodeTimer(v3s16 p)
 	block->removeNodeTimer(p_rel);
 }
 
-bool Map::determineAdditionalOcclusionCheck(const v3s16 &pos_camera,
+bool Map::determineAdditionalOcclusionCheck(const v3s16 pos_camera,
 	const core::aabbox3d<s16> &block_bounds, v3s16 &check)
 {
 	/*
@@ -664,7 +664,7 @@ bool Map::determineAdditionalOcclusionCheck(const v3s16 &pos_camera,
 	return false;
 }
 
-bool Map::isOccluded(const v3s16 &pos_camera, const v3s16 &pos_target,
+bool Map::isOccluded(const v3s16 pos_camera, const v3s16 pos_target,
 	float step, float stepfac, float offset, float end_offset, u32 needed_count)
 {
 	v3f direction = intToFloat(pos_target - pos_camera, BS);

src/map.h

@@ -120,14 +120,6 @@ public:
 	virtual ~Map();
 	DISABLE_CLASS_COPY(Map);
 
-	/*
-		Drop (client) or delete (server) the map.
-	*/
-	virtual void drop()
-	{
-		delete this;
-	}
-
 	void addEventReceiver(MapEventReceiver *event_receiver);
 	void removeEventReceiver(MapEventReceiver *event_receiver);
 	// event shall be deleted by caller after the call.
@@ -314,9 +306,9 @@ protected:
 	// Can be implemented by child class
 	virtual void reportMetrics(u64 save_time_us, u32 saved_blocks, u32 all_blocks) {}
 
-	bool determineAdditionalOcclusionCheck(const v3s16 &pos_camera,
-		const core::aabbox3d<s16> &block_bounds, v3s16 &check);
-	bool isOccluded(const v3s16 &pos_camera, const v3s16 &pos_target,
+	bool determineAdditionalOcclusionCheck(v3s16 pos_camera,
+		const core::aabbox3d<s16> &block_bounds, v3s16 &to_check);
+	bool isOccluded(v3s16 pos_camera, v3s16 pos_target,
 		float step, float stepfac, float start_offset, float end_offset,
 		u32 needed_count);
 };

src/mapgen/mg_decoration.cpp

@@ -492,5 +492,5 @@ size_t DecoLSystem::generate(MMVManip *vm, PcgRandom *pr, v3s16 p, bool ceiling)
 
 	// Make sure that tree_def can't be modified, since it is shared.
 	const auto &ref = *tree_def;
-	return treegen::make_ltree(*vm, p, m_ndef, ref);
+	return treegen::make_ltree(*vm, p, ref);
 }

src/mapgen/treegen.cpp

@@ -19,14 +19,14 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 
-#include "irr_v3d.h"
 #include <stack>
+#include "treegen.h"
+#include "irr_v3d.h"
 #include "util/pointer.h"
 #include "util/numeric.h"
 #include "servermap.h"
 #include "mapblock.h"
-#include "nodedef.h"
-#include "treegen.h"
+#include "noise.h"
 #include "voxelalgorithms.h"
 
 namespace treegen
@@ -42,6 +42,23 @@ void TreeDef::resolveNodeNames()
 		getIdFromNrBacklog(&fruitnode.param0, "", CONTENT_IGNORE);
 }
 
+/*
+	L-System tree gen helper functions
+
+	NOTE: the PseudoRandom parameters here were probably accidentally used
+	as by-value instead of by-reference. But don't change this now to keep
+	the old behaviour.
+*/
+static void tree_trunk_placement(MMVManip &vmanip, v3f p0, const TreeDef &def);
+static void tree_leaves_placement(MMVManip &vmanip, v3f p0,
+	PseudoRandom ps, const TreeDef &def);
+static void tree_single_leaves_placement(MMVManip &vmanip, v3f p0,
+	PseudoRandom ps, const TreeDef &def);
+static void tree_fruit_placement(MMVManip &vmanip, v3f p0, const TreeDef &def);
+
+static void setRotationAxisRadians(core::matrix4 &M, float angle, v3f axis);
+static v3f transposeMatrix(const core::matrix4 &M, v3f v);
+
 void make_tree(MMVManip &vmanip, v3s16 p0, bool is_apple_tree,
 	const NodeDefManager *ndef, s32 seed)
 {
@@ -128,20 +145,18 @@ void make_tree(MMVManip &vmanip, v3s16 p0, bool is_apple_tree,
 }
 
 
-// L-System tree LUA spawner
 treegen::error spawn_ltree(ServerMap *map, v3s16 p0,
-	const NodeDefManager *ndef, const TreeDef &tree_definition)
+	const TreeDef &tree_definition)
 {
-	std::map<v3s16, MapBlock*> modified_blocks;
 	MMVManip vmanip(map);
 	v3s16 tree_blockp = getNodeBlockPos(p0);
-	treegen::error e;
 
 	vmanip.initialEmerge(tree_blockp - v3s16(1, 1, 1), tree_blockp + v3s16(1, 3, 1));
-	e = make_ltree(vmanip, p0, ndef, tree_definition);
+	treegen::error e = make_ltree(vmanip, p0, tree_definition);
 	if (e != SUCCESS)
 		return e;
 
+	std::map<v3s16, MapBlock*> modified_blocks;
 	voxalgo::blit_back_with_light(map, &vmanip, &modified_blocks);
 
 	// Send a MEET_OTHER event
@@ -153,9 +168,8 @@ treegen::error spawn_ltree(ServerMap *map, v3s16 p0,
 }
 
 
-//L-System tree generator
 treegen::error make_ltree(MMVManip &vmanip, v3s16 p0,
-	const NodeDefManager *ndef, TreeDef tree_definition)
+	const TreeDef &tree_definition)
 {
 	s32 seed;
 	if (tree_definition.explicit_seed)
@@ -165,10 +179,10 @@ treegen::error make_ltree(MMVManip &vmanip, v3s16 p0,
 	PseudoRandom ps(seed);
 
 	// chance of inserting abcd rules
-	double prop_a = 9;
-	double prop_b = 8;
-	double prop_c = 7;
-	double prop_d = 6;
+	constexpr float prop_a = 9;
+	constexpr float prop_b = 8;
+	constexpr float prop_c = 7;
+	constexpr float prop_d = 6;
 
 	//randomize tree growth level, minimum=2
 	s16 iterations = tree_definition.iterations;
@@ -177,13 +191,13 @@ treegen::error make_ltree(MMVManip &vmanip, v3s16 p0,
 	if (iterations < 2)
 		iterations = 2;
 
-	s16 MAX_ANGLE_OFFSET = 5;
-	double angle_in_radians = (double)tree_definition.angle * M_PI / 180;
-	double angleOffset_in_radians = (s16)(ps.range(0, 1) % MAX_ANGLE_OFFSET) * M_PI / 180;
+	constexpr s16 MAX_ANGLE_OFFSET = 5;
+	float angle_in_radians = tree_definition.angle * M_PI / 180;
+	float angleOffset_in_radians = (s16)(ps.range(0, 1) % MAX_ANGLE_OFFSET) * M_PI / 180;
 
 	//initialize rotation matrix, position and stacks for branches
 	core::matrix4 rotation;
-	rotation = setRotationAxisRadians(rotation, M_PI / 2, v3f(0, 0, 1));
+	setRotationAxisRadians(rotation, M_PI / 2, v3f(0, 0, 1));
 	v3f position;
 	position.X = p0.X;
 	position.Y = p0.Y;
@@ -494,37 +508,37 @@ treegen::error make_ltree(MMVManip &vmanip, v3s16 p0,
 			break;
 		case '+':
 			temp_rotation.makeIdentity();
-			temp_rotation = setRotationAxisRadians(temp_rotation,
+			setRotationAxisRadians(temp_rotation,
 					angle_in_radians + angleOffset_in_radians, v3f(0, 0, 1));
 			rotation *= temp_rotation;
 			break;
 		case '-':
 			temp_rotation.makeIdentity();
-			temp_rotation = setRotationAxisRadians(temp_rotation,
+			setRotationAxisRadians(temp_rotation,
 					angle_in_radians + angleOffset_in_radians, v3f(0, 0, -1));
 			rotation *= temp_rotation;
 			break;
 		case '&':
 			temp_rotation.makeIdentity();
-			temp_rotation = setRotationAxisRadians(temp_rotation,
+			setRotationAxisRadians(temp_rotation,
 					angle_in_radians + angleOffset_in_radians, v3f(0, 1, 0));
 			rotation *= temp_rotation;
 			break;
 		case '^':
 			temp_rotation.makeIdentity();
-			temp_rotation = setRotationAxisRadians(temp_rotation,
+			setRotationAxisRadians(temp_rotation,
 					angle_in_radians + angleOffset_in_radians, v3f(0, -1, 0));
 			rotation *= temp_rotation;
 			break;
 		case '*':
 			temp_rotation.makeIdentity();
-			temp_rotation = setRotationAxisRadians(temp_rotation,
+			setRotationAxisRadians(temp_rotation,
 					angle_in_radians, v3f(1, 0, 0));
 			rotation *= temp_rotation;
 			break;
 		case '/':
 			temp_rotation.makeIdentity();
-			temp_rotation = setRotationAxisRadians(temp_rotation,
+			setRotationAxisRadians(temp_rotation,
 					angle_in_radians, v3f(-1, 0, 0));
 			rotation *= temp_rotation;
 			break;
@@ -537,7 +551,7 @@ treegen::error make_ltree(MMVManip &vmanip, v3s16 p0,
 }
 
 
-void tree_trunk_placement(MMVManip &vmanip, v3f p0, TreeDef &tree_definition)
+void tree_trunk_placement(MMVManip &vmanip, v3f p0, const TreeDef &tree_definition)
 {
 	v3s16 p1 = v3s16(myround(p0.X), myround(p0.Y), myround(p0.Z));
 	if (!vmanip.m_area.contains(p1))
@@ -554,7 +568,7 @@ void tree_trunk_placement(MMVManip &vmanip, v3f p0, TreeDef &tree_definition)
 
 
 void tree_leaves_placement(MMVManip &vmanip, v3f p0,
-		PseudoRandom ps, TreeDef &tree_definition)
+		PseudoRandom ps, const TreeDef &tree_definition)
 {
 	MapNode leavesnode = tree_definition.leavesnode;
 	if (ps.range(1, 100) > 100 - tree_definition.leaves2_chance)
@@ -578,7 +592,7 @@ void tree_leaves_placement(MMVManip &vmanip, v3f p0,
 
 
 void tree_single_leaves_placement(MMVManip &vmanip, v3f p0,
-		PseudoRandom ps, TreeDef &tree_definition)
+		PseudoRandom ps, const TreeDef &tree_definition)
 {
 	MapNode leavesnode = tree_definition.leavesnode;
 	if (ps.range(1, 100) > 100 - tree_definition.leaves2_chance)
@@ -594,7 +608,7 @@ void tree_single_leaves_placement(MMVManip &vmanip, v3f p0,
 }
 
 
-void tree_fruit_placement(MMVManip &vmanip, v3f p0, TreeDef &tree_definition)
+void tree_fruit_placement(MMVManip &vmanip, v3f p0, const TreeDef &tree_definition)
 {
 	v3s16 p1 = v3s16(myround(p0.X), myround(p0.Y), myround(p0.Z));
 	if (!vmanip.m_area.contains(p1))
@@ -607,18 +621,18 @@ void tree_fruit_placement(MMVManip &vmanip, v3f p0, TreeDef &tree_definition)
 }
 
 
-irr::core::matrix4 setRotationAxisRadians(irr::core::matrix4 M, double angle, v3f axis)
+void setRotationAxisRadians(core::matrix4 &M, float angle, v3f axis)
 {
-	double c = cos(angle);
-	double s = sin(angle);
-	double t = 1.0 - c;
+	float c = std::cos(angle);
+	float s = std::sin(angle);
+	float t = 1.0f - c;
 
-	double tx  = t * axis.X;
-	double ty  = t * axis.Y;
-	double tz  = t * axis.Z;
-	double sx  = s * axis.X;
-	double sy  = s * axis.Y;
-	double sz  = s * axis.Z;
+	float tx  = t * axis.X;
+	float ty  = t * axis.Y;
+	float tz  = t * axis.Z;
+	float sx  = s * axis.X;
+	float sy  = s * axis.Y;
+	float sz  = s * axis.Z;
 
 	M[0] = tx * axis.X + c;
 	M[1] = tx * axis.Y + sz;
@@ -631,19 +645,15 @@ irr::core::matrix4 setRotationAxisRadians(irr::core::matrix4 M, double angle, v3
 	M[8]  = tz * axis.X + sy;
 	M[9]  = tz * axis.Y - sx;
 	M[10] = tz * axis.Z + c;
-	return M;
 }
 
 
-v3f transposeMatrix(irr::core::matrix4 M, v3f v)
+v3f transposeMatrix(const core::matrix4 &M, v3f v)
 {
 	v3f translated;
-	double x = M[0] * v.X + M[4] * v.Y + M[8]  * v.Z +M[12];
-	double y = M[1] * v.X + M[5] * v.Y + M[9]  * v.Z +M[13];
-	double z = M[2] * v.X + M[6] * v.Y + M[10] * v.Z +M[14];
-	translated.X = x;
-	translated.Y = y;
-	translated.Z = z;
+	translated.X = M[0] * v.X + M[4] * v.Y + M[8]  * v.Z + M[12];
+	translated.Y = M[1] * v.X + M[5] * v.Y + M[9]  * v.Z + M[13];
+	translated.Z = M[2] * v.X + M[6] * v.Y + M[10] * v.Z + M[14];
 	return translated;
 }
 

src/mapgen/treegen.h

@@ -21,8 +21,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #pragma once
 
-#include <matrix4.h>
-#include "noise.h"
+#include <string>
+#include "irr_v3d.h"
+#include "nodedef.h"
+#include "mapnode.h"
 
 class MMVManip;
 class NodeDefManager;
@@ -77,24 +79,8 @@ namespace treegen {
 	void make_pine_tree(MMVManip &vmanip, v3s16 p0,
 		const NodeDefManager *ndef, s32 seed);
 
-	// Add L-Systems tree (used by engine)
-	treegen::error make_ltree(MMVManip &vmanip, v3s16 p0,
-		const NodeDefManager *ndef, TreeDef tree_definition);
-	// Spawn L-systems tree from LUA
-	treegen::error spawn_ltree (ServerMap *map, v3s16 p0,
-		const NodeDefManager *ndef, const TreeDef &tree_definition);
-
-	// L-System tree gen helper functions
-	void tree_trunk_placement(MMVManip &vmanip, v3f p0,
-		TreeDef &tree_definition);
-	void tree_leaves_placement(MMVManip &vmanip, v3f p0,
-		PseudoRandom ps, TreeDef &tree_definition);
-	void tree_single_leaves_placement(MMVManip &vmanip, v3f p0,
-		PseudoRandom ps, TreeDef &tree_definition);
-	void tree_fruit_placement(MMVManip &vmanip, v3f p0,
-		TreeDef &tree_definition);
-	irr::core::matrix4 setRotationAxisRadians(irr::core::matrix4 M, double angle, v3f axis);
-
-	v3f transposeMatrix(irr::core::matrix4 M ,v3f v);
-
+	// Spawn L-Systems tree on VManip
+	treegen::error make_ltree(MMVManip &vmanip, v3s16 p0, const TreeDef &def);
+	// Helper to spawn it directly on map
+	treegen::error spawn_ltree(ServerMap *map, v3s16 p0, const TreeDef &def);
 }; // namespace treegen

src/network/connection.cpp

@@ -68,7 +68,7 @@ u16 BufferedPacket::getSeqnum() const
 	return readU16(&data[BASE_HEADER_SIZE + 1]);
 }
 
-BufferedPacketPtr makePacket(Address &address, const SharedBuffer<u8> &data,
+BufferedPacketPtr makePacket(const Address &address, const SharedBuffer<u8> &data,
 		u32 protocol_id, session_t sender_peer_id, u8 channel)
 {
 	u32 packet_size = data.getSize() + BASE_HEADER_SIZE;
@@ -834,13 +834,6 @@ void Channel::UpdateTimers(float dtime)
 	Peer
 */
 
-PeerHelper::PeerHelper(Peer* peer) :
-	m_peer(peer)
-{
-	if (peer && !peer->IncUseCount())
-		m_peer = nullptr;
-}
-
 PeerHelper::~PeerHelper()
 {
 	if (m_peer)
@@ -851,32 +844,14 @@ PeerHelper::~PeerHelper()
 
 PeerHelper& PeerHelper::operator=(Peer* peer)
 {
+	if (m_peer)
+		m_peer->DecUseCount();
 	m_peer = peer;
 	if (peer && !peer->IncUseCount())
 		m_peer = nullptr;
 	return *this;
 }
 
-Peer* PeerHelper::operator->() const
-{
-	return m_peer;
-}
-
-Peer* PeerHelper::operator&() const
-{
-	return m_peer;
-}
-
-bool PeerHelper::operator!()
-{
-	return ! m_peer;
-}
-
-bool PeerHelper::operator!=(void* ptr)
-{
-	return ((void*) m_peer != ptr);
-}
-
 bool Peer::IncUseCount()
 {
 	MutexAutoLock lock(m_exclusive_access_mutex);
@@ -989,8 +964,8 @@ void Peer::Drop()
 	delete this;
 }
 
-UDPPeer::UDPPeer(session_t a_id, Address a_address, Connection* connection) :
-	Peer(a_address,a_id,connection)
+UDPPeer::UDPPeer(session_t id, const Address &address, Connection *connection) :
+	Peer(id, address, connection)
 {
 	for (Channel &channel : channels)
 		channel.setWindowSize(START_RELIABLE_WINDOW_SIZE);
@@ -1014,17 +989,6 @@ bool UDPPeer::isTimedOut(float timeout, std::string &reason)
 	return false;
 }
 
-bool UDPPeer::getAddress(MTProtocols type,Address& toset)
-{
-	if ((type == MTP_UDP) || (type == MTP_MINETEST_RELIABLE_UDP) || (type == MTP_PRIMARY))
-	{
-		toset = address;
-		return true;
-	}
-
-	return false;
-}
-
 void UDPPeer::reportRTT(float rtt)
 {
 	if (rtt < 0.0) {
@@ -1377,20 +1341,12 @@ PeerHelper Connection::getPeerNoEx(session_t peer_id)
 session_t Connection::lookupPeer(const Address& sender)
 {
 	MutexAutoLock peerlock(m_peers_mutex);
-	std::map<u16, Peer*>::iterator j;
-	j = m_peers.begin();
-	for(; j != m_peers.end(); ++j)
-	{
-		Peer *peer = j->second;
+	for (auto &it: m_peers) {
+		Peer *peer = it.second;
 		if (peer->isPendingDeletion())
 			continue;
 
-		Address tocheck;
-
-		if ((peer->getAddress(MTP_MINETEST_RELIABLE_UDP, tocheck)) && (tocheck == sender))
-			return peer->id;
-
-		if ((peer->getAddress(MTP_UDP, tocheck)) && (tocheck == sender))
+		if (peer->getAddress() == sender)
 			return peer->id;
 	}
 
@@ -1427,12 +1383,8 @@ bool Connection::deletePeer(session_t peer_id, bool timeout)
 		m_peer_ids.erase(it);
 	}
 
-	Address peer_address;
-	//any peer has a primary address this never fails!
-	peer->getAddress(MTP_PRIMARY, peer_address);
-
 	// Create event
-	putEvent(ConnectionEvent::peerRemoved(peer_id, timeout, peer_address));
+	putEvent(ConnectionEvent::peerRemoved(peer_id, timeout, peer->getAddress()));
 
 	peer->Drop();
 	return true;
@@ -1562,15 +1514,14 @@ Address Connection::GetPeerAddress(session_t peer_id)
 
 	if (!peer)
 		throw PeerNotFoundException("No address for peer found!");
-	Address peer_address;
-	peer->getAddress(MTP_PRIMARY, peer_address);
-	return peer_address;
+	return peer->getAddress();
 }
 
 float Connection::getPeerStat(session_t peer_id, rtt_stat_type type)
 {
 	PeerHelper peer = getPeerNoEx(peer_id);
-	if (!peer) return -1;
+	if (!peer)
+		return -1;
 	return peer->getStat(type);
 }
 
@@ -1580,7 +1531,7 @@ float Connection::getLocalStat(rate_stat_type type)
 
 	FATAL_ERROR_IF(!peer, "Connection::getLocalStat we couldn't get our own peer? are you serious???");
 
-	float retval = 0.0;
+	float retval = 0;
 
 	for (Channel &channel : dynamic_cast<UDPPeer *>(&peer)->channels) {
 		switch(type) {
@@ -1609,7 +1560,7 @@ float Connection::getLocalStat(rate_stat_type type)
 	return retval;
 }
 
-session_t Connection::createPeer(Address& sender, MTProtocols protocol, int fd)
+session_t Connection::createPeer(const Address &sender, int fd)
 {
 	// Somebody wants to make a new connection
 
@@ -1694,12 +1645,10 @@ void Connection::sendAck(session_t peer_id, u8 channelnum, u16 seqnum)
 	m_sendThread->Trigger();
 }
 
-UDPPeer* Connection::createServerPeer(Address& address)
+UDPPeer* Connection::createServerPeer(const Address &address)
 {
 	if (ConnectedToServer())
-	{
 		throw ConnectionException("Already connected to a server");
-	}
 
 	UDPPeer *peer = new UDPPeer(PEER_ID_SERVER, address, this);
 	peer->SetFullyOpen();

src/network/connection.h

@@ -25,8 +25,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "constants.h"
 #include "util/pointer.h"
 #include "util/container.h"
-#include "util/thread.h"
 #include "util/numeric.h"
+#include "porting.h"
 #include "networkprotocol.h"
 #include <iostream>
 #include <vector>
@@ -40,12 +40,6 @@ namespace con
 class ConnectionReceiveThread;
 class ConnectionSendThread;
 
-enum MTProtocols {
-	MTP_PRIMARY,
-	MTP_UDP,
-	MTP_MINETEST_RELIABLE_UDP
-};
-
 enum rate_stat_type {
 	CUR_DL_RATE,
 	AVG_DL_RATE,
@@ -57,18 +51,20 @@ enum rate_stat_type {
 
 class Peer;
 
+// FIXME: Peer refcounting should generally be replaced by std::shared_ptr
 class PeerHelper
 {
 public:
 	PeerHelper() = default;
-	PeerHelper(Peer* peer);
+	inline PeerHelper(Peer *peer) { *this = peer; }
 	~PeerHelper();
 
-	PeerHelper&   operator=(Peer* peer);
-	Peer*         operator->() const;
-	bool          operator!();
-	Peer*         operator&() const;
-	bool          operator!=(void* ptr);
+	PeerHelper& operator=(Peer *peer);
+	inline Peer* operator->() const { return m_peer; }
+	inline Peer* operator&() const { return m_peer; }
+
+	inline bool operator!() { return !m_peer; }
+	inline bool operator!=(std::nullptr_t) { return !!m_peer; }
 
 private:
 	Peer *m_peer = nullptr;
@@ -127,18 +123,10 @@ class Peer {
 	public:
 		friend class PeerHelper;
 
-		Peer(Address address_,session_t id_,Connection* connection) :
-			id(id_),
-			m_connection(connection),
-			address(address_),
-			m_last_timeout_check(porting::getTimeMs())
-		{
-		};
-
 		virtual ~Peer() {
 			MutexAutoLock usage_lock(m_exclusive_access_mutex);
 			FATAL_ERROR_IF(m_usage != 0, "Reference counting failure");
-		};
+		}
 
 		// Unique id of the peer
 		const session_t id;
@@ -148,16 +136,25 @@ class Peer {
 		virtual void PutReliableSendCommand(ConnectionCommandPtr &c,
 						unsigned int max_packet_size) {};
 
-		virtual bool getAddress(MTProtocols type, Address& toset) = 0;
+		virtual const Address &getAddress() const = 0;
 
-		bool isPendingDeletion()
-		{ MutexAutoLock lock(m_exclusive_access_mutex); return m_pending_deletion; };
+		bool isPendingDeletion() const {
+			MutexAutoLock lock(m_exclusive_access_mutex);
+			return m_pending_deletion;
+		}
+		void ResetTimeout() {
+			MutexAutoLock lock(m_exclusive_access_mutex);
+			m_timeout_counter = 0;
+		}
 
-		void ResetTimeout()
-			{MutexAutoLock lock(m_exclusive_access_mutex); m_timeout_counter = 0.0; };
-
-		bool isHalfOpen() const { return m_half_open; }
-		void SetFullyOpen() { m_half_open = false; }
+		bool isHalfOpen() const {
+			MutexAutoLock lock(m_exclusive_access_mutex);
+			return m_half_open;
+		}
+		void SetFullyOpen() {
+			MutexAutoLock lock(m_exclusive_access_mutex);
+			m_half_open = false;
+		}
 
 		virtual bool isTimedOut(float timeout, std::string &reason);
 
@@ -168,10 +165,8 @@ class Peer {
 		virtual SharedBuffer<u8> addSplitPacket(u8 channel, BufferedPacketPtr &toadd,
 				bool reliable)
 		{
-			errorstream << "Peer::addSplitPacket called,"
-					<< " this is supposed to be never called!" << std::endl;
-			return SharedBuffer<u8>(0);
-		};
+			FATAL_ERROR("unimplemented in abstract class");
+		}
 
 		virtual bool Ping(float dtime, SharedBuffer<u8>& data) { return false; };
 
@@ -192,7 +187,16 @@ class Peer {
 			}
 			return -1;
 		}
+
 	protected:
+		Peer(session_t id, const Address &address, Connection *connection) :
+			id(id),
+			m_connection(connection),
+			address(address),
+			m_last_timeout_check(porting::getTimeMs())
+		{
+		}
+
 		virtual void reportRTT(float rtt) {};
 
 		void RTTStatistics(float rtt,
@@ -206,15 +210,15 @@ class Peer {
 
 		bool m_pending_deletion = false;
 
-		Connection* m_connection;
+		Connection *m_connection;
 
 		// Address of the peer
 		Address address;
 
 		// Ping timer
 		float m_ping_timer = 0.0f;
-	private:
 
+	private:
 		struct rttstats {
 			float jitter_min = FLT_MAX;
 			float jitter_max = 0.0f;
@@ -222,8 +226,6 @@ class Peer {
 			float min_rtt = FLT_MAX;
 			float max_rtt = 0.0f;
 			float avg_rtt = -1.0f;
-
-			rttstats() = default;
 		};
 
 		rttstats m_rtt;
@@ -285,8 +287,8 @@ protected:
 	PeerHelper getPeerNoEx(session_t peer_id);
 	session_t   lookupPeer(const Address& sender);
 
-	session_t createPeer(Address& sender, MTProtocols protocol, int fd);
-	UDPPeer*  createServerPeer(Address& sender);
+	session_t createPeer(const Address& sender, int fd);
+	UDPPeer*  createServerPeer(const Address& sender);
 	bool deletePeer(session_t peer_id, bool timeout);
 
 	void SetPeerID(session_t id) { m_peer_id = id; }

src/network/connection_internal.h

@@ -193,7 +193,7 @@ private:
 
 
 // This adds the base headers to the data and makes a packet out of it
-BufferedPacketPtr makePacket(Address &address, const SharedBuffer<u8> &data,
+BufferedPacketPtr makePacket(const Address &address, const SharedBuffer<u8> &data,
 		u32 protocol_id, session_t sender_peer_id, u8 channel);
 
 // Depending on size, make a TYPE_ORIGINAL or TYPE_SPLIT packet
@@ -235,9 +235,7 @@ private:
 class ReliablePacketBuffer
 {
 public:
-	ReliablePacketBuffer() = default;
-
-	bool getFirstSeqnum(u16& result);
+	bool getFirstSeqnum(u16 &result);
 
 	BufferedPacketPtr popFirst();
 	BufferedPacketPtr popSeqnum(u16 seqnum);
@@ -273,6 +271,7 @@ class IncomingSplitBuffer
 {
 public:
 	~IncomingSplitBuffer();
+
 	/*
 		Returns a reference counted buffer of length != 0 when a full split
 		packet is constructed. If not, returns one of length 0.
@@ -450,13 +449,15 @@ public:
 	friend class ConnectionSendThread;
 	friend class Connection;
 
-	UDPPeer(u16 id, Address address, Connection *connection);
+	UDPPeer(session_t id, const Address &address, Connection *connection);
 	virtual ~UDPPeer() = default;
 
 	void PutReliableSendCommand(ConnectionCommandPtr &c,
 							unsigned int max_packet_size) override;
 
-	bool getAddress(MTProtocols type, Address& toset) override;
+	virtual const Address &getAddress() const {
+		return address;
+	}
 
 	u16 getNextSplitSequenceNumber(u8 channel) override;
 	void setNextSplitSequenceNumber(u8 channel, u16 seqnum) override;

src/network/connectionthreads.cpp

@@ -348,11 +348,9 @@ bool ConnectionSendThread::rawSendAsPacket(session_t peer_id, u8 channelnum,
 			return false;
 
 		SharedBuffer<u8> reliable = makeReliablePacket(data, seqnum);
-		Address peer_address;
-		peer->getAddress(MTP_MINETEST_RELIABLE_UDP, peer_address);
 
 		// Add base headers and make a packet
-		BufferedPacketPtr p = con::makePacket(peer_address, reliable,
+		BufferedPacketPtr p = con::makePacket(peer->getAddress(), reliable,
 			m_connection->GetProtocolID(), m_connection->GetPeerID(),
 			channelnum);
 
@@ -374,22 +372,14 @@ bool ConnectionSendThread::rawSendAsPacket(session_t peer_id, u8 channelnum,
 		return false;
 	}
 
-	Address peer_address;
-	if (peer->getAddress(MTP_UDP, peer_address)) {
-		// Add base headers and make a packet
-		BufferedPacketPtr p = con::makePacket(peer_address, data,
-			m_connection->GetProtocolID(), m_connection->GetPeerID(),
-			channelnum);
+	// Add base headers and make a packet
+	BufferedPacketPtr p = con::makePacket(peer->getAddress(), data,
+		m_connection->GetProtocolID(), m_connection->GetPeerID(),
+		channelnum);
 
-		// Send the packet
-		rawSend(p.get());
-		return true;
-	}
-
-	LOG(dout_con << m_connection->getDesc()
-		<< " INFO: dropped unreliable packet for peer_id: " << peer_id
-		<< " because of (yet) missing udp address" << std::endl);
-	return false;
+	// Send the packet
+	rawSend(p.get());
+	return true;
 }
 
 void ConnectionSendThread::processReliableCommand(ConnectionCommandPtr &c)
@@ -984,7 +974,7 @@ void ConnectionReceiveThread::receive(SharedBuffer<u8> &packetdata,
 					l.logged = true;
 					// We simply drop the packet, the client can try again.
 				} else {
-					peer_id = m_connection->createPeer(sender, MTP_MINETEST_RELIABLE_UDP, 0);
+					peer_id = m_connection->createPeer(sender, 0);
 				}
 			}
 		}
@@ -999,17 +989,9 @@ void ConnectionReceiveThread::receive(SharedBuffer<u8> &packetdata,
 
 		// Validate peer address
 
-		Address peer_address;
-		if (peer->getAddress(MTP_UDP, peer_address)) {
-			if (peer_address != sender) {
-				LOG(derr_con << m_connection->getDesc()
-					<< " Peer " << peer_id << " sending from different address."
-					" Ignoring." << std::endl);
-				return;
-			}
-		} else {
+		if (sender != peer->getAddress()) {
 			LOG(derr_con << m_connection->getDesc()
-				<< " Peer " << peer_id << " doesn't have an address?!"
+				<< " Peer " << peer_id << " sending from different address."
 				" Ignoring." << std::endl);
 			return;
 		}
@@ -1284,32 +1266,25 @@ SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Original(Channel *cha
 SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Split(Channel *channel,
 	const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum, bool reliable)
 {
-	Address peer_address;
+	// We have to create a packet again for buffering
+	// This isn't actually too bad an idea.
+	BufferedPacketPtr packet = con::makePacket(peer->getAddress(),
+		packetdata,
+		m_connection->GetProtocolID(),
+		peer->id,
+		channelnum);
 
-	if (peer->getAddress(MTP_UDP, peer_address)) {
-		// We have to create a packet again for buffering
-		// This isn't actually too bad an idea.
-		BufferedPacketPtr packet = con::makePacket(peer_address,
-			packetdata,
-			m_connection->GetProtocolID(),
-			peer->id,
-			channelnum);
+	// Buffer the packet
+	SharedBuffer<u8> data = peer->addSplitPacket(channelnum, packet, reliable);
 
-		// Buffer the packet
-		SharedBuffer<u8> data = peer->addSplitPacket(channelnum, packet, reliable);
-
-		if (data.getSize() != 0) {
-			LOG(dout_con << m_connection->getDesc()
-				<< "RETURNING TYPE_SPLIT: Constructed full data, "
-				<< "size=" << data.getSize() << std::endl);
-			return data;
-		}
-		LOG(dout_con << m_connection->getDesc() << "BUFFERED TYPE_SPLIT" << std::endl);
-		throw ProcessedSilentlyException("Buffered a split packet chunk");
+	if (data.getSize() != 0) {
+		LOG(dout_con << m_connection->getDesc()
+			<< "RETURNING TYPE_SPLIT: Constructed full data, "
+			<< "size=" << data.getSize() << std::endl);
+		return data;
 	}
-
-	// We should never get here.
-	FATAL_ERROR("Invalid execution point");
+	LOG(dout_con << m_connection->getDesc() << "BUFFERED TYPE_SPLIT" << std::endl);
+	throw ProcessedSilentlyException("Buffered a split packet chunk");
 }
 
 SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Reliable(Channel *channel,
@@ -1361,15 +1336,11 @@ SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Reliable(Channel *cha
 	}
 
 	if (seqnum != channel->readNextIncomingSeqNum()) {
-		Address peer_address;
-
-		// this is a reliable packet so we have a udp address for sure
-		peer->getAddress(MTP_MINETEST_RELIABLE_UDP, peer_address);
 		// This one comes later, buffer it.
 		// Actually we have to make a packet to buffer one.
 		// Well, we have all the ingredients, so just do it.
 		BufferedPacketPtr packet = con::makePacket(
-			peer_address,
+			peer->getAddress(),
 			packetdata,
 			m_connection->GetProtocolID(),
 			peer->id,

src/raycast.cpp

@@ -70,8 +70,8 @@ RaycastState::RaycastState(const core::line3d<f32> &shootline,
 }
 
 
-bool boxLineCollision(const aabb3f &box, const v3f &start,
-	const v3f &dir, v3f *collision_point, v3f *collision_normal)
+bool boxLineCollision(const aabb3f &box, const v3f start,
+	const v3f dir, v3f *collision_point, v3f *collision_normal)
 {
 	if (box.isPointInside(start)) {
 		*collision_point = start;
@@ -139,8 +139,8 @@ bool boxLineCollision(const aabb3f &box, const v3f &start,
 	return false;
 }
 
-bool boxLineCollision(const aabb3f &box, const v3f &rotation,
-	const v3f &start, const v3f &dir,
+bool boxLineCollision(const aabb3f &box, const v3f rotation,
+	const v3f start, const v3f dir,
 	v3f *collision_point, v3f *collision_normal, v3f *raw_collision_normal)
 {
 	// Inversely transform the ray rather than rotating the box faces;
@@ -149,7 +149,8 @@ bool boxLineCollision(const aabb3f &box, const v3f &rotation,
 	rot.makeInverse();
 
 	bool collision = boxLineCollision(box, rot * start, rot * dir, collision_point, collision_normal);
-	if (!collision) return collision;
+	if (!collision)
+		return collision;
 
 	// Transform the results back
 	rot.makeInverse();

src/raycast.h

@@ -74,9 +74,9 @@ public:
  * outwards of the surface. If start is in the box, zero vector.
  * @returns true if a collision point was found
  */
-bool boxLineCollision(const aabb3f &box, const v3f &start, const v3f &dir,
+bool boxLineCollision(const aabb3f &box, v3f start, v3f dir,
 	v3f *collision_point, v3f *collision_normal);
 
-bool boxLineCollision(const aabb3f &box, const v3f &box_rotation,
-	const v3f &start, const v3f &dir,
+bool boxLineCollision(const aabb3f &box, v3f box_rotation,
+	v3f start, v3f dir,
 	v3f *collision_point, v3f *collision_normal, v3f *raw_collision_normal);

src/script/lua_api/l_env.cpp

@@ -1346,7 +1346,7 @@ int ModApiEnv::l_spawn_tree(lua_State *L)
 
 	ServerMap *map = &env->getServerMap();
 	treegen::error e;
-	if ((e = treegen::spawn_ltree (map, p0, ndef, tree_def)) != treegen::SUCCESS) {
+	if ((e = treegen::spawn_ltree (map, p0, tree_def)) != treegen::SUCCESS) {
 		if (e == treegen::UNBALANCED_BRACKETS) {
 			luaL_error(L, "spawn_tree(): closing ']' has no matching opening bracket");
 		} else {
@@ -1652,7 +1652,7 @@ int ModApiEnvVM::l_spawn_tree(lua_State *L)
 		return 0;
 
 	treegen::error e;
-	if ((e = treegen::make_ltree(*vm, p0, ndef, tree_def)) != treegen::SUCCESS) {
+	if ((e = treegen::make_ltree(*vm, p0, tree_def)) != treegen::SUCCESS) {
 		if (e == treegen::UNBALANCED_BRACKETS) {
 			throw LuaError("spawn_tree(): closing ']' has no matching opening bracket");
 		} else {

src/server.cpp

@@ -400,11 +400,8 @@ Server::~Server()
 
 	// Delete the rest in the reverse order of creation
 	delete m_game_settings;
-	delete m_emerge;
 	delete m_banmanager;
 	delete m_mod_storage_database;
-	delete m_startup_server_map; // if available
-	delete m_script;
 	delete m_rollback;
 	delete m_itemdef;
 	delete m_nodedef;
@@ -438,7 +435,7 @@ void Server::init()
 	}
 
 	// Create emerge manager
-	m_emerge = new EmergeManager(this, m_metrics_backend.get());
+	m_emerge = std::make_unique<EmergeManager>(this, m_metrics_backend.get());
 
 	// Create ban manager
 	std::string ban_path = m_path_world + DIR_DELIM "ipban.txt";
@@ -460,13 +457,13 @@ void Server::init()
 	MutexAutoLock envlock(m_env_mutex);
 
 	// Create the Map (loads map_meta.txt, overriding configured mapgen params)
-	ServerMap *servermap = new ServerMap(m_path_world, this, m_emerge, m_metrics_backend.get());
-	m_startup_server_map = servermap;
+	auto startup_server_map = std::make_unique<ServerMap>(m_path_world, this,
+			m_emerge.get(), m_metrics_backend.get());
 
 	// Initialize scripting
 	infostream << "Server: Initializing Lua" << std::endl;
 
-	m_script = new ServerScripting(this);
+	m_script = std::make_unique<ServerScripting>(this);
 
 	// Must be created before mod loading because we have some inventory creation
 	m_inventory_mgr = std::make_unique<ServerInventoryManager>();
@@ -474,7 +471,7 @@ void Server::init()
 	m_script->loadBuiltin();
 
 	m_gamespec.checkAndLog();
-	m_modmgr->loadMods(m_script);
+	m_modmgr->loadMods(*m_script);
 
 	m_script->saveGlobals();
 
@@ -506,19 +503,18 @@ void Server::init()
 	m_craftdef->initHashes(this);
 
 	// Initialize Environment
-	m_startup_server_map = nullptr; // Ownership moved to ServerEnvironment
-	m_env = new ServerEnvironment(servermap, m_script, this,
-		m_path_world, m_metrics_backend.get());
+	m_env = new ServerEnvironment(std::move(startup_server_map),
+		this, m_metrics_backend.get());
 	m_env->init();
 
 	m_inventory_mgr->setEnv(m_env);
 	m_clients.setEnv(m_env);
 
-	if (!servermap->settings_mgr.makeMapgenParams())
-		FATAL_ERROR("Couldn't create any mapgen type");
-
 	// Initialize mapgens
-	m_emerge->initMapgens(servermap->getMapgenParams());
+	auto &servermap = m_env->getServerMap();
+	if (!servermap.settings_mgr.makeMapgenParams())
+		FATAL_ERROR("Couldn't create any mapgen type");
+	m_emerge->initMapgens(servermap.getMapgenParams());
 
 	if (g_settings->getBool("enable_rollback_recording")) {
 		// Create rollback manager
@@ -532,7 +528,7 @@ void Server::init()
 	m_script->initAsync();
 
 	// Register us to receive map edit events
-	servermap->addEventReceiver(this);
+	servermap.addEventReceiver(this);
 
 	m_env->loadMeta();
 
@@ -1502,8 +1498,7 @@ void Server::SendInventory(RemotePlayer *player, bool incremental)
 	player->inventory.setModified(false);
 	player->setModified(true);
 
-	const std::string &s = os.str();
-	pkt.putRawString(s.c_str(), s.size());
+	pkt.putRawString(os.str());
 	Send(&pkt);
 }
 
@@ -1638,32 +1633,30 @@ void Server::SendAddParticleSpawner(session_t peer_id, u16 protocol_version,
 
 	pkt << p.amount << p.time;
 
+	std::ostringstream os(std::ios_base::binary);
 	if (protocol_version >= 42) {
 		// Serialize entire thing
-		std::ostringstream os(std::ios_base::binary);
 		p.pos.serialize(os);
 		p.vel.serialize(os);
 		p.acc.serialize(os);
 		p.exptime.serialize(os);
 		p.size.serialize(os);
-		pkt.putRawString(os.str());
 	} else {
 		// serialize legacy fields only (compatibility)
-		std::ostringstream os(std::ios_base::binary);
 		p.pos.start.legacySerialize(os);
 		p.vel.start.legacySerialize(os);
 		p.acc.start.legacySerialize(os);
 		p.exptime.start.legacySerialize(os);
 		p.size.start.legacySerialize(os);
-		pkt.putRawString(os.str());
 	}
+	pkt.putRawString(os.str());
 	pkt << p.collisiondetection;
 
 	pkt.putLongString(p.texture.string);
 
 	pkt << id << p.vertical << p.collision_removal << attached_id;
 	{
-		std::ostringstream os(std::ios_base::binary);
+		os.str("");
 		p.animation.serialize(os, protocol_version);
 		pkt.putRawString(os.str());
 	}
@@ -1671,7 +1664,7 @@ void Server::SendAddParticleSpawner(session_t peer_id, u16 protocol_version,
 	pkt << p.node.param0 << p.node.param2 << p.node_tile;
 
 	{ // serialize new fields
-		std::ostringstream os(std::ios_base::binary);
+		os.str("");
 		if (protocol_version < 42) {
 			// initial bias for older properties
 			pkt << p.pos.start.bias
@@ -2121,7 +2114,7 @@ void Server::SendActiveObjectMessages(session_t peer_id, const std::string &data
 	NetworkPacket pkt(TOCLIENT_ACTIVE_OBJECT_MESSAGES,
 			datas.size(), peer_id);
 
-	pkt.putRawString(datas.c_str(), datas.size());
+	pkt.putRawString(datas);
 
 	auto &ccf = clientCommandFactoryTable[pkt.getCommand()];
 	m_clients.sendCustom(pkt.getPeerId(), reliable ? ccf.channel : 1, &pkt, reliable);
@@ -2459,7 +2452,7 @@ void Server::SendBlocks(float dtime)
 
 			total_sending += client->getSendingCount();
 			const auto old_count = queue.size();
-			client->GetNextBlocks(m_env,m_emerge, dtime, queue);
+			client->GetNextBlocks(m_env, m_emerge.get(), dtime, queue);
 			unique_clients += queue.size() > old_count ? 1 : 0;
 		}
 	}
@@ -2693,7 +2686,7 @@ void Server::sendRequestedMedia(session_t peer_id,
 
 	// Note that applying a "real" bin packing algorithm here is not necessarily
 	// an improvement (might even perform worse) since games usually have lots
-	// of files larger than 5KB and the current algorithm already minimized
+	// of files larger than 5KB and the current algorithm already minimizes
 	// the amount of bunches quite well (at the expense of overshooting).
 
 	u32 file_size_bunch_total = 0;
@@ -3800,7 +3793,7 @@ bool Server::rollbackRevertActions(const std::list<RollbackAction> &actions,
 		std::list<std::string> *log)
 {
 	infostream<<"Server::rollbackRevertActions(len="<<actions.size()<<")"<<std::endl;
-	ServerMap *map = (ServerMap*)(&m_env->getMap());
+	auto *map = &m_env->getServerMap();
 
 	// Fail if no actions to handle
 	if (actions.empty()) {
@@ -3989,24 +3982,22 @@ void Server::requestShutdown(const std::string &msg, bool reconnect, float delay
 	} else if (delay < 0.0f && m_shutdown_state.isTimerRunning()) {
 		// Negative delay, cancel shutdown if requested
 		m_shutdown_state.reset();
-		std::wstringstream ws;
 
-		ws << L"*** Server shutdown canceled.";
+		const char *s = "*** Server shutdown canceled.";
 
-		infostream << wide_to_utf8(ws.str()).c_str() << std::endl;
-		SendChatMessage(PEER_ID_INEXISTENT, ws.str());
-		// m_shutdown_* are already handled, skip.
+		infostream << s << std::endl;
+		SendChatMessage(PEER_ID_INEXISTENT, utf8_to_wide(s));
+		// m_shutdown_state already handled, skip.
 		return;
 	} else if (delay > 0.0f) {
 	// Positive delay, tell the clients when the server will shut down
-		std::wstringstream ws;
+		std::ostringstream oss;
 
-		ws << L"*** Server shutting down in "
-				<< duration_to_string(myround(delay)).c_str()
-				<< ".";
+		oss << "*** Server shutting down in "
+			<< duration_to_string(myround(delay)) << ".";
 
-		infostream << wide_to_utf8(ws.str()).c_str() << std::endl;
-		SendChatMessage(PEER_ID_INEXISTENT, ws.str());
+		infostream << oss.str() << std::endl;
+		SendChatMessage(PEER_ID_INEXISTENT, utf8_to_wide(oss.str()));
 	}
 
 	m_shutdown_state.trigger(delay, msg, reconnect);

src/server.h

@@ -280,7 +280,7 @@ public:
 	void sendDetachedInventory(Inventory *inventory, const std::string &name, session_t peer_id);
 
 	// Envlock and conlock should be locked when using scriptapi
-	ServerScripting *getScriptIface(){ return m_script; }
+	inline ServerScripting *getScriptIface() { return m_script.get(); }
 
 	// actions: time-reversed list
 	// Return value: success/failure
@@ -294,7 +294,7 @@ public:
 	virtual ICraftDefManager* getCraftDefManager();
 	virtual u16 allocateUnknownNodeId(const std::string &name);
 	IRollbackManager *getRollbackManager() { return m_rollback; }
-	virtual EmergeManager *getEmergeManager() { return m_emerge; }
+	virtual EmergeManager *getEmergeManager() { return m_emerge.get(); }
 	virtual ModStorageDatabase *getModStorageDatabase() { return m_mod_storage_database; }
 
 	IWritableItemDefManager* getWritableItemDefManager();
@@ -432,6 +432,16 @@ public:
 	// Environment mutex (envlock)
 	std::mutex m_env_mutex;
 
+protected:
+	/* Do not add more members here, this is only required to make unit tests work. */
+
+	// Scripting
+	// Envlock and conlock should be locked when using Lua
+	std::unique_ptr<ServerScripting> m_script;
+
+	// Mods
+	std::unique_ptr<ServerModManager> m_modmgr;
+
 private:
 	friend class EmergeThread;
 	friend class RemoteClient;
@@ -478,7 +488,6 @@ private:
 	void SendBreath(session_t peer_id, u16 breath);
 	void SendAccessDenied(session_t peer_id, AccessDeniedCode reason,
 		const std::string &custom_reason, bool reconnect = false);
-	void SendAccessDenied_Legacy(session_t peer_id, const std::wstring &reason);
 	void SendDeathscreen(session_t peer_id, bool set_camera_point_target,
 		v3f camera_point_target);
 	void SendItemDef(session_t peer_id, IItemDefManager *itemdef, u16 protocol_version);
@@ -608,6 +617,8 @@ private:
 	u16 m_max_chatmessage_length;
 	// For "dedicated" server list flag
 	bool m_dedicated;
+
+	// Game settings layer
 	Settings *m_game_settings = nullptr;
 
 	// Thread can set; step() will throw as ServerError
@@ -626,10 +637,6 @@ private:
 	// Environment
 	ServerEnvironment *m_env = nullptr;
 
-	// Reference to the server map until ServerEnvironment is initialized
-	// after that this variable must be a nullptr
-	ServerMap *m_startup_server_map = nullptr;
-
 	// server connection
 	std::shared_ptr<con::Connection> m_con;
 
@@ -640,11 +647,7 @@ private:
 	IRollbackManager *m_rollback = nullptr;
 
 	// Emerge manager
-	EmergeManager *m_emerge = nullptr;
-
-	// Scripting
-	// Envlock and conlock should be locked when using Lua
-	ServerScripting *m_script = nullptr;
+	std::unique_ptr<EmergeManager> m_emerge;
 
 	// Item definition manager
 	IWritableItemDefManager *m_itemdef;
@@ -655,9 +658,6 @@ private:
 	// Craft definition manager
 	IWritableCraftDefManager *m_craftdef;
 
-	// Mods
-	std::unique_ptr<ServerModManager> m_modmgr;
-
 	std::unordered_map<std::string, Translations> server_translations;
 
 	/*

src/server/mods.cpp

@@ -50,7 +50,7 @@ ServerModManager::ServerModManager(const std::string &worldpath):
 }
 
 // This function cannot be currenctly easily tested but it should be ASAP
-void ServerModManager::loadMods(ServerScripting *script)
+void ServerModManager::loadMods(ServerScripting &script)
 {
 	// Print mods
 	infostream << "Server: Loading mods: ";
@@ -65,13 +65,13 @@ void ServerModManager::loadMods(ServerScripting *script)
 
 		std::string script_path = mod.path + DIR_DELIM + "init.lua";
 		auto t = porting::getTimeMs();
-		script->loadMod(script_path, mod.name);
+		script.loadMod(script_path, mod.name);
 		infostream << "Mod \"" << mod.name << "\" loaded after "
 			<< (porting::getTimeMs() - t) << " ms" << std::endl;
 	}
 
 	// Run a callback when mods are loaded
-	script->on_mods_loaded();
+	script.on_mods_loaded();
 }
 
 const ModSpec *ServerModManager::getModSpec(const std::string &modname) const

src/server/mods.h

@@ -22,12 +22,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "content/mod_configuration.h"
 #include <memory>
 
-class MetricsBackend;
-class MetricCounter;
 class ServerScripting;
 
 /**
- * Manage server mods
+ * Manages server mods
  *
  * All new calls to this class must be tested in test_servermodmanager.cpp
  */
@@ -36,12 +34,20 @@ class ServerModManager
 	ModConfiguration configuration;
 
 public:
+
 	/**
 	 * Creates a ServerModManager which targets worldpath
 	 * @param worldpath
 	 */
 	ServerModManager(const std::string &worldpath);
-	void loadMods(ServerScripting *script);
+
+	/**
+	 * Creates an empty ServerModManager. For testing purposes.
+	 * Note: definition looks like this so it isn't accidentally used.
+	*/
+	explicit ServerModManager(std::nullptr_t) {};
+
+	void loadMods(ServerScripting &script);
 	const ModSpec *getModSpec(const std::string &modname) const;
 	void getModNames(std::vector<std::string> &modlist) const;
 

src/serverenvironment.cpp

@@ -34,6 +34,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "scripting_server.h"
 #include "server.h"
 #include "util/serialize.h"
+#include "util/numeric.h"
 #include "util/basic_macros.h"
 #include "util/pointedthing.h"
 #include "threading/mutex_auto_lock.h"
@@ -158,8 +159,7 @@ void LBMManager::loadIntroductionTimes(const std::string &times,
 	// Storing it in a map first instead of
 	// handling the stuff directly in the loop
 	// removes all duplicate entries.
-	// TODO make this std::unordered_map
-	std::map<std::string, u32> introduction_times;
+	std::unordered_map<std::string, u32> introduction_times;
 
 	/*
 	The introduction times string consists of name~time entries,
@@ -181,13 +181,11 @@ void LBMManager::loadIntroductionTimes(const std::string &times,
 	}
 
 	// Put stuff from introduction_times into m_lbm_lookup
-	for (std::map<std::string, u32>::const_iterator it = introduction_times.begin();
-		it != introduction_times.end(); ++it) {
-		const std::string &name = it->first;
-		u32 time = it->second;
+	for (auto &it : introduction_times) {
+		const std::string &name = it.first;
+		u32 time = it.second;
 
-		std::map<std::string, LoadingBlockModifierDef *>::iterator def_it =
-			m_lbm_defs.find(name);
+		auto def_it = m_lbm_defs.find(name);
 		if (def_it == m_lbm_defs.end()) {
 			// This seems to be an LBM entry for
 			// an LBM we haven't loaded. Discard it.
@@ -298,7 +296,7 @@ void LBMManager::applyLBMs(ServerEnvironment *env, MapBlock *block,
 	ActiveBlockList
 */
 
-void fillRadiusBlock(v3s16 p0, s16 r, std::set<v3s16> &list)
+static void fillRadiusBlock(v3s16 p0, s16 r, std::set<v3s16> &list)
 {
 	v3s16 p;
 	for(p.X=p0.X-r; p.X<=p0.X+r; p.X++)
@@ -313,7 +311,7 @@ void fillRadiusBlock(v3s16 p0, s16 r, std::set<v3s16> &list)
 			}
 }
 
-void fillViewConeBlock(v3s16 p0,
+static void fillViewConeBlock(v3s16 p0,
 	const s16 r,
 	const v3f camera_pos,
 	const v3f camera_dir,
@@ -435,18 +433,12 @@ void OnMapblocksChangedReceiver::onMapEditEvent(const MapEditEvent &event)
 	ServerEnvironment
 */
 
-// Random device to seed pseudo random generators.
-static std::random_device seed;
-
-ServerEnvironment::ServerEnvironment(ServerMap *map,
-	ServerScripting *script_iface, Server *server,
-	const std::string &path_world, MetricsBackend *mb):
+ServerEnvironment::ServerEnvironment(std::unique_ptr<ServerMap> map,
+		Server *server, MetricsBackend *mb):
 	Environment(server),
-	m_map(map),
-	m_script(script_iface),
-	m_server(server),
-	m_path_world(path_world),
-	m_rgen(seed())
+	m_map(std::move(map)),
+	m_script(server->getScriptIface()),
+	m_server(server)
 {
 	m_step_time_counter = mb->addCounter(
 		"minetest_env_step_time", "Time spent in environment step (in microseconds)");
@@ -461,7 +453,8 @@ ServerEnvironment::ServerEnvironment(ServerMap *map,
 void ServerEnvironment::init()
 {
 	// Determine which database backend to use
-	std::string conf_path = m_path_world + DIR_DELIM + "world.mt";
+	const std::string world_path = m_server->getWorldPath();
+	const std::string conf_path = world_path + DIR_DELIM "world.mt";
 	Settings conf;
 
 	std::string player_backend_name = "sqlite3";
@@ -475,7 +468,7 @@ void ServerEnvironment::init()
 		u16 blocksize = 16;
 		conf.getU16NoEx("blocksize", blocksize);
 		if (blocksize != MAP_BLOCKSIZE) {
-			throw BaseException(std::string("The map's blocksize is not supported."));
+			throw BaseException("The map's blocksize is not supported.");
 		}
 
 		// Read those values before setting defaults
@@ -524,8 +517,8 @@ void ServerEnvironment::init()
 				<< "please read http://wiki.minetest.net/Database_backends." << std::endl;
 	}
 
-	m_player_database = openPlayerDatabase(player_backend_name, m_path_world, conf);
-	m_auth_database = openAuthDatabase(auth_backend_name, m_path_world, conf);
+	m_player_database = openPlayerDatabase(player_backend_name, world_path, conf);
+	m_auth_database = openAuthDatabase(auth_backend_name, world_path, conf);
 
 	if (m_map && m_script->has_on_mapblocks_changed()) {
 		m_map->addEventReceiver(&m_on_mapblocks_changed_receiver);
@@ -547,8 +540,7 @@ ServerEnvironment::~ServerEnvironment()
 	assert(m_active_blocks.size() == 0); // deactivateBlocksAndObjects does this
 
 	// Drop/delete map
-	if (m_map)
-		m_map->drop();
+	m_map.reset();
 
 	// Delete ActiveBlockModifiers
 	for (ABMWithState &m_abm : m_abms) {
@@ -702,7 +694,7 @@ void ServerEnvironment::saveMeta()
 	if (!m_meta_loaded)
 		return;
 
-	std::string path = m_path_world + DIR_DELIM "env_meta.txt";
+	std::string path = m_server->getWorldPath() + DIR_DELIM "env_meta.txt";
 
 	// Open file and serialize
 	std::ostringstream ss(std::ios_base::binary);
@@ -730,8 +722,10 @@ void ServerEnvironment::loadMeta()
 	SANITY_CHECK(!m_meta_loaded);
 	m_meta_loaded = true;
 
+	std::string path = m_server->getWorldPath() + DIR_DELIM "env_meta.txt";
+
 	// If file doesn't exist, load default environment metadata
-	if (!fs::PathExists(m_path_world + DIR_DELIM "env_meta.txt")) {
+	if (!fs::PathExists(path)) {
 		infostream << "ServerEnvironment: Loading default environment metadata"
 			<< std::endl;
 		loadDefaultMeta();
@@ -740,8 +734,6 @@ void ServerEnvironment::loadMeta()
 
 	infostream << "ServerEnvironment: Loading environment metadata" << std::endl;
 
-	std::string path = m_path_world + DIR_DELIM "env_meta.txt";
-
 	// Open file and deserialize
 	std::ifstream is(path.c_str(), std::ios_base::binary);
 	if (!is.good()) {
@@ -786,8 +778,7 @@ void ServerEnvironment::loadMeta()
 	}
 	m_lbm_mgr.loadIntroductionTimes(lbm_introduction_times, m_server, m_game_time);
 
-	m_day_count = args.exists("day_count") ?
-		args.getU64("day_count") : 0;
+	m_day_count = args.exists("day_count") ? args.getU32("day_count") : 0;
 }
 
 /**
@@ -1518,7 +1509,7 @@ void ServerEnvironment::step(float dtime)
 		TimeTaker timer("modify in active blocks per interval");
 
 		// Shuffle to prevent persistent artifacts of ordering
-		std::shuffle(m_abms.begin(), m_abms.end(), m_rgen);
+		std::shuffle(m_abms.begin(), m_abms.end(), MyRandGenerator());
 
 		// Initialize handling of ActiveBlockModifiers
 		ABMHandler abmhandler(m_abms, m_cache_abm_interval, this, true);
@@ -1532,7 +1523,7 @@ void ServerEnvironment::step(float dtime)
 		// Shuffle the active blocks so that each block gets an equal chance
 		// of having its ABMs run.
 		std::copy(m_active_blocks.m_abm_list.begin(), m_active_blocks.m_abm_list.end(), output.begin());
-		std::shuffle(output.begin(), output.end(), m_rgen);
+		std::shuffle(output.begin(), output.end(), MyRandGenerator());
 
 		int i = 0;
 		// determine the time budget for ABMs

src/serverenvironment.h

@@ -20,7 +20,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #pragma once
 
 #include <set>
-#include <random>
 #include <utility>
 
 #include "activeobject.h"
@@ -113,7 +112,7 @@ struct LBMContentMapping
 	// many times during operation in the lbm_lookup_map.
 	void deleteContents();
 	void addLBM(LoadingBlockModifierDef *lbm_def, IGameDef *gamedef);
-	const std::vector<LoadingBlockModifierDef *> *lookup(content_t c) const;
+	const lbm_map::mapped_type *lookup(content_t c) const;
 };
 
 class LBMManager
@@ -145,8 +144,7 @@ private:
 
 	// For m_query_mode == false:
 	// The key of the map is the LBM def's name.
-	// TODO make this std::unordered_map
-	std::map<std::string, LoadingBlockModifierDef *> m_lbm_defs;
+	std::unordered_map<std::string, LoadingBlockModifierDef *> m_lbm_defs;
 
 	// For m_query_mode == true:
 	// The key of the map is the LBM def's first introduction time.
@@ -221,8 +219,7 @@ enum ClearObjectsMode {
 class ServerEnvironment final : public Environment
 {
 public:
-	ServerEnvironment(ServerMap *map, ServerScripting *script_iface,
-		Server *server, const std::string &path_world, MetricsBackend *mb);
+	ServerEnvironment(std::unique_ptr<ServerMap> map, Server *server, MetricsBackend *mb);
 	~ServerEnvironment();
 
 	void init();
@@ -457,7 +454,7 @@ private:
 	*/
 
 	// The map
-	ServerMap *m_map;
+	std::unique_ptr<ServerMap> m_map;
 	// Lua state
 	ServerScripting* m_script;
 	// Server definition
@@ -466,8 +463,6 @@ private:
 	server::ActiveObjectMgr m_ao_manager;
 	// on_mapblocks_changed map event receiver
 	OnMapblocksChangedReceiver m_on_mapblocks_changed_receiver;
-	// World path
-	const std::string m_path_world;
 	// Outgoing network message buffer for active objects
 	std::queue<ActiveObjectMessage> m_active_object_messages;
 	// Some timers
@@ -504,9 +499,6 @@ private:
 	PlayerDatabase *m_player_database = nullptr;
 	AuthDatabase *m_auth_database = nullptr;
 
-	// Pseudo random generator for shuffling, etc.
-	std::mt19937 m_rgen;
-
 	// Particles
 	IntervalLimiter m_particle_management_interval;
 	std::unordered_map<u32, float> m_particle_spawners;

src/tool.cpp

@@ -402,11 +402,11 @@ DigParams getDigParams(const ItemGroupList &groups,
 			continue;
 
 		const std::string &groupname = groupcap.first;
-		float time = 0;
 		int rating = itemgroup_get(groups, groupname);
-		bool time_exists = cap.getTime(rating, &time);
-		if (!time_exists)
+		const auto time_o = cap.getTime(rating);
+		if (!time_o.has_value())
 			continue;
+		float time = *time_o;
 
 		if (leveldiff > 1)
 			time /= leveldiff;

src/tool.h

@@ -23,11 +23,12 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "itemgroup.h"
 #include "json-forwards.h"
 #include "common/c_types.h"
-#include <json/json.h>
 #include <SColor.h>
 
 #include <string>
 #include <iostream>
+#include <map>
+#include <unordered_map>
 #include <optional>
 
 struct ItemDefinition;
@@ -41,15 +42,11 @@ struct ToolGroupCap
 
 	ToolGroupCap() = default;
 
-	bool getTime(int rating, float *time) const
-	{
-		std::unordered_map<int, float>::const_iterator i = times.find(rating);
-		if (i == times.end()) {
-			*time = 0;
-			return false;
-		}
-		*time = i->second;
-		return true;
+	std::optional<float> getTime(int rating) const {
+		auto i = times.find(rating);
+		if (i == times.end())
+			return std::nullopt;
+		return i->second;
 	}
 
 	void toJson(Json::Value &object) const;
@@ -91,7 +88,7 @@ struct ToolCapabilities
 struct WearBarParams
 {
 	std::map<f32, video::SColor> colorStops;
-	enum BlendMode: u8 {
+	enum BlendMode : u8 {
 	    BLEND_MODE_CONSTANT,
 	    BLEND_MODE_LINEAR,
 	    BlendMode_END // Dummy for validity check
@@ -99,7 +96,7 @@ struct WearBarParams
 	constexpr const static EnumString es_BlendMode[3] = {
 		{WearBarParams::BLEND_MODE_CONSTANT, "constant"},
 		{WearBarParams::BLEND_MODE_LINEAR, "linear"},
-		{0, NULL}
+		{0, nullptr}
 	};
 	BlendMode blend;
 
@@ -109,7 +106,7 @@ struct WearBarParams
 	{}
 
 	WearBarParams(const video::SColor color):
-		WearBarParams({{0.0, color}}, WearBarParams::BLEND_MODE_CONSTANT)
+		WearBarParams({{0.0f, color}}, WearBarParams::BLEND_MODE_CONSTANT)
 	{};
 
 	void serialize(std::ostream &os) const;

src/unittest/mock_server.h

@@ -20,17 +20,29 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #pragma once
 
 #include "server.h"
+#include "server/mods.h"
+#include "scripting_server.h"
 
 class MockServer : public Server
 {
 public:
-	/* Set path_world to a real existing folder if you plan to initialize scripting! */
+	/* Set `path_world` to a real existing folder if you plan to initialize scripting! */
 	MockServer(const std::string &path_world = "fakepath") :
 		Server(path_world, SubgameSpec("fakespec", "fakespec"), true,
 			Address(), true, nullptr
 		)
 	{}
 
+	/*
+	 * Use this in unit tests to create scripting.
+	 * Note that you still need to call script->loadBuiltin() and don't forget
+	 * a try-catch for `ModError`.
+	 */
+	void createScripting() {
+		m_script = std::make_unique<ServerScripting>(this);
+		m_modmgr = std::make_unique<ServerModManager>(nullptr);
+	}
+
 	void start() = delete;
 	void stop() = delete;
 

src/unittest/test_moveaction.cpp

@@ -22,8 +22,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "mock_inventorymanager.h"
 #include "mock_server.h"
 #include "mock_serveractiveobject.h"
-#include "scripting_server.h"
-#include "server/mods.h"
 
 class TestMoveAction : public TestBase
 {
@@ -53,27 +51,21 @@ void TestMoveAction::runTests(IGameDef *gamedef)
 {
 	MockServer server(getTestTempDirectory());
 
-	ServerScripting server_scripting(&server);
+	server.createScripting();
 	try {
-		// FIXME: When removing the line below, the unittest does NOT crash
-		// (but it should) when running all unittests in order or registration.
-		// Some Lua API functions used in builtin require the Mgr to be present.
-		server.m_modmgr = std::make_unique<ServerModManager>(server.m_path_world);
-
 		std::string builtin = Server::getBuiltinLuaPath() + DIR_DELIM;
-		server_scripting.loadBuiltin();
-		server_scripting.loadMod(builtin + "game" DIR_DELIM "tests" DIR_DELIM "test_moveaction.lua", BUILTIN_MOD_NAME);
+		auto script = server.getScriptIface();
+		script->loadBuiltin();
+		script->loadMod(builtin + "game" DIR_DELIM "tests" DIR_DELIM "test_moveaction.lua", BUILTIN_MOD_NAME);
 	} catch (ModError &e) {
-		// Print backtrace in case of syntax errors
 		rawstream << e.what() << std::endl;
 		num_tests_failed = 1;
 		return;
 	}
 
-	server.m_script = &server_scripting;
-
 	MetricsBackend mb;
-	ServerEnvironment server_env(nullptr, &server_scripting, &server, "", &mb);
+	auto null_map = std::unique_ptr<ServerMap>();
+	ServerEnvironment server_env(std::move(null_map), &server, &mb);
 	MockServerActiveObject obj(&server_env);
 
 	TEST(testMove, &obj, gamedef);
@@ -88,8 +80,6 @@ void TestMoveAction::runTests(IGameDef *gamedef)
 
 	TEST(testCallbacks, &obj, &server);
 	TEST(testCallbacksSwap, &obj, &server);
-
-	server.m_script = nullptr; // Do not free stack memory
 }
 
 static ItemStack parse_itemstack(const char *s)

src/unittest/test_sao.cpp

@@ -20,7 +20,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "test.h"
 
 #include "mock_server.h"
-#include "scripting_server.h"
 #include "server/luaentity_sao.h"
 #include "emerge.h"
 
@@ -73,10 +72,11 @@ void TestSAO::runTests(IGameDef *gamedef)
 		ofs2 << "backend = dummy\n";
 	}
 
-	ServerScripting server_scripting(&server);
+	server.createScripting();
 	try {
-		server_scripting.loadBuiltin();
-		server_scripting.loadMod(helper_lua, BUILTIN_MOD_NAME);
+		auto script = server.getScriptIface();
+		script->loadBuiltin();
+		script->loadMod(helper_lua, BUILTIN_MOD_NAME);
 	} catch (ModError &e) {
 		rawstream << e.what() << std::endl;
 		num_tests_failed = 1;
@@ -88,8 +88,8 @@ void TestSAO::runTests(IGameDef *gamedef)
 	//       EmergeManager should become mockable
 	MetricsBackend mb;
 	EmergeManager emerge(&server, &mb);
-	auto *map = new ServerMap(server.getWorldPath(), gamedef, &emerge, &mb);
-	ServerEnvironment env(map, &server_scripting, &server, "", &mb);
+	auto map = std::make_unique<ServerMap>(server.getWorldPath(), gamedef, &emerge, &mb);
+	ServerEnvironment env(std::move(map), &server, &mb);
 	env.loadMeta();
 
 	m_step_interval = std::max(

src/util/numeric.cpp

@@ -29,7 +29,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 // myrand
 
-PcgRandom g_pcgrand;
+static PcgRandom g_pcgrand;
 
 u32 myrand()
 {
@@ -187,7 +187,7 @@ s16 adjustDist(s16 dist, float zoom_fov)
 	return std::round(adjustDist((float)dist, zoom_fov));
 }
 
-void setPitchYawRollRad(core::matrix4 &m, const v3f &rot)
+void setPitchYawRollRad(core::matrix4 &m, v3f rot)
 {
 	f64 a1 = rot.Z, a2 = rot.X, a3 = rot.Y;
 	f64 c1 = cos(a1), s1 = sin(a1);

src/util/numeric.h

@@ -250,6 +250,16 @@ int myrand_range(int min, int max);
 float myrand_range(float min, float max);
 float myrand_float();
 
+// Implements a C++11 UniformRandomBitGenerator using the above functions
+struct MyRandGenerator {
+	typedef u32 result_type;
+	static constexpr result_type min() { return 0; }
+	static constexpr result_type max() { return MYRAND_RANGE; }
+	inline result_type operator()() {
+		return myrand();
+	}
+};
+
 /*
 	Miscellaneous functions
 */
@@ -448,18 +458,18 @@ inline void wrappedApproachShortest(T &current, const T target, const T stepsize
 	}
 }
 
-void setPitchYawRollRad(core::matrix4 &m, const v3f &rot);
+void setPitchYawRollRad(core::matrix4 &m, v3f rot);
 
-inline void setPitchYawRoll(core::matrix4 &m, const v3f &rot)
+inline void setPitchYawRoll(core::matrix4 &m, v3f rot)
 {
-	setPitchYawRollRad(m, rot * core::DEGTORAD64);
+	setPitchYawRollRad(m, rot * core::DEGTORAD);
 }
 
 v3f getPitchYawRollRad(const core::matrix4 &m);
 
 inline v3f getPitchYawRoll(const core::matrix4 &m)
 {
-	return getPitchYawRollRad(m) * core::RADTODEG64;
+	return getPitchYawRollRad(m) * core::RADTODEG;
 }
 
 // Muliply the RGB value of a color linearly, and clamp to black/white

src/util/string.cpp

@@ -574,7 +574,7 @@ bool parseColorString(const std::string &value, video::SColor &color, bool quiet
 	return success;
 }
 
-std::string encodeHexColorString(const video::SColor &color)
+std::string encodeHexColorString(video::SColor color)
 {
 	std::string color_string = "#";
 	const char red = color.getRed();

src/util/string.h

@@ -97,7 +97,7 @@ char *mystrtok_r(char *s, const char *sep, char **lasts) noexcept;
 u64 read_seed(const char *str);
 bool parseColorString(const std::string &value, video::SColor &color, bool quiet,
 		unsigned char default_alpha = 0xff);
-std::string encodeHexColorString(const video::SColor &color);
+std::string encodeHexColorString(video::SColor color);
 
 
 /**