Merge branch 'master' into doc-refactor-2

doc/mkdocs/build.sh

@@ -17,6 +17,7 @@ markdown_extensions:
     - pymdownx.superfences
     - pymdownx.highlight:
         css_class: codehilite
+    - gfm_admonition
 plugins:
     - search:
         separator: '[\s\-\.\(]+'

doc/mkdocs/requirements.txt

@@ -1,3 +1,4 @@
 mkdocs~=1.4.3
 pygments~=2.15.1
 pymdown-extensions~=10.3
+markdown-gfm-admonition~=0.1.0

src/CMakeLists.txt

@@ -366,9 +366,7 @@ set(common_SRCS
 	${mapgen_SRCS}
 	${server_SRCS}
 	${content_SRCS}
-	ban.cpp
 	chat.cpp
-	clientiface.cpp
 	collision.cpp
 	content_mapnode.cpp
 	content_nodemeta.cpp
@@ -413,12 +411,10 @@ set(common_SRCS
 	raycast.cpp
 	reflowscan.cpp
 	remoteplayer.cpp
-	rollback.cpp
 	rollback_interface.cpp
 	serialization.cpp
 	server.cpp
 	serverenvironment.cpp
-	serverlist.cpp
 	settings.cpp
 	staticobject.cpp
 	terminal_chat_console.cpp

src/client/client.cpp

@@ -83,8 +83,10 @@ u32 PacketCounter::sum() const
 void PacketCounter::print(std::ostream &o) const
 {
 	for (const auto &it : m_packets) {
-		auto name = it.first >= TOCLIENT_NUM_MSG_TYPES ? "?"
+		auto name = it.first >= TOCLIENT_NUM_MSG_TYPES ? nullptr
 			: toClientCommandTable[it.first].name;
+		if (!name)
+			name = "?";
 		o << "cmd " << it.first << " (" << name << ") count "
 			<< it.second << std::endl;
 	}
@@ -991,27 +993,25 @@ inline void Client::handleCommand(NetworkPacket* pkt)
 void Client::ProcessData(NetworkPacket *pkt)
 {
 	ToClientCommand command = (ToClientCommand) pkt->getCommand();
-	u32 sender_peer_id = pkt->getPeerId();
 
-	//infostream<<"Client: received command="<<command<<std::endl;
+	m_packetcounter.add(static_cast<u16>(command));
-	m_packetcounter.add((u16)command);
 	g_profiler->graphAdd("client_received_packets", 1);
 
 	/*
 		If this check is removed, be sure to change the queue
 		system to know the ids
 	*/
-	if(sender_peer_id != PEER_ID_SERVER) {
+	if (pkt->getPeerId() != PEER_ID_SERVER) {
 		infostream << "Client::ProcessData(): Discarding data not "
-			"coming from server: peer_id=" << sender_peer_id << " command=" << pkt->getCommand()
+			"coming from server: peer_id=" << static_cast<int>(pkt->getPeerId())
-			<< std::endl;
+			<< " command=" << static_cast<unsigned>(command) << std::endl;
 		return;
 	}
 
 	// Command must be handled into ToClientCommandHandler
 	if (command >= TOCLIENT_NUM_MSG_TYPES) {
 		infostream << "Client: Ignoring unknown command "
-			<< command << std::endl;
+			<< static_cast<unsigned>(command) << std::endl;
 		return;
 	}
 
@@ -1027,24 +1027,19 @@ void Client::ProcessData(NetworkPacket *pkt)
 
 	if (m_server_ser_ver == SER_FMT_VER_INVALID) {
 		infostream << "Client: Server serialization"
-				" format invalid or not initialized."
+				" format invalid. Skipping incoming command "
-				" Skipping incoming command=" << command << std::endl;
+				<< static_cast<unsigned>(command) << std::endl;
 		return;
 	}
 
-	/*
-	  Handle runtime commands
-	*/
-
 	handleCommand(pkt);
 }
 
 void Client::Send(NetworkPacket* pkt)
 {
-	m_con->Send(PEER_ID_SERVER,
+	auto &scf = serverCommandFactoryTable[pkt->getCommand()];
-		serverCommandFactoryTable[pkt->getCommand()].channel,
+	FATAL_ERROR_IF(!scf.name, "packet type missing in table");
-		pkt,
+	m_con->Send(PEER_ID_SERVER, scf.channel, pkt, scf.reliable);
-		serverCommandFactoryTable[pkt->getCommand()].reliable);
 }
 
 // Will fill up 12 + 12 + 4 + 4 + 4 + 1 + 1 + 1 bytes

src/client/clientlauncher.cpp

@@ -27,7 +27,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "chat.h"
 #include "gettext.h"
 #include "profiler.h"
-#include "serverlist.h"
 #include "gui/guiEngine.h"
 #include "fontengine.h"
 #include "clientlauncher.h"

src/client/game.cpp

@@ -4207,7 +4207,7 @@ void Game::updateFrame(ProfilerGraph *graph, RunStats *stats, f32 dtime,
 	/*
 		==================== Drawing begins ====================
 	*/
-	if (RenderingEngine::shouldRender())
+	if (device->isWindowVisible())
 		drawScene(graph, stats);
 	/*
 		==================== End scene ====================

src/client/renderingengine.h

@@ -141,17 +141,6 @@ public:
 			const irr::core::dimension2d<u32> initial_screen_size,
 			const bool initial_window_maximized);
 
-	static bool shouldRender()
-	{
-		// On Android, pause rendering while the app is in background (generally not visible).
-		// Don't do this on desktop because windows can be partially visible.
-#ifdef __ANDROID__
-		return get_raw_device()->isWindowActive();
-#else
-		return true;
-#endif
-	};
-
 private:
 	v2u32 _getWindowSize() const;
 

src/client/tile.cpp

@@ -39,7 +39,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 /*
 	A cache from texture name to texture path
 */
-MutexedMap<std::string, std::string> g_texturename_to_path_cache;
+static MutexedMap<std::string, std::string> g_texturename_to_path_cache;
 
 /*
 	Replaces the filename extension.
@@ -186,11 +186,11 @@ struct TextureInfo
 	TextureInfo(
 			const std::string &name_,
 			video::ITexture *texture_,
-			std::set<std::string> &sourceImages_
+			std::set<std::string> &&sourceImages_
 		):
 		name(name_),
 		texture(texture_),
-		sourceImages(sourceImages_)
+		sourceImages(std::move(sourceImages_))
 	{
 	}
 };
@@ -487,8 +487,6 @@ TextureSource::~TextureSource()
 
 u32 TextureSource::getTextureId(const std::string &name)
 {
-	//infostream<<"getTextureId(): \""<<name<<"\""<<std::endl;
-
 	{
 		/*
 			See if texture already exists
@@ -553,16 +551,16 @@ static void blit_with_alpha_overlay(video::IImage *src, video::IImage *dst,
 // color alpha with the destination alpha.
 // Otherwise, any pixels that are not fully transparent get the color alpha.
 static void apply_colorize(video::IImage *dst, v2u32 dst_pos, v2u32 size,
-		const video::SColor &color, int ratio, bool keep_alpha);
+		const video::SColor color, int ratio, bool keep_alpha);
 
 // paint a texture using the given color
 static void apply_multiplication(video::IImage *dst, v2u32 dst_pos, v2u32 size,
-		const video::SColor &color);
+		const video::SColor color);
 
 // Perform a Screen blend with the given color. The opposite effect of a
 // Multiply blend.
 static void apply_screen(video::IImage *dst, v2u32 dst_pos, v2u32 size,
-		const video::SColor &color);
+		const video::SColor color);
 
 // Adjust the hue, saturation, and lightness of destination. Like
 // "Hue-Saturation" in GIMP.
@@ -608,8 +606,6 @@ void imageTransform(u32 transform, video::IImage *src, video::IImage *dst);
 */
 u32 TextureSource::generateTexture(const std::string &name)
 {
-	//infostream << "generateTexture(): name=\"" << name << "\"" << std::endl;
-
 	// Empty name means texture 0
 	if (name.empty()) {
 		infostream<<"generateTexture(): name is empty"<<std::endl;
@@ -621,8 +617,7 @@ u32 TextureSource::generateTexture(const std::string &name)
 			See if texture already exists
 		*/
 		MutexAutoLock lock(m_textureinfo_cache_mutex);
-		std::map<std::string, u32>::iterator n;
+		auto n = m_name_to_id.find(name);
-		n = m_name_to_id.find(name);
 		if (n != m_name_to_id.end()) {
 			return n->second;
 		}
@@ -661,8 +656,8 @@ u32 TextureSource::generateTexture(const std::string &name)
 	MutexAutoLock lock(m_textureinfo_cache_mutex);
 
 	u32 id = m_textureinfo_cache.size();
-	TextureInfo ti(name, tex, source_image_names);
+	TextureInfo ti(name, tex, std::move(source_image_names));
-	m_textureinfo_cache.push_back(ti);
+	m_textureinfo_cache.emplace_back(std::move(ti));
 	m_name_to_id[name] = id;
 
 	return id;
@@ -708,7 +703,7 @@ video::ITexture* TextureSource::getTextureForMesh(const std::string &name, u32 *
 	const bool filter_needed =
 		m_setting_mipmap || m_setting_trilinear_filter ||
 		m_setting_bilinear_filter || m_setting_anisotropic_filter;
-	if (filter_needed)
+	if (filter_needed && !name.empty())
 		return getTexture(name + "^[applyfiltersformesh", id);
 	return getTexture(name, id);
 }
@@ -780,19 +775,12 @@ void TextureSource::processQueue()
 		GetRequest<std::string, u32, std::thread::id, u8>
 				request = m_get_texture_queue.pop();
 
-		/*infostream<<"TextureSource::processQueue(): "
-				<<"got texture request with "
-				<<"name=\""<<request.key<<"\""
-				<<std::endl;*/
-
 		m_get_texture_queue.pushResult(request, generateTexture(request.key));
 	}
 }
 
 void TextureSource::insertSourceImage(const std::string &name, video::IImage *img)
 {
-	//infostream<<"TextureSource::insertSourceImage(): name="<<name<<std::endl;
-
 	sanity_check(std::this_thread::get_id() == m_main_thread);
 
 	m_sourcecache.insert(name, img, true);
@@ -839,8 +827,7 @@ void TextureSource::rebuildImagesAndTextures()
 
 void TextureSource::rebuildTexture(video::IVideoDriver *driver, TextureInfo &ti)
 {
-	if (ti.name.empty())
+	assert(!ti.name.empty());
-		return; // this shouldn't happen, just a precaution
 
 	// replaces the previous sourceImages
 	// shouldn't really need to be done, but can't hurt
@@ -857,7 +844,7 @@ void TextureSource::rebuildTexture(video::IVideoDriver *driver, TextureInfo &ti)
 	video::ITexture *t_old = ti.texture;
 	// Replace texture
 	ti.texture = t;
-	ti.sourceImages = source_image_names;
+	ti.sourceImages = std::move(source_image_names);
 
 	if (t_old)
 		m_texture_trash.push_back(t_old);
@@ -1075,6 +1062,12 @@ video::IImage* TextureSource::generateImage(const std::string &name, std::set<st
 	if (baseimg == NULL) {
 		errorstream << "generateImage(): baseimg is NULL (attempted to"
 				" create texture \"" << name << "\")" << std::endl;
+	} else if (baseimg->getDimension().Width == 0 ||
+			baseimg->getDimension().Height == 0) {
+		errorstream << "generateImage(): zero-sized image was created?! "
+			"(attempted to create texture \"" << name << "\")" << std::endl;
+		baseimg->drop();
+		baseimg = nullptr;
 	}
 
 	return baseimg;
@@ -1183,6 +1176,31 @@ void blitBaseImage(video::IImage* &src, video::IImage* &dst)
 	blit_with_alpha(src, dst, pos_from, pos_to, dim_dst);
 }
 
+#define CHECK_BASEIMG() \
+	do { \
+		if (!baseimg) { \
+			errorstream << "generateImagePart(): baseimg == NULL" \
+					<< " for part_of_name=\"" << part_of_name \
+					<< "\", cancelling." << std::endl; \
+			return false; \
+		} \
+	} while(0)
+
+#define COMPLAIN_INVALID(description) \
+	do { \
+		errorstream << "generateImagePart(): invalid " << (description) \
+			<< " for part_of_name=\"" << part_of_name \
+			<< "\", cancelling." << std::endl; \
+		return false; \
+	} while(0)
+
+#define CHECK_DIM(w, h) \
+	do { \
+		if ((w) <= 0 || (h) <= 0 || (w) >= 0xffff || (h) >= 0xffff) { \
+			COMPLAIN_INVALID("width or height"); \
+		} \
+	} while(0)
+
 bool TextureSource::generateImagePart(std::string part_of_name,
 		video::IImage *& baseimg, std::set<std::string> &source_image_names)
 {
@@ -1190,48 +1208,44 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 	video::IVideoDriver *driver = RenderingEngine::get_video_driver();
 	sanity_check(driver);
 
+	if (baseimg && (baseimg->getDimension().Width == 0 ||
+			baseimg->getDimension().Height == 0)) {
+		errorstream << "generateImagePart(): baseimg is zero-sized?!"
+			<< std::endl;
+		baseimg->drop();
+		baseimg = nullptr;
+	}
+
 	// Stuff starting with [ are special commands
 	if (part_of_name.empty() || part_of_name[0] != '[') {
 		source_image_names.insert(part_of_name);
 		video::IImage *image = m_sourcecache.getOrLoad(part_of_name);
-		if (image == NULL) {
+		if (!image) {
-			if (!part_of_name.empty()) {
+			// Do not create the dummy texture
+			if (part_of_name.empty())
+				return true;
 
 			// Do not create normalmap dummies
-				if (part_of_name.find("_normal.png") != std::string::npos) {
+			if (str_ends_with(part_of_name, "_normal.png")) {
-					warningstream << "generateImage(): Could not load normal map \""
+				warningstream << "generateImagePart(): Could not load normal map \""
 					<< part_of_name << "\"" << std::endl;
 				return true;
 			}
 
-				errorstream << "generateImage(): Could not load image \""
+			errorstream << "generateImagePart(): Could not load image \""
 				<< part_of_name << "\" while building texture; "
 				"Creating a dummy image" << std::endl;
-			}
 
-			// Just create a dummy image
-			//core::dimension2d<u32> dim(2,2);
 			core::dimension2d<u32> dim(1,1);
 			image = driver->createImage(video::ECF_A8R8G8B8, dim);
 			sanity_check(image != NULL);
-			/*image->setPixel(0,0, video::SColor(255,255,0,0));
-			image->setPixel(1,0, video::SColor(255,0,255,0));
-			image->setPixel(0,1, video::SColor(255,0,0,255));
-			image->setPixel(1,1, video::SColor(255,255,0,255));*/
 			image->setPixel(0,0, video::SColor(255,myrand()%256,
 					myrand()%256,myrand()%256));
-			/*image->setPixel(1,0, video::SColor(255,myrand()%256,
-					myrand()%256,myrand()%256));
-			image->setPixel(0,1, video::SColor(255,myrand()%256,
-					myrand()%256,myrand()%256));
-			image->setPixel(1,1, video::SColor(255,myrand()%256,
-					myrand()%256,myrand()%256));*/
 		}
 
 		// If base image is NULL, load as base.
 		if (baseimg == NULL)
 		{
-			//infostream<<"Setting "<<part_of_name<<" as base"<<std::endl;
 			/*
 				Copy it this way to get an alpha channel.
 				Otherwise images with alpha cannot be blitted on
@@ -1246,17 +1260,13 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		{
 			blitBaseImage(image, baseimg);
 		}
-		//cleanup
+
 		image->drop();
 	}
 	else
 	{
 		// A special texture modification
 
-		/*infostream<<"generateImage(): generating special "
-				<<"modification \""<<part_of_name<<"\""
-				<<std::endl;*/
-
 		/*
 			[crack:N:P
 			[cracko:N:P
@@ -1265,12 +1275,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		if (str_starts_with(part_of_name, "[crack"))
 		{
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream<<"generateImagePart(): baseimg == NULL "
-						<<"for part_of_name=\""<<part_of_name
-						<<"\", cancelling."<<std::endl;
-				return false;
-			}
 
 			// Crack image number and overlay option
 			// Format: crack[o][:<tiles>]:<frame_count>:<frame>
@@ -1318,6 +1323,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			sf.next(":");
 			u32 w0 = stoi(sf.next("x"));
 			u32 h0 = stoi(sf.next(":"));
+			CHECK_DIM(w0, h0);
 			core::dimension2d<u32> dim(w0,h0);
 			if (baseimg == NULL) {
 				baseimg = driver->createImage(video::ECF_A8R8G8B8, dim);
@@ -1327,9 +1333,13 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 				u32 x = stoi(sf.next(","));
 				u32 y = stoi(sf.next("="));
 				std::string filename = unescape_string(sf.next_esc(":", escape), escape);
+
+				if (x >= w0 || y >= h0)
+					COMPLAIN_INVALID("X or Y offset");
 				infostream<<"Adding \""<<filename
 						<<"\" to combined ("<<x<<","<<y<<")"
 						<<std::endl;
+
 				video::IImage *img = generateImage(filename, source_image_names);
 				if (img) {
 					core::dimension2d<u32> dim = img->getDimension();
@@ -1338,10 +1348,6 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 							driver->createImage(video::ECF_A8R8G8B8, dim);
 					img->copyTo(img2);
 					img->drop();
-					/*img2->copyToWithAlpha(baseimg, pos_base,
-							core::rect<s32>(v2s32(0,0), dim),
-							video::SColor(255,255,255,255),
-							NULL);*/
 					blit_with_alpha(img2, baseimg, v2s32(0,0), pos_base, dim);
 					img2->drop();
 				} else {
@@ -1358,8 +1364,8 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[fill"))
 		{
-			s32 x = 0;
+			u32 x = 0;
-			s32 y = 0;
+			u32 y = 0;
 
 			Strfnd sf(part_of_name);
 			sf.next(":");
@@ -1378,6 +1384,13 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			}
 			core::dimension2d<u32> dim(width, height);
 
+			CHECK_DIM(dim.Width, dim.Height);
+			if (baseimg) {
+				auto basedim = baseimg->getDimension();
+				if (x >= basedim.Width || y >= basedim.Height)
+					COMPLAIN_INVALID("X or Y offset");
+			}
+
 			video::IImage *img = driver->createImage(video::ECF_A8R8G8B8, dim);
 			img->fill(color);
 
@@ -1393,12 +1406,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[brighten"))
 		{
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream<<"generateImagePart(): baseimg==NULL "
-						<<"for part_of_name=\""<<part_of_name
-						<<"\", cancelling."<<std::endl;
-				return false;
-			}
 
 			brighten(baseimg);
 		}
@@ -1411,13 +1419,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[noalpha"))
 		{
-			if (baseimg == NULL){
+			CHECK_BASEIMG();
-				errorstream<<"generateImagePart(): baseimg==NULL "
-						<<"for part_of_name=\""<<part_of_name
-						<<"\", cancelling."<<std::endl;
-				return false;
-			}
-
 			core::dimension2d<u32> dim = baseimg->getDimension();
 
 			// Set alpha to full
@@ -1435,12 +1437,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[makealpha:"))
 		{
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream<<"generateImagePart(): baseimg == NULL "
-						<<"for part_of_name=\""<<part_of_name
-						<<"\", cancelling."<<std::endl;
-				return false;
-			}
 
 			Strfnd sf(part_of_name.substr(11));
 			u32 r1 = stoi(sf.next(","));
@@ -1449,12 +1446,6 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 
 			core::dimension2d<u32> dim = baseimg->getDimension();
 
-			/*video::IImage *oldbaseimg = baseimg;
-			baseimg = driver->createImage(video::ECF_A8R8G8B8, dim);
-			oldbaseimg->copyTo(baseimg);
-			oldbaseimg->drop();*/
-
-			// Set alpha to full
 			for (u32 y=0; y<dim.Height; y++)
 			for (u32 x=0; x<dim.Width; x++)
 			{
@@ -1490,12 +1481,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[transform"))
 		{
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream<<"generateImagePart(): baseimg == NULL "
-						<<"for part_of_name=\""<<part_of_name
-						<<"\", cancelling."<<std::endl;
-				return false;
-			}
 
 			u32 transform = parseImageTransform(part_of_name.substr(10));
 			core::dimension2d<u32> dim = imageTransformDimension(
@@ -1517,7 +1503,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[inventorycube"))
 		{
-			if (baseimg != NULL){
+			if (baseimg) {
 				errorstream<<"generateImagePart(): baseimg != NULL "
 						<<"for part_of_name=\""<<part_of_name
 						<<"\", cancelling."<<std::endl;
@@ -1540,8 +1526,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 				errorstream << "generateImagePart(): Failed to create textures"
 						<< " for inventorycube \"" << part_of_name << "\""
 						<< std::endl;
-				baseimg = generateImage(imagename_top, source_image_names);
+				return false;
-				return true;
 			}
 
 			baseimg = createInventoryCubeImage(img_top, img_left, img_right);
@@ -1561,30 +1546,26 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		{
 			Strfnd sf(part_of_name);
 			sf.next(":");
-			u32 percent = stoi(sf.next(":"));
+			u32 percent = stoi(sf.next(":"), 0, 100);
 			std::string filename = unescape_string(sf.next_esc(":", escape), escape);
 
-			if (baseimg == NULL)
-				baseimg = driver->createImage(video::ECF_A8R8G8B8, v2u32(16,16));
 			video::IImage *img = generateImage(filename, source_image_names);
-			if (img)
+			if (img) {
-			{
 				core::dimension2d<u32> dim = img->getDimension();
+				if (!baseimg)
+					baseimg = driver->createImage(video::ECF_A8R8G8B8, dim);
+
 				core::position2d<s32> pos_base(0, 0);
-				video::IImage *img2 =
-						driver->createImage(video::ECF_A8R8G8B8, dim);
-				img->copyTo(img2);
-				img->drop();
 				core::position2d<s32> clippos(0, 0);
 				clippos.Y = dim.Height * (100-percent) / 100;
 				core::dimension2d<u32> clipdim = dim;
 				clipdim.Height = clipdim.Height * percent / 100 + 1;
 				core::rect<s32> cliprect(clippos, clipdim);
-				img2->copyToWithAlpha(baseimg, pos_base,
+				img->copyToWithAlpha(baseimg, pos_base,
 						core::rect<s32>(v2s32(0,0), dim),
 						video::SColor(255,255,255,255),
 						&cliprect);
-				img2->drop();
+				img->drop();
 			}
 		}
 		/*
@@ -1594,6 +1575,8 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[verticalframe:"))
 		{
+			CHECK_BASEIMG();
+
 			Strfnd sf(part_of_name);
 			sf.next(":");
 			u32 frame_count = stoi(sf.next(":"));
@@ -1605,25 +1588,14 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 						<< "\", using frame_count = 1 instead." << std::endl;
 				frame_count = 1;
 			}
-
+			if (frame_index >= frame_count)
-			if (baseimg == NULL){
+				frame_index = frame_count - 1;
-				errorstream<<"generateImagePart(): baseimg != NULL "
-						<<"for part_of_name=\""<<part_of_name
-						<<"\", cancelling."<<std::endl;
-				return false;
-			}
 
 			v2u32 frame_size = baseimg->getDimension();
 			frame_size.Y /= frame_count;
 
 			video::IImage *img = driver->createImage(video::ECF_A8R8G8B8,
 					frame_size);
-			if (!img){
-				errorstream<<"generateImagePart(): Could not create image "
-						<<"for part_of_name=\""<<part_of_name
-						<<"\", cancelling."<<std::endl;
-				return false;
-			}
 
 			// Fill target image with transparency
 			img->fill(video::SColor(0,0,0,0));
@@ -1645,12 +1617,8 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[mask:"))
 		{
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream << "generateImage(): baseimg == NULL "
+
-						<< "for part_of_name=\"" << part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 			Strfnd sf(part_of_name);
 			sf.next(":");
 			std::string filename = unescape_string(sf.next_esc(":", escape), escape);
@@ -1661,8 +1629,8 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 						img->getDimension());
 				img->drop();
 			} else {
-				errorstream << "generateImage(): Failed to load \""
+				errorstream << "generateImagePart(): Failed to load image \""
-						<< filename << "\".";
+						<< filename << "\" for [mask" << std::endl;
 			}
 		}
 		/*
@@ -1681,12 +1649,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			sf.next(":");
 			std::string color_str = sf.next(":");
 
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg != NULL "
-						<< "for part_of_name=\"" << part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			video::SColor color;
 
@@ -1712,12 +1675,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			std::string color_str = sf.next(":");
 			std::string ratio_str = sf.next(":");
 
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg != NULL "
-						<< "for part_of_name=\"" << part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			video::SColor color;
 			int ratio = -1;
@@ -1742,12 +1700,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			/* IMPORTANT: When changing this, getTextureForMesh() needs to be
 			 * updated too. */
 
-			if (!baseimg) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg == NULL "
-						<< "for part_of_name=\"" << part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			// Apply the "clean transparent" filter, if needed
 			if (m_setting_mipmap || m_setting_bilinear_filter ||
@@ -1769,12 +1722,6 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 				 * equal to the target minimum.  If e.g. this is a vertical frames
 				 * animation, the short dimension will be the real size.
 				 */
-				if (dim.Width == 0 || dim.Height == 0) {
-					errorstream << "generateImagePart(): Illegal 0 dimension "
-						<< "for part_of_name=\""<< part_of_name
-						<< "\", cancelling." << std::endl;
-					return false;
-				}
 				u32 xscale = scaleto / dim.Width;
 				u32 yscale = scaleto / dim.Height;
 				const s32 scale = std::max(xscale, yscale);
@@ -1798,21 +1745,16 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[resize"))
 		{
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg == NULL "
-						<< "for part_of_name=\""<< part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			Strfnd sf(part_of_name);
 			sf.next(":");
 			u32 width = stoi(sf.next("x"));
 			u32 height = stoi(sf.next(""));
-			core::dimension2d<u32> dim(width, height);
+			CHECK_DIM(width, height);
 
-			video::IImage *image = RenderingEngine::get_video_driver()->
+			video::IImage *image = driver->
-				createImage(video::ECF_A8R8G8B8, dim);
+				createImage(video::ECF_A8R8G8B8, {width, height});
 			baseimg->copyToScaling(image);
 			baseimg->drop();
 			baseimg = image;
@@ -1825,12 +1767,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			255 means totally opaque.
 		*/
 		else if (str_starts_with(part_of_name, "[opacity:")) {
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg == NULL "
-						<< "for part_of_name=\"" << part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			Strfnd sf(part_of_name);
 			sf.next(":");
@@ -1855,12 +1792,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			will be inverted.
 		*/
 		else if (str_starts_with(part_of_name, "[invert:")) {
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg == NULL "
-						<< "for part_of_name=\"" << part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			Strfnd sf(part_of_name);
 			sf.next(":");
@@ -1892,13 +1824,8 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			from the base image it assumes to be a
 			tilesheet with dimensions W,H (in tiles).
 		*/
-		else if (part_of_name.substr(0,7) == "[sheet:") {
+		else if (str_starts_with(part_of_name, "[sheet:")) {
-			if (baseimg == NULL) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg != NULL "
-						<< "for part_of_name=\"" << part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			Strfnd sf(part_of_name);
 			sf.next(":");
@@ -1907,26 +1834,21 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			u32 x0 = stoi(sf.next(","));
 			u32 y0 = stoi(sf.next(":"));
 
-			if (w0 == 0 || h0 == 0) {
+			CHECK_DIM(w0, h0);
-				errorstream << "generateImagePart(): invalid width or height "
+			if (x0 >= w0 || y0 >= h0)
-						<< "for part_of_name=\"" << part_of_name
+				COMPLAIN_INVALID("tile position (X,Y)");
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			core::dimension2d<u32> img_dim = baseimg->getDimension();
 			core::dimension2d<u32> tile_dim(v2u32(img_dim) / v2u32(w0, h0));
+			if (tile_dim.Width == 0)
+				tile_dim.Width = 1;
+			if (tile_dim.Height == 0)
+				tile_dim.Height = 1;
 
 			video::IImage *img = driver->createImage(
 					video::ECF_A8R8G8B8, tile_dim);
-			if (!img) {
-				errorstream << "generateImagePart(): Could not create image "
-						<< "for part_of_name=\"" << part_of_name
-						<< "\", cancelling." << std::endl;
-				return false;
-			}
-
 			img->fill(video::SColor(0,0,0,0));
+
 			v2u32 vdim(tile_dim);
 			core::rect<s32> rect(v2s32(x0 * vdim.X, y0 * vdim.Y), tile_dim);
 			baseimg->copyToWithAlpha(img, v2s32(0), rect,
@@ -1943,15 +1865,12 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			to produce a valid string.
 		*/
 		else if (str_starts_with(part_of_name, "[png:")) {
-			Strfnd sf(part_of_name);
-			sf.next(":");
 			std::string png;
 			{
-				std::string blob = sf.next("");
+				std::string blob = part_of_name.substr(5);
 				if (!base64_is_valid(blob)) {
 					errorstream << "generateImagePart(): "
-								<< "malformed base64 in '[png'"
+								<< "malformed base64 in [png" << std::endl;
-								<< std::endl;
 					return false;
 				}
 				png = base64_decode(blob);
@@ -1996,12 +1915,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		else if (str_starts_with(part_of_name, "[hsl:") ||
 		         str_starts_with(part_of_name, "[colorizehsl:")) {
 
-			if (baseimg == nullptr) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg == NULL "
-					<< "for part_of_name=\"" << part_of_name
-					<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			bool colorize = str_starts_with(part_of_name, "[colorizehsl:");
 
@@ -2038,12 +1952,8 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		else if (str_starts_with(part_of_name, "[overlay:") ||
 		         str_starts_with(part_of_name, "[hardlight:")) {
 
-			if (baseimg == nullptr) {
+			CHECK_BASEIMG();
-				errorstream << "generateImage(): baseimg == NULL "
+
-					<< "for part_of_name=\"" << part_of_name
-					<< "\", cancelling." << std::endl;
-				return false;
-			}
 			Strfnd sf(part_of_name);
 			sf.next(":");
 			std::string filename = unescape_string(sf.next_esc(":", escape), escape);
@@ -2057,8 +1967,8 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 						img->getDimension(), hardlight);
 				img->drop();
 			} else {
-				errorstream << "generateImage(): Failed to load \""
+				errorstream << "generateImage(): Failed to load image \""
-					<< filename << "\".";
+					<< filename << "\" for [overlay or [hardlight" << std::endl;
 			}
 		}
 		/*
@@ -2072,12 +1982,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 		*/
 		else if (str_starts_with(part_of_name, "[contrast:")) {
 
-			if (baseimg == nullptr) {
+			CHECK_BASEIMG();
-				errorstream << "generateImagePart(): baseimg == NULL "
-					<< "for part_of_name=\"" << part_of_name
-					<< "\", cancelling." << std::endl;
-				return false;
-			}
 
 			Strfnd sf(part_of_name);
 			sf.next(":");
@@ -2097,6 +2002,12 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 	return true;
 }
 
+#undef CHECK_BASEIMG
+
+#undef COMPLAIN_INVALID
+
+#undef CHECK_DIM
+
 /*
 	Calculate the color of a single pixel drawn on top of another pixel.
 
@@ -2105,7 +2016,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 	pixel with alpha=64 drawn atop a pixel with alpha=128 should yield a
 	pixel with alpha=160, while getInterpolated would yield alpha=96.
 */
-static inline video::SColor blitPixel(const video::SColor &src_c, const video::SColor &dst_c, u32 ratio)
+static inline video::SColor blitPixel(const video::SColor src_c, const video::SColor dst_c, u32 ratio)
 {
 	if (dst_c.getAlpha() == 0)
 		return src_c;
@@ -2198,7 +2109,7 @@ static void blit_with_interpolate_overlay(video::IImage *src, video::IImage *dst
 	Apply color to destination, using a weighted interpolation blend
 */
 static void apply_colorize(video::IImage *dst, v2u32 dst_pos, v2u32 size,
-		const video::SColor &color, int ratio, bool keep_alpha)
+		const video::SColor color, int ratio, bool keep_alpha)
 {
 	u32 alpha = color.getAlpha();
 	video::SColor dst_c;
@@ -2236,7 +2147,7 @@ static void apply_colorize(video::IImage *dst, v2u32 dst_pos, v2u32 size,
 	Apply color to destination, using a Multiply blend mode
 */
 static void apply_multiplication(video::IImage *dst, v2u32 dst_pos, v2u32 size,
-		const video::SColor &color)
+		const video::SColor color)
 {
 	video::SColor dst_c;
 
@@ -2257,7 +2168,7 @@ static void apply_multiplication(video::IImage *dst, v2u32 dst_pos, v2u32 size,
 	Apply color to destination, using a Screen blend mode
 */
 static void apply_screen(video::IImage *dst, v2u32 dst_pos, v2u32 size,
-		const video::SColor &color)
+		const video::SColor color)
 {
 	video::SColor dst_c;
 
@@ -2700,7 +2611,7 @@ namespace {
 		return v / 12.92f;
 	}
 
-	v3f srgb_to_linear(const video::SColor &col_srgb)
+	v3f srgb_to_linear(const video::SColor col_srgb)
 	{
 		v3f col(col_srgb.getRed(), col_srgb.getGreen(), col_srgb.getBlue());
 		col /= 255.0f;
@@ -2710,7 +2621,7 @@ namespace {
 		return col;
 	}
 
-	video::SColor linear_to_srgb(const v3f &col_linear)
+	video::SColor linear_to_srgb(const v3f col_linear)
 	{
 		v3f col;
 		col.X = linear_to_srgb_component(col_linear.X);

src/gui/guiEngine.cpp

@@ -231,6 +231,7 @@ bool GUIEngine::loadMainMenuScript()
 /******************************************************************************/
 void GUIEngine::run()
 {
+	IrrlichtDevice *device = m_rendering_engine->get_raw_device();
 	// Always create clouds because they may or may not be
 	// needed based on the game selected
 	video::IVideoDriver *driver = m_rendering_engine->get_video_driver();
@@ -265,7 +266,7 @@ void GUIEngine::run()
 	f32 dtime = 0.0f;
 
 	while (m_rendering_engine->run() && (!m_startgame) && (!m_kill)) {
-		if (RenderingEngine::shouldRender()) {
+		if (device->isWindowVisible()) {
 			// check if we need to update the "upper left corner"-text
 			if (text_height != g_fontengine->getTextHeight()) {
 				updateTopLeftTextSize();
@@ -295,8 +296,6 @@ void GUIEngine::run()
 			driver->endScene();
 		}
 
-		IrrlichtDevice *device = m_rendering_engine->get_raw_device();
-
 		u32 frametime_min = 1000 / (device->isWindowFocused()
 			? g_settings->getFloat("fps_max")
 			: g_settings->getFloat("fps_max_unfocused"));

src/gui/touchscreengui.cpp

@@ -1091,6 +1091,15 @@ void TouchScreenGUI::applyContextControls(const TouchInteractionMode &mode)
 
 	u64 now = porting::getTimeMs();
 
+	// If the meanings of short and long taps have been swapped, abort any ongoing
+	// short taps because they would do something else than the player expected.
+	// Long taps don't need this, they're adjusted to the swapped meanings instead.
+	if (mode != m_last_mode) {
+		m_dig_pressed_until = 0;
+		m_place_pressed_until = 0;
+	}
+	m_last_mode = mode;
+
 	switch (m_tap_state) {
 	case TapState::ShortTap:
 		if (mode == SHORT_DIG_LONG_PLACE) {

src/gui/touchscreengui.h

@@ -314,6 +314,7 @@ private:
 
 	v2s32 getPointerPos();
 	void emitMouseEvent(EMOUSE_INPUT_EVENT type);
+	TouchInteractionMode m_last_mode = TouchInteractionMode_END;
 	TapState m_tap_state = TapState::None;
 
 	bool m_dig_pressed = false;

src/network/clientopcodes.cpp

@@ -19,8 +19,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 */
 
 #include "clientopcodes.h"
+#include "client/client.h"
 
-const static ToClientCommandHandler null_command_handler = {"TOCLIENT_NULL", TOCLIENT_STATE_ALL, &Client::handleCommand_Null};
+const static ToClientCommandHandler null_command_handler =
+	{"TOCLIENT_NULL", TOCLIENT_STATE_ALL, &Client::handleCommand_Null};
 
 const ToClientCommandHandler toClientCommandTable[TOCLIENT_NUM_MSG_TYPES] =
 {
@@ -126,7 +128,7 @@ const ToClientCommandHandler toClientCommandTable[TOCLIENT_NUM_MSG_TYPES] =
 	{ "TOCLIENT_SET_LIGHTING",             TOCLIENT_STATE_CONNECTED, &Client::handleCommand_SetLighting }, // 0x63,
 };
 
-const static ServerCommandFactory null_command_factory = { "TOSERVER_NULL", 0, false };
+const static ServerCommandFactory null_command_factory = { nullptr, 0, false };
 
 /*
 	Channels used for Client -> Server communication
@@ -223,5 +225,5 @@ const ServerCommandFactory serverCommandFactoryTable[TOSERVER_NUM_MSG_TYPES] =
 	{ "TOSERVER_FIRST_SRP",          1, true }, // 0x50
 	{ "TOSERVER_SRP_BYTES_A",        1, true }, // 0x51
 	{ "TOSERVER_SRP_BYTES_M",        1, true }, // 0x52
-	{ "TOSERVER_UPDATE_CLIENT_INFO", 1, true }, // 0x53
+	{ "TOSERVER_UPDATE_CLIENT_INFO", 2, true }, // 0x53
 };

src/network/clientopcodes.h

@@ -20,10 +20,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #pragma once
 
-#include "client/client.h"
 #include "networkprotocol.h"
 
 class NetworkPacket;
+class Client;
 
 enum ToClientConnectionState {
 	TOCLIENT_STATE_NOT_CONNECTED,

src/network/networkpacket.cpp

@@ -23,27 +23,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "util/serialize.h"
 #include "networkprotocol.h"
 
-NetworkPacket::NetworkPacket(u16 command, u32 datasize, session_t peer_id):
+void NetworkPacket::checkReadOffset(u32 from_offset, u32 field_size) const
-m_datasize(datasize), m_command(command), m_peer_id(peer_id)
-{
-	m_data.resize(m_datasize);
-}
-
-NetworkPacket::NetworkPacket(u16 command, u32 datasize):
-m_datasize(datasize), m_command(command)
-{
-	m_data.resize(m_datasize);
-}
-
-NetworkPacket::~NetworkPacket()
-{
-	m_data.clear();
-}
-
-void NetworkPacket::checkReadOffset(u32 from_offset, u32 field_size)
 {
 	if (from_offset + field_size > m_datasize) {
-		std::stringstream ss;
+		std::ostringstream ss;
 		ss << "Reading outside packet (offset: " <<
 				from_offset << ", packet size: " << getSize() << ")";
 		throw PacketError(ss.str());
@@ -56,6 +39,7 @@ void NetworkPacket::putRawPacket(const u8 *data, u32 datasize, session_t peer_id
 	// This is not permitted
 	assert(m_command == 0);
 
+	assert(datasize >= 2);
 	m_datasize = datasize - 2;
 	m_peer_id = peer_id;
 
@@ -76,7 +60,7 @@ void NetworkPacket::clear()
 	m_peer_id = 0;
 }
 
-const char* NetworkPacket::getString(u32 from_offset)
+const char* NetworkPacket::getString(u32 from_offset) const
 {
 	checkReadOffset(from_offset, 0);
 
@@ -85,10 +69,7 @@ const char* NetworkPacket::getString(u32 from_offset)
 
 void NetworkPacket::putRawString(const char* src, u32 len)
 {
-	if (m_read_offset + len > m_datasize) {
+	checkDataSize(len);
-		m_datasize = m_read_offset + len;
-		m_data.resize(m_datasize);
-	}
 
 	if (len == 0)
 		return;
@@ -279,7 +260,7 @@ NetworkPacket& NetworkPacket::operator<<(bool src)
 {
 	checkDataSize(1);
 
-	writeU8(&m_data[m_read_offset], src);
+	writeU8(&m_data[m_read_offset], src ? 1 : 0);
 
 	m_read_offset += 1;
 	return *this;
@@ -329,7 +310,7 @@ NetworkPacket& NetworkPacket::operator>>(bool& dst)
 {
 	checkReadOffset(m_read_offset, 1);
 
-	dst = readU8(&m_data[m_read_offset]);
+	dst = readU8(&m_data[m_read_offset]) != 0;
 
 	m_read_offset += 1;
 	return *this;
@@ -360,7 +341,7 @@ u8* NetworkPacket::getU8Ptr(u32 from_offset)
 
 	checkReadOffset(from_offset, 1);
 
-	return (u8*)&m_data[from_offset];
+	return &m_data[from_offset];
 }
 
 NetworkPacket& NetworkPacket::operator>>(u16& dst)

src/network/networkpacket.h

@@ -20,19 +20,25 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #pragma once
 
 #include "util/pointer.h"
-#include "util/numeric.h"
 #include "networkprotocol.h"
 #include <SColor.h>
 
 class NetworkPacket
 {
-
 public:
-	NetworkPacket(u16 command, u32 datasize, session_t peer_id);
+	NetworkPacket(u16 command, u32 preallocate, session_t peer_id) :
-	NetworkPacket(u16 command, u32 datasize);
+		m_command(command), m_peer_id(peer_id)
+	{
+		m_data.reserve(preallocate);
+	}
+	NetworkPacket(u16 command, u32 preallocate) :
+		m_command(command)
+	{
+		m_data.reserve(preallocate);
+	}
 	NetworkPacket() = default;
 
-	~NetworkPacket();
+	~NetworkPacket() = default;
 
 	void putRawPacket(const u8 *data, u32 datasize, session_t peer_id);
 	void clear();
@@ -40,13 +46,13 @@ public:
 	// Getters
 	u32 getSize() const { return m_datasize; }
 	session_t getPeerId() const { return m_peer_id; }
-	u16 getCommand() { return m_command; }
+	u16 getCommand() const { return m_command; }
 	u32 getRemainingBytes() const { return m_datasize - m_read_offset; }
 	const char *getRemainingString() { return getString(m_read_offset); }
 
 	// Returns a c-string without copying.
 	// A better name for this would be getRawString()
-	const char *getString(u32 from_offset);
+	const char *getString(u32 from_offset) const;
 	// major difference to putCString(): doesn't write len into the buffer
 	void putRawString(const char *src, u32 len);
 	void putRawString(const std::string &src)
@@ -115,11 +121,11 @@ public:
 	NetworkPacket &operator<<(video::SColor src);
 
 	// Temp, we remove SharedBuffer when migration finished
-	// ^ this comment has been here for 4 years
+	// ^ this comment has been here for 7 years
 	Buffer<u8> oldForgePacket();
 
 private:
-	void checkReadOffset(u32 from_offset, u32 field_size);
+	void checkReadOffset(u32 from_offset, u32 field_size) const;
 
 	inline void checkDataSize(u32 field_size)
 	{

src/network/networkprotocol.h

@@ -251,7 +251,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 typedef u16 session_t;
 
-enum ToClientCommand
+enum ToClientCommand : u16
 {
 	TOCLIENT_HELLO = 0x02,
 	/*
@@ -288,9 +288,7 @@ enum ToClientCommand
 		u8 (bool) reconnect
 	*/
 
-	TOCLIENT_INIT_LEGACY = 0x10, // Obsolete
+	TOCLIENT_BLOCKDATA = 0x20,
-
-	TOCLIENT_BLOCKDATA = 0x20, //TODO: Multiple blocks
 	TOCLIENT_ADDNODE = 0x21,
 	/*
 		v3s16 position
@@ -299,23 +297,15 @@ enum ToClientCommand
 	*/
 	TOCLIENT_REMOVENODE = 0x22,
 
-	TOCLIENT_PLAYERPOS = 0x23, // Obsolete
-	TOCLIENT_PLAYERINFO = 0x24, // Obsolete
-	TOCLIENT_OPT_BLOCK_NOT_FOUND = 0x25, // Obsolete
-	TOCLIENT_SECTORMETA = 0x26, // Obsolete
-
 	TOCLIENT_INVENTORY = 0x27,
 	/*
 		[0] u16 command
 		[2] serialized inventory
 	*/
 
-	TOCLIENT_OBJECTDATA = 0x28, // Obsolete
-
 	TOCLIENT_TIME_OF_DAY = 0x29,
 	/*
 		u16 time (0-23999)
-		Added in a later version:
 		f1000 time_speed
 	*/
 
@@ -337,8 +327,6 @@ enum ToClientCommand
 		bool should_be_cached
 	*/
 
-	// (oops, there is some gap here)
-
 	TOCLIENT_CHAT_MESSAGE = 0x2F,
 	/*
 		u8 version
@@ -349,8 +337,6 @@ enum ToClientCommand
 		wstring message
 	*/
 
-	TOCLIENT_CHAT_MESSAGE_OLD = 0x30, // Obsolete
-
 	TOCLIENT_ACTIVE_OBJECT_REMOVE_ADD = 0x31,
 	/*
 		u16 count of removed objects
@@ -424,26 +410,13 @@ enum ToClientCommand
 		string url
 	*/
 
-	TOCLIENT_TOOLDEF = 0x39,
-	/*
-		u32 length of the next item
-		serialized ToolDefManager
-	*/
-
 	TOCLIENT_NODEDEF = 0x3a,
 	/*
 		u32 length of the next item
 		serialized NodeDefManager
 	*/
 
-	TOCLIENT_CRAFTITEMDEF = 0x3b,
-	/*
-		u32 length of the next item
-		serialized CraftiItemDefManager
-	*/
-
 	TOCLIENT_ANNOUNCE_MEDIA = 0x3c,
-
 	/*
 		u32 number of files
 		for each texture {
@@ -647,8 +620,6 @@ enum ToClientCommand
 
 	*/
 
-	TOCLIENT_DELETE_PARTICLESPAWNER_LEGACY = 0x48, // Obsolete
-
 	TOCLIENT_HUDADD = 0x49,
 	/*
 		u32 id
@@ -882,7 +853,7 @@ enum ToClientCommand
 	TOCLIENT_NUM_MSG_TYPES = 0x64,
 };
 
-enum ToServerCommand
+enum ToServerCommand : u16
 {
 	TOSERVER_INIT = 0x02,
 	/*
@@ -895,14 +866,10 @@ enum ToServerCommand
 		std::string player name
 	*/
 
-	TOSERVER_INIT_LEGACY = 0x10, // Obsolete
-
 	TOSERVER_INIT2 = 0x11,
 	/*
-		Sent as an ACK for TOCLIENT_INIT.
+		Sent as an ACK for TOCLIENT_AUTH_ACCEPT.
 		After this, the server can send data.
-
-		[0] u16 TOSERVER_INIT2
 	*/
 
 	TOSERVER_MODCHANNEL_JOIN = 0x17,
@@ -925,10 +892,6 @@ enum ToServerCommand
 	 	std::string message
 	 */
 
-	TOSERVER_GETBLOCK = 0x20, // Obsolete
-	TOSERVER_ADDNODE = 0x21, // Obsolete
-	TOSERVER_REMOVENODE = 0x22, // Obsolete
-
 	TOSERVER_PLAYERPOS = 0x23,
 	/*
 		[0] u16 command
@@ -961,12 +924,6 @@ enum ToServerCommand
 		...
 	*/
 
-	TOSERVER_ADDNODE_FROM_INVENTORY = 0x26, // Obsolete
-	TOSERVER_CLICK_OBJECT = 0x27, // Obsolete
-	TOSERVER_GROUND_ACTION = 0x28, // Obsolete
-	TOSERVER_RELEASE = 0x29, // Obsolete
-	TOSERVER_SIGNTEXT = 0x30, // Obsolete
-
 	TOSERVER_INVENTORY_ACTION = 0x31,
 	/*
 		See InventoryAction in inventorymanager.h
@@ -978,16 +935,11 @@ enum ToServerCommand
 		wstring message
 	*/
 
-	TOSERVER_SIGNNODETEXT = 0x33, // Obsolete
-	TOSERVER_CLICK_ACTIVEOBJECT = 0x34, // Obsolete
-
 	TOSERVER_DAMAGE = 0x35,
 	/*
 		u8 amount
 	*/
 
-	TOSERVER_PASSWORD_LEGACY = 0x36, // Obsolete
-
 	TOSERVER_PLAYERITEM = 0x37,
 	/*
 		Sent to change selected item.
@@ -1063,8 +1015,6 @@ enum ToServerCommand
 			u32 token
 	*/
 
-	TOSERVER_BREATH = 0x42, // Obsolete
-
 	TOSERVER_CLIENT_READY = 0x43,
 	/*
 		u8 major

src/network/serveropcodes.cpp

@@ -19,8 +19,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 */
 
 #include "serveropcodes.h"
+#include "server.h"
 
-const static ToServerCommandHandler null_command_handler = { "TOSERVER_NULL", TOSERVER_STATE_ALL, &Server::handleCommand_Null };
+const static ToServerCommandHandler null_command_handler =
+	{ "TOSERVER_NULL", TOSERVER_STATE_ALL, &Server::handleCommand_Null };
 
 const ToServerCommandHandler toServerCommandTable[TOSERVER_NUM_MSG_TYPES] =
 {
@@ -110,7 +112,7 @@ const ToServerCommandHandler toServerCommandTable[TOSERVER_NUM_MSG_TYPES] =
 	{ "TOSERVER_UPDATE_CLIENT_INFO",       TOSERVER_STATE_INGAME, &Server::handleCommand_UpdateClientInfo }, // 0x53
 };
 
-const static ClientCommandFactory null_command_factory = { "TOCLIENT_NULL", 0, false };
+const static ClientCommandFactory null_command_factory = { nullptr, 0, false };
 
 /*
 	Channels used for Server -> Client communication
@@ -140,7 +142,7 @@ const ClientCommandFactory clientCommandFactoryTable[TOCLIENT_NUM_MSG_TYPES] =
 	null_command_factory, // 0x0D
 	null_command_factory, // 0x0E
 	null_command_factory, // 0x0F
-	{ "TOCLIENT_INIT",                     0, true }, // 0x10
+	null_command_factory, // 0x10
 	null_command_factory, // 0x11
 	null_command_factory, // 0x12
 	null_command_factory, // 0x13
@@ -217,7 +219,7 @@ const ClientCommandFactory clientCommandFactoryTable[TOCLIENT_NUM_MSG_TYPES] =
 	{ "TOCLIENT_SET_SUN",                  0, true }, // 0x5a
 	{ "TOCLIENT_SET_MOON",                 0, true }, // 0x5b
 	{ "TOCLIENT_SET_STARS",                0, true }, // 0x5c
-	null_command_factory, // 0x5d
+	{ "TOCLIENT_MOVE_PLAYER_REL",          0, true }, // 0x5d
 	null_command_factory, // 0x5e
 	null_command_factory, // 0x5f
 	{ "TOCLIENT_SRP_BYTES_S_B",            0, true }, // 0x60

src/network/serveropcodes.h

@@ -20,10 +20,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #pragma once
 
-#include "server.h"
 #include "networkprotocol.h"
 
 class NetworkPacket;
+class Server;
 
 enum ToServerConnectionState {
 	TOSERVER_STATE_NOT_CONNECTED,
@@ -33,7 +33,7 @@ enum ToServerConnectionState {
 };
 struct ToServerCommandHandler
 {
-	const std::string name;
+	const char *name;
 	ToServerConnectionState state;
 	void (Server::*handler)(NetworkPacket* pkt);
 };

src/network/serverpackethandler.cpp

@@ -95,6 +95,9 @@ void Server::handleCommand_Init(NetworkPacket* pkt)
 		return;
 	}
 
+	if (denyIfBanned(peer_id))
+		return;
+
 	// First byte after command is maximum supported
 	// serialization version
 	u8 client_max;

src/script/lua_api/l_mainmenu.cpp

@@ -32,7 +32,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "convert_json.h"
 #include "content/content.h"
 #include "content/subgames.h"
-#include "serverlist.h"
 #include "mapgen/mapgen.h"
 #include "settings.h"
 #include "client/client.h"

src/server.cpp

@@ -24,7 +24,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "network/connection.h"
 #include "network/networkprotocol.h"
 #include "network/serveropcodes.h"
-#include "ban.h"
+#include "server/ban.h"
 #include "environment.h"
 #include "map.h"
 #include "threading/mutex_auto_lock.h"
@@ -49,9 +49,9 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "content_nodemeta.h"
 #include "content/mods.h"
 #include "modchannels.h"
-#include "serverlist.h"
+#include "server/serverlist.h"
 #include "util/string.h"
-#include "rollback.h"
+#include "server/rollback.h"
 #include "util/serialize.h"
 #include "util/thread.h"
 #include "defaultsettings.h"
@@ -1062,11 +1062,13 @@ void Server::Receive(float timeout)
 			infostream << "Server::Receive(): SerializationError: what()="
 					<< e.what() << std::endl;
 		} catch (const ClientStateError &e) {
-			errorstream << "ProcessData: peer=" << peer_id << " what()="
+			errorstream << "ClientStateError: peer=" << peer_id << " what()="
 					 << e.what() << std::endl;
 			DenyAccess(peer_id, SERVER_ACCESSDENIED_UNEXPECTED_DATA);
-		} catch (const con::PeerNotFoundException &e) {
+		} catch (con::PeerNotFoundException &e) {
-			// Do nothing
+			infostream << "Server: PeerNotFoundException" << std::endl;
+		} catch (ClientNotFoundException &e) {
+			infostream << "Server: ClientNotFoundException" << std::endl;
 		}
 	}
 }
@@ -1158,38 +1160,13 @@ void Server::ProcessData(NetworkPacket *pkt)
 	ScopeProfiler sp(g_profiler, "Server: Process network packet (sum)");
 	u32 peer_id = pkt->getPeerId();
 
-	try {
-		Address address = getPeerAddress(peer_id);
-		std::string addr_s = address.serializeString();
-
-		// FIXME: Isn't it a bit excessive to check this for every packet?
-		if (m_banmanager->isIpBanned(addr_s)) {
-			std::string ban_name = m_banmanager->getBanName(addr_s);
-			infostream << "Server: A banned client tried to connect from "
-					<< addr_s << "; banned name was " << ban_name << std::endl;
-			DenyAccess(peer_id, SERVER_ACCESSDENIED_CUSTOM_STRING,
-				"Your IP is banned. Banned name was " + ban_name);
-			return;
-		}
-	} catch (con::PeerNotFoundException &e) {
-		/*
-		 * no peer for this packet found
-		 * most common reason is peer timeout, e.g. peer didn't
-		 * respond for some time, your server was overloaded or
-		 * things like that.
-		 */
-		infostream << "Server::ProcessData(): Canceling: peer "
-				<< peer_id << " not found" << std::endl;
-		return;
-	}
-
 	try {
 		ToServerCommand command = (ToServerCommand) pkt->getCommand();
 
 		// Command must be handled into ToServerCommandHandler
 		if (command >= TOSERVER_NUM_MSG_TYPES) {
 			infostream << "Server: Ignoring unknown command "
-					 << command << std::endl;
+					 << static_cast<unsigned>(command) << std::endl;
 			return;
 		}
 
@@ -1201,9 +1178,9 @@ void Server::ProcessData(NetworkPacket *pkt)
 		u8 peer_ser_ver = getClient(peer_id, CS_InitDone)->serialization_version;
 
 		if(peer_ser_ver == SER_FMT_VER_INVALID) {
-			errorstream << "Server::ProcessData(): Cancelling: Peer"
+			errorstream << "Server: Peer serialization format invalid. "
-					" serialization format invalid or not initialized."
+					"Skipping incoming command "
-					" Skipping incoming command=" << command << std::endl;
+					<< static_cast<unsigned>(command) << std::endl;
 			return;
 		}
 
@@ -1216,9 +1193,10 @@ void Server::ProcessData(NetworkPacket *pkt)
 		if (m_clients.getClientState(peer_id) < CS_Active) {
 			if (command == TOSERVER_PLAYERPOS) return;
 
-			errorstream << "Got packet command: " << command << " for peer id "
+			errorstream << "Server: Got packet command "
-					<< peer_id << " but client isn't active yet. Dropping packet "
+					<< static_cast<unsigned>(command)
-					<< std::endl;
+					<< " for peer id " << peer_id
+					<< " but client isn't active yet. Dropping packet." << std::endl;
 			return;
 		}
 
@@ -1346,15 +1324,13 @@ void Server::printToConsoleOnly(const std::string &text)
 
 void Server::Send(NetworkPacket *pkt)
 {
+	FATAL_ERROR_IF(pkt->getPeerId() == 0, "Server::Send() missing peer ID");
 	Send(pkt->getPeerId(), pkt);
 }
 
 void Server::Send(session_t peer_id, NetworkPacket *pkt)
 {
-	m_clients.send(peer_id,
+	m_clients.send(peer_id, pkt);
-		clientCommandFactoryTable[pkt->getCommand()].channel,
-		pkt,
-		clientCommandFactoryTable[pkt->getCommand()].reliable);
 }
 
 void Server::SendMovement(session_t peer_id)
@@ -2137,9 +2113,8 @@ void Server::SendActiveObjectMessages(session_t peer_id, const std::string &data
 
 	pkt.putRawString(datas.c_str(), datas.size());
 
-	m_clients.send(pkt.getPeerId(),
+	auto &ccf = clientCommandFactoryTable[pkt.getCommand()];
-			reliable ? clientCommandFactoryTable[pkt.getCommand()].channel : 1,
+	m_clients.sendCustom(pkt.getPeerId(), reliable ? ccf.channel : 1, &pkt, reliable);
-			&pkt, reliable);
 }
 
 void Server::SendCSMRestrictionFlags(session_t peer_id)
@@ -2237,12 +2212,12 @@ s32 Server::playSound(ServerPlayingSound &params, bool ephemeral)
 			<< params.spec.loop << params.spec.fade << params.spec.pitch
 			<< ephemeral << params.spec.start_time;
 
-	bool as_reliable = !ephemeral;
+	const bool as_reliable = !ephemeral;
 
 	for (const session_t peer_id : dst_clients) {
 		if (!ephemeral)
 			params.clients.insert(peer_id);
-		m_clients.send(peer_id, 0, &pkt, as_reliable);
+		m_clients.sendCustom(peer_id, 0, &pkt, as_reliable);
 	}
 
 	if (!ephemeral)
@@ -2261,8 +2236,7 @@ void Server::stopSound(s32 handle)
 	pkt << handle;
 
 	for (session_t peer_id : psound.clients) {
-		// Send as reliable
+		Send(peer_id, &pkt);
-		m_clients.send(peer_id, 0, &pkt, true);
 	}
 
 	// Remove sound reference
@@ -2282,8 +2256,7 @@ void Server::fadeSound(s32 handle, float step, float gain)
 	pkt << handle << step << gain;
 
 	for (session_t peer_id : psound.clients) {
-		// Send as reliable
+		Send(peer_id, &pkt);
-		m_clients.send(peer_id, 0, &pkt, true);
 	}
 
 	// Remove sound reference
@@ -2340,8 +2313,7 @@ void Server::sendNodeChangePkt(NetworkPacket &pkt, v3s16 block_pos,
 			continue;
 		}
 
-		// Send as reliable
+		Send(client_id, &pkt);
-		m_clients.send(client_id, 0, &pkt, true);
 	}
 }
 
@@ -3290,6 +3262,11 @@ void Server::reportFormspecPrependModified(const std::string &name)
 void Server::setIpBanned(const std::string &ip, const std::string &name)
 {
 	m_banmanager->add(ip, name);
+
+	auto clients = m_clients.getClientIDs(CS_Created);
+	for (const auto peer_id : clients) {
+		denyIfBanned(peer_id);
+	}
 }
 
 void Server::unsetIpBanned(const std::string &ip_or_name)
@@ -3302,6 +3279,22 @@ std::string Server::getBanDescription(const std::string &ip_or_name)
 	return m_banmanager->getBanDescription(ip_or_name);
 }
 
+bool Server::denyIfBanned(session_t peer_id)
+{
+	Address address = getPeerAddress(peer_id);
+	std::string addr_s = address.serializeString();
+
+	if (m_banmanager->isIpBanned(addr_s)) {
+		std::string ban_name = m_banmanager->getBanName(addr_s);
+		actionstream << "Server: A banned client tried to connect from "
+			<< addr_s << "; banned name was " << ban_name << std::endl;
+		DenyAccess(peer_id, SERVER_ACCESSDENIED_CUSTOM_STRING,
+			"Your IP is banned. Banned name was " + ban_name);
+		return true;
+	}
+	return false;
+}
+
 void Server::notifyPlayer(const char *name, const std::wstring &msg)
 {
 	// m_env will be NULL if the server is initializing
@@ -3670,8 +3663,8 @@ bool Server::dynamicAddMedia(std::string filepath,
 					to push the media over ALL channels to ensure it is processed before
 					it is used. In practice this means channels 1 and 0.
 				*/
-				m_clients.send(peer_id, 1, &legacy_pkt, true);
+				m_clients.sendCustom(peer_id, 1, &legacy_pkt, true);
-				m_clients.send(peer_id, 0, &legacy_pkt, true);
+				m_clients.sendCustom(peer_id, 0, &legacy_pkt, true);
 			} else {
 				waiting.emplace(peer_id);
 				Send(peer_id, &pkt);

src/server.h

@@ -34,7 +34,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "util/basic_macros.h"
 #include "util/metricsbackend.h"
 #include "serverenvironment.h"
-#include "clientiface.h"
+#include "server/clientiface.h"
 #include "chatmessage.h"
 #include "sound.h"
 #include "translation.h"
@@ -245,6 +245,7 @@ public:
 	void setIpBanned(const std::string &ip, const std::string &name);
 	void unsetIpBanned(const std::string &ip_or_name);
 	std::string getBanDescription(const std::string &ip_or_name);
+	bool denyIfBanned(session_t peer_id);
 
 	void notifyPlayer(const char *name, const std::wstring &msg);
 	void notifyPlayers(const std::wstring &msg);

src/server/CMakeLists.txt

@@ -1,9 +1,13 @@
 set(server_SRCS
 	${CMAKE_CURRENT_SOURCE_DIR}/activeobjectmgr.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/ban.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/clientiface.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/luaentity_sao.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/mods.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/player_sao.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/serveractiveobject.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/serverinventorymgr.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/serverlist.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/unit_sao.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/rollback.cpp
 	PARENT_SCOPE)

src/server/clientiface.cpp

@@ -345,11 +345,13 @@ void RemoteClient::GetNextBlocks (
 			if (m_blocks_sent.find(p) != m_blocks_sent.end())
 				continue;
 
-			bool block_not_found = false;
 			if (block) {
-				// Check whether the block exists (with data)
+				/*
-				if (!block->isGenerated())
+					If block is not generated and generating new ones is
-					block_not_found = true;
+					not wanted, skip block.
+				*/
+				if (!block->isGenerated() && !generate)
+					continue;
 
 				/*
 					If block is not close, don't send it unless it is near
@@ -379,19 +381,10 @@ void RemoteClient::GetNextBlocks (
 				continue;
 			}
 
-			/*
-				If block has been marked to not exist on disk (dummy) or is
-				not generated and generating new ones is not wanted, skip block.
-			*/
-			if (!generate && block_not_found) {
-				// get next one.
-				continue;
-			}
-
 			/*
 				Add inexistent block to emerge queue.
 			*/
-			if (block == NULL || block_not_found) {
+			if (!block || !block->isGenerated()) {
 				if (emerge->enqueueBlockEmerge(peer_id, p, generate)) {
 					if (nearest_emerged_d == -1)
 						nearest_emerged_d = d;
@@ -791,10 +784,21 @@ void ClientInterface::UpdatePlayerList()
 	}
 }
 
-void ClientInterface::send(session_t peer_id, u8 channelnum,
+void ClientInterface::send(session_t peer_id, NetworkPacket *pkt)
-		NetworkPacket *pkt, bool reliable)
 {
-	m_con->Send(peer_id, channelnum, pkt, reliable);
+	auto &ccf = clientCommandFactoryTable[pkt->getCommand()];
+	FATAL_ERROR_IF(!ccf.name, "packet type missing in table");
+
+	m_con->Send(peer_id, ccf.channel, pkt, ccf.reliable);
+}
+
+void ClientInterface::sendCustom(session_t peer_id, u8 channel, NetworkPacket *pkt, bool reliable)
+{
+	// check table anyway to prevent mistakes
+	FATAL_ERROR_IF(!clientCommandFactoryTable[pkt->getCommand()].name,
+		"packet type missing in table");
+
+	m_con->Send(peer_id, channel, pkt, reliable);
 }
 
 void ClientInterface::sendToAll(NetworkPacket *pkt)
@@ -804,9 +808,9 @@ void ClientInterface::sendToAll(NetworkPacket *pkt)
 		RemoteClient *client = client_it.second;
 
 		if (client->net_proto_version != 0) {
-			m_con->Send(client->peer_id,
+			auto &ccf = clientCommandFactoryTable[pkt->getCommand()];
-					clientCommandFactoryTable[pkt->getCommand()].channel, pkt,
+			FATAL_ERROR_IF(!ccf.name, "packet type missing in table");
-					clientCommandFactoryTable[pkt->getCommand()].reliable);
+			m_con->Send(client->peer_id, ccf.channel, pkt, ccf.reliable);
 		}
 	}
 }

src/server/clientiface.h

@@ -482,8 +482,11 @@ public:
 	/* get list of client player names */
 	const std::vector<std::string> &getPlayerNames() const { return m_clients_names; }
 
-	/* send message to client */
+	/* send to one client */
-	void send(session_t peer_id, u8 channelnum, NetworkPacket *pkt, bool reliable);
+	void send(session_t peer_id, NetworkPacket *pkt);
+
+	/* send to one client, deviating from the standard params */
+	void sendCustom(session_t peer_id, u8 channel, NetworkPacket *pkt, bool reliable);
 
 	/* send to all clients */
 	void sendToAll(NetworkPacket *pkt);

src/server/serverlist.h

@@ -24,6 +24,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #pragma once
 
+// Note that client serverlist handling is all in Lua, this is only announcements now.
+
 namespace ServerList
 {
 #if USE_CURL
@@ -36,4 +38,4 @@ void sendAnnounce(AnnounceAction, u16 port,
 		bool dedicated = false);
 #endif
 
-} // namespace ServerList
+}

src/unittest/test_ban.cpp

@@ -19,7 +19,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "test.h"
 
-#include "ban.h"
+#include "server/ban.h"
 
 class TestBan : public TestBase
 {