From 2ef080a51b5982e4e2ce17953f2a9cc62edf13fa Mon Sep 17 00:00:00 2001 From: lhofhansl Date: Thu, 25 Jan 2024 11:32:18 -0800 Subject: [PATCH 01/11] Slight simplification of RemoteClient::getNextBlocks(...) (#14302) --- src/clientiface.cpp | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/clientiface.cpp b/src/clientiface.cpp index f76ef0a882..e6849a5b49 100644 --- a/src/clientiface.cpp +++ b/src/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()) - block_not_found = true; + /* + If block is not generated and generating new ones is + 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; From a46fe799393e9866521c7a0b2067e8a4a7a3a526 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 26 Jan 2024 14:37:09 +0100 Subject: [PATCH 02/11] Reduce code duplication in tile.cpp --- src/client/tile.cpp | 259 ++++++++++++-------------------------------- 1 file changed, 68 insertions(+), 191 deletions(-) diff --git a/src/client/tile.cpp b/src/client/tile.cpp index 0c6b96933c..d750f3a929 100644 --- a/src/client/tile.cpp +++ b/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 g_texturename_to_path_cache; +static MutexedMap 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 &sourceImages_ + std::set &&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(): \""<::iterator n; - n = m_name_to_id.find(name); + auto 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); - m_textureinfo_cache.push_back(ti); + TextureInfo ti(name, tex, std::move(source_image_names)); + m_textureinfo_cache.emplace_back(std::move(ti)); m_name_to_id[name] = id; return id; @@ -780,19 +775,12 @@ void TextureSource::processQueue() GetRequest request = m_get_texture_queue.pop(); - /*infostream<<"TextureSource::processQueue(): " - <<"got texture request with " - <<"name=\""<= 0xffff || (h) >= 0xffff) { \ + errorstream << "generateImagePart(): invalid width or height " \ + << "for part_of_name=\"" << part_of_name \ + << "\", cancelling." << std::endl; \ + return false; \ + } \ + } while(0) + bool TextureSource::generateImagePart(std::string part_of_name, video::IImage *& baseimg, std::set &source_image_names) { @@ -1210,28 +1217,16 @@ bool TextureSource::generateImagePart(std::string part_of_name, } // Just create a dummy image - //core::dimension2d dim(2,2); core::dimension2d 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 "<drop(); } else { // A special texture modification - /*infostream<<"generateImage(): generating special " - <<"modification \""<]:: @@ -1318,6 +1304,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 dim(w0,h0); if (baseimg == NULL) { baseimg = driver->createImage(video::ECF_A8R8G8B8, dim); @@ -1338,10 +1325,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(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 { @@ -1378,6 +1361,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, } core::dimension2d dim(width, height); + CHECK_DIM(dim.Width, dim.Height); video::IImage *img = driver->createImage(video::ECF_A8R8G8B8, dim); img->fill(color); @@ -1393,12 +1377,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, */ else if (str_starts_with(part_of_name, "[brighten")) { - if (baseimg == NULL) { - errorstream<<"generateImagePart(): baseimg==NULL " - <<"for part_of_name=\""< dim = baseimg->getDimension(); // Set alpha to full @@ -1435,12 +1408,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, */ else if (str_starts_with(part_of_name, "[makealpha:")) { - if (baseimg == NULL) { - errorstream<<"generateImagePart(): baseimg == NULL " - <<"for part_of_name=\""< 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 = imageTransformDimension( @@ -1517,7 +1474,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=\""<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=\""<fill(video::SColor(0,0,0,0)); @@ -1645,12 +1591,8 @@ bool TextureSource::generateImagePart(std::string part_of_name, */ else if (str_starts_with(part_of_name, "[mask:")) { - if (baseimg == NULL) { - errorstream << "generateImage(): baseimg == NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); + Strfnd sf(part_of_name); sf.next(":"); std::string filename = unescape_string(sf.next_esc(":", escape), escape); @@ -1681,12 +1623,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, sf.next(":"); std::string color_str = sf.next(":"); - if (baseimg == NULL) { - errorstream << "generateImagePart(): baseimg != NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); video::SColor color; @@ -1712,12 +1649,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, std::string color_str = sf.next(":"); std::string ratio_str = sf.next(":"); - if (baseimg == NULL) { - errorstream << "generateImagePart(): baseimg != NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); video::SColor color; int ratio = -1; @@ -1742,12 +1674,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, /* IMPORTANT: When changing this, getTextureForMesh() needs to be * updated too. */ - if (!baseimg) { - errorstream << "generateImagePart(): baseimg == NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); // Apply the "clean transparent" filter, if needed if (m_setting_mipmap || m_setting_bilinear_filter || @@ -1769,12 +1696,7 @@ 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; - } + CHECK_DIM(dim.Width, dim.Height); u32 xscale = scaleto / dim.Width; u32 yscale = scaleto / dim.Height; const s32 scale = std::max(xscale, yscale); @@ -1798,21 +1720,16 @@ bool TextureSource::generateImagePart(std::string part_of_name, */ else if (str_starts_with(part_of_name, "[resize")) { - if (baseimg == NULL) { - errorstream << "generateImagePart(): baseimg == NULL " - << "for part_of_name=\""<< part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); Strfnd sf(part_of_name); sf.next(":"); u32 width = stoi(sf.next("x")); u32 height = stoi(sf.next("")); - core::dimension2d dim(width, height); + CHECK_DIM(width, height); video::IImage *image = RenderingEngine::get_video_driver()-> - createImage(video::ECF_A8R8G8B8, dim); + createImage(video::ECF_A8R8G8B8, {width, height}); baseimg->copyToScaling(image); baseimg->drop(); baseimg = image; @@ -1825,12 +1742,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) { - errorstream << "generateImagePart(): baseimg == NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); Strfnd sf(part_of_name); sf.next(":"); @@ -1855,12 +1767,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, will be inverted. */ else if (str_starts_with(part_of_name, "[invert:")) { - if (baseimg == NULL) { - errorstream << "generateImagePart(): baseimg == NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); Strfnd sf(part_of_name); sf.next(":"); @@ -1892,13 +1799,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:") { - if (baseimg == NULL) { - errorstream << "generateImagePart(): baseimg != NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + else if (str_starts_with(part_of_name, "[sheet:")) { + CHECK_BASEIMG(); Strfnd sf(part_of_name); sf.next(":"); @@ -1907,24 +1809,13 @@ bool TextureSource::generateImagePart(std::string part_of_name, u32 x0 = stoi(sf.next(",")); u32 y0 = stoi(sf.next(":")); - if (w0 == 0 || h0 == 0) { - errorstream << "generateImagePart(): invalid width or height " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_DIM(w0, h0); core::dimension2d img_dim = baseimg->getDimension(); core::dimension2d tile_dim(v2u32(img_dim) / v2u32(w0, h0)); 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); @@ -1996,12 +1887,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) { - errorstream << "generateImagePart(): baseimg == NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); bool colorize = str_starts_with(part_of_name, "[colorizehsl:"); @@ -2038,12 +1924,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) { - errorstream << "generateImage(): baseimg == NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); + Strfnd sf(part_of_name); sf.next(":"); std::string filename = unescape_string(sf.next_esc(":", escape), escape); @@ -2072,12 +1954,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, */ else if (str_starts_with(part_of_name, "[contrast:")) { - if (baseimg == nullptr) { - errorstream << "generateImagePart(): baseimg == NULL " - << "for part_of_name=\"" << part_of_name - << "\", cancelling." << std::endl; - return false; - } + CHECK_BASEIMG(); Strfnd sf(part_of_name); sf.next(":"); @@ -2105,7 +1982,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 +2075,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 +2113,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 +2134,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; @@ -2479,7 +2356,7 @@ video::IImage *create_crack_image(video::IImage *crack, s32 frame_index, core::rect frame(v2s32(0, frame_index * frame_size.Height), frame_size); video::IImage *result = nullptr; -// extract crack frame + // extract crack frame video::IImage *crack_tile = driver->createImage(video::ECF_A8R8G8B8, tile_size); if (!crack_tile) return nullptr; @@ -2496,7 +2373,7 @@ video::IImage *create_crack_image(video::IImage *crack, s32 frame_index, if (tiles == 1) return crack_tile; -// tile it + // tile it result = driver->createImage(video::ECF_A8R8G8B8, size); if (!result) goto exit__has_tile; @@ -2700,7 +2577,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 +2587,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); From 8927e7caf6f9f0c3641b96d5f4692313a45e4097 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 26 Jan 2024 15:22:27 +0100 Subject: [PATCH 03/11] Handle some edge cases in tile images --- src/client/tile.cpp | 128 ++++++++++++++++++++++++++++---------------- 1 file changed, 81 insertions(+), 47 deletions(-) diff --git a/src/client/tile.cpp b/src/client/tile.cpp index d750f3a929..cc6b3273c7 100644 --- a/src/client/tile.cpp +++ b/src/client/tile.cpp @@ -703,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); } @@ -1062,6 +1062,12 @@ video::IImage* TextureSource::generateImage(const std::string &name, std::setgetDimension().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; @@ -1173,20 +1179,25 @@ void blitBaseImage(video::IImage* &src, video::IImage* &dst) #define CHECK_BASEIMG() \ do { \ if (!baseimg) { \ - errorstream << "generateImagePart(): baseimg == NULL " \ - << "for part_of_name=\"" << part_of_name \ + 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) { \ - errorstream << "generateImagePart(): invalid width or height " \ - << "for part_of_name=\"" << part_of_name \ - << "\", cancelling." << std::endl; \ - return false; \ + COMPLAIN_INVALID("width or height"); \ } \ } while(0) @@ -1197,26 +1208,34 @@ 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 (!part_of_name.empty()) { + if (!image) { + // 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) { - warningstream << "generateImage(): Could not load normal map \"" - << part_of_name << "\"" << std::endl; - return true; - } - - errorstream << "generateImage(): Could not load image \"" - << part_of_name << "\" while building texture; " - "Creating a dummy image" << std::endl; + // Do not create normalmap dummies + if (str_ends_with(part_of_name, "_normal.png")) { + warningstream << "generateImagePart(): Could not load normal map \"" + << part_of_name << "\"" << std::endl; + return true; } - // Just create a dummy image + errorstream << "generateImagePart(): Could not load image \"" + << part_of_name << "\" while building texture; " + "Creating a dummy image" << std::endl; + core::dimension2d dim(1,1); image = driver->createImage(video::ECF_A8R8G8B8, dim); sanity_check(image != NULL); @@ -1314,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 \""< dim = img->getDimension(); @@ -1341,8 +1364,8 @@ bool TextureSource::generateImagePart(std::string part_of_name, */ else if (str_starts_with(part_of_name, "[fill")) { - s32 x = 0; - s32 y = 0; + u32 x = 0; + u32 y = 0; Strfnd sf(part_of_name); sf.next(":"); @@ -1362,6 +1385,12 @@ bool TextureSource::generateImagePart(std::string part_of_name, core::dimension2d 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); @@ -1497,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 true; + return false; } baseimg = createInventoryCubeImage(img_top, img_left, img_right); @@ -1518,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 dim = img->getDimension(); + if (!baseimg) + baseimg = driver->createImage(video::ECF_A8R8G8B8, dim); + core::position2d pos_base(0, 0); - video::IImage *img2 = - driver->createImage(video::ECF_A8R8G8B8, dim); - img->copyTo(img2); - img->drop(); core::position2d clippos(0, 0); clippos.Y = dim.Height * (100-percent) / 100; core::dimension2d clipdim = dim; clipdim.Height = clipdim.Height * percent / 100 + 1; core::rect cliprect(clippos, clipdim); - img2->copyToWithAlpha(baseimg, pos_base, + img->copyToWithAlpha(baseimg, pos_base, core::rect(v2s32(0,0), dim), video::SColor(255,255,255,255), &cliprect); - img2->drop(); + img->drop(); } } /* @@ -1564,6 +1588,8 @@ bool TextureSource::generateImagePart(std::string part_of_name, << "\", using frame_count = 1 instead." << std::endl; frame_count = 1; } + if (frame_index >= frame_count) + frame_index = frame_count - 1; v2u32 frame_size = baseimg->getDimension(); frame_size.Y /= frame_count; @@ -1603,8 +1629,8 @@ bool TextureSource::generateImagePart(std::string part_of_name, img->getDimension()); img->drop(); } else { - errorstream << "generateImage(): Failed to load \"" - << filename << "\"."; + errorstream << "generateImagePart(): Failed to load image \"" + << filename << "\" for [mask" << std::endl; } } /* @@ -1696,7 +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. */ - CHECK_DIM(dim.Width, dim.Height); u32 xscale = scaleto / dim.Width; u32 yscale = scaleto / dim.Height; const s32 scale = std::max(xscale, yscale); @@ -1728,7 +1753,7 @@ bool TextureSource::generateImagePart(std::string part_of_name, u32 height = stoi(sf.next("")); CHECK_DIM(width, height); - video::IImage *image = RenderingEngine::get_video_driver()-> + video::IImage *image = driver-> createImage(video::ECF_A8R8G8B8, {width, height}); baseimg->copyToScaling(image); baseimg->drop(); @@ -1810,14 +1835,20 @@ bool TextureSource::generateImagePart(std::string part_of_name, u32 y0 = stoi(sf.next(":")); CHECK_DIM(w0, h0); + if (x0 >= w0 || y0 >= h0) + COMPLAIN_INVALID("tile position (X,Y)"); core::dimension2d img_dim = baseimg->getDimension(); core::dimension2d 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); - img->fill(video::SColor(0,0,0,0)); + v2u32 vdim(tile_dim); core::rect rect(v2s32(x0 * vdim.X, y0 * vdim.Y), tile_dim); baseimg->copyToWithAlpha(img, v2s32(0), rect, @@ -1834,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'" - << std::endl; + << "malformed base64 in [png" << std::endl; return false; } png = base64_decode(blob); @@ -1939,8 +1967,8 @@ bool TextureSource::generateImagePart(std::string part_of_name, img->getDimension(), hardlight); img->drop(); } else { - errorstream << "generateImage(): Failed to load \"" - << filename << "\"."; + errorstream << "generateImage(): Failed to load image \"" + << filename << "\" for [overlay or [hardlight" << std::endl; } } /* @@ -1974,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. From 415875926543131a008823686b52c43dda54ecd1 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 26 Jan 2024 16:10:52 +0100 Subject: [PATCH 04/11] Move mtevent.h to src/client/ --- src/{ => client}/mtevent.h | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/{ => client}/mtevent.h (100%) diff --git a/src/mtevent.h b/src/client/mtevent.h similarity index 100% rename from src/mtevent.h rename to src/client/mtevent.h From df9975f35dfc98964a83c2d8369ac6eea59da352 Mon Sep 17 00:00:00 2001 From: Bradley Pierce <90872694+Bituvo@users.noreply.github.com> Date: Fri, 26 Jan 2024 16:10:57 -0500 Subject: [PATCH 05/11] Add markdown admonition extension (#14303) --- doc/mkdocs/build.sh | 1 + doc/mkdocs/requirements.txt | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/mkdocs/build.sh b/doc/mkdocs/build.sh index 7f837866ff..8d8ecb7ca1 100755 --- a/doc/mkdocs/build.sh +++ b/doc/mkdocs/build.sh @@ -17,6 +17,7 @@ markdown_extensions: - pymdownx.superfences - pymdownx.highlight: css_class: codehilite + - gfm_admonition plugins: - search: separator: '[\s\-\.\(]+' diff --git a/doc/mkdocs/requirements.txt b/doc/mkdocs/requirements.txt index e67f3d07fd..61e9c03c33 100644 --- a/doc/mkdocs/requirements.txt +++ b/doc/mkdocs/requirements.txt @@ -1,3 +1,4 @@ mkdocs~=1.4.3 pygments~=2.15.1 -pymdown-extensions~=10.3 \ No newline at end of file +pymdown-extensions~=10.3 +markdown-gfm-admonition~=0.1.0 \ No newline at end of file From 2b99dabdac823fee3ea843a9ded8bbe7718c3b44 Mon Sep 17 00:00:00 2001 From: grorp Date: Fri, 26 Jan 2024 23:19:06 +0100 Subject: [PATCH 06/11] Touchscreen: Abort ongoing short taps if touch interaction mode changes (#14305) --- src/gui/touchscreengui.cpp | 9 +++++++++ src/gui/touchscreengui.h | 1 + 2 files changed, 10 insertions(+) diff --git a/src/gui/touchscreengui.cpp b/src/gui/touchscreengui.cpp index c9fe5569e6..d8ac39c31b 100644 --- a/src/gui/touchscreengui.cpp +++ b/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) { diff --git a/src/gui/touchscreengui.h b/src/gui/touchscreengui.h index 2e48a71cf3..661f70f10a 100644 --- a/src/gui/touchscreengui.h +++ b/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; From 397682a5b08ae683431618ba52b6dec1829bcce3 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 24 Jan 2024 18:23:11 +0100 Subject: [PATCH 07/11] Clean up client and server command sending / tables --- src/client/client.cpp | 35 +++++++++------------ src/clientiface.cpp | 23 ++++++++++---- src/clientiface.h | 7 +++-- src/network/clientopcodes.cpp | 8 +++-- src/network/clientopcodes.h | 2 +- src/network/networkprotocol.h | 58 +++-------------------------------- src/network/serveropcodes.cpp | 10 +++--- src/network/serveropcodes.h | 4 +-- src/server.cpp | 51 +++++++++++++++--------------- 9 files changed, 79 insertions(+), 119 deletions(-) diff --git a/src/client/client.cpp b/src/client/client.cpp index ef3308d030..5d9c07ce54 100644 --- a/src/client/client.cpp +++ b/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)); 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() - << std::endl; + "coming from server: peer_id=" << static_cast(pkt->getPeerId()) + << " command=" << static_cast(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(command) << std::endl; return; } @@ -1020,31 +1020,26 @@ void Client::ProcessData(NetworkPacket *pkt) * But we must use the new ToClientConnectionState in the future, * as a byte mask */ - if(toClientCommandTable[command].state == TOCLIENT_STATE_NOT_CONNECTED) { + if (toClientCommandTable[command].state == TOCLIENT_STATE_NOT_CONNECTED) { handleCommand(pkt); return; } - if(m_server_ser_ver == SER_FMT_VER_INVALID) { + if (m_server_ser_ver == SER_FMT_VER_INVALID) { infostream << "Client: Server serialization" - " format invalid or not initialized." - " Skipping incoming command=" << command << std::endl; + " format invalid. Skipping incoming command " + << static_cast(command) << std::endl; return; } - /* - Handle runtime commands - */ - handleCommand(pkt); } void Client::Send(NetworkPacket* pkt) { - m_con->Send(PEER_ID_SERVER, - serverCommandFactoryTable[pkt->getCommand()].channel, - pkt, - serverCommandFactoryTable[pkt->getCommand()].reliable); + auto &scf = serverCommandFactoryTable[pkt->getCommand()]; + FATAL_ERROR_IF(!scf.name, "packet type missing in table"); + m_con->Send(PEER_ID_SERVER, scf.channel, pkt, scf.reliable); } // Will fill up 12 + 12 + 4 + 4 + 4 + 1 + 1 + 1 bytes diff --git a/src/clientiface.cpp b/src/clientiface.cpp index e6849a5b49..22e3e42d03 100644 --- a/src/clientiface.cpp +++ b/src/clientiface.cpp @@ -784,10 +784,21 @@ void ClientInterface::UpdatePlayerList() } } -void ClientInterface::send(session_t peer_id, u8 channelnum, - NetworkPacket *pkt, bool reliable) +void ClientInterface::send(session_t peer_id, NetworkPacket *pkt) { - 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) @@ -797,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, - clientCommandFactoryTable[pkt->getCommand()].channel, pkt, - clientCommandFactoryTable[pkt->getCommand()].reliable); + auto &ccf = clientCommandFactoryTable[pkt->getCommand()]; + FATAL_ERROR_IF(!ccf.name, "packet type missing in table"); + m_con->Send(client->peer_id, ccf.channel, pkt, ccf.reliable); } } } diff --git a/src/clientiface.h b/src/clientiface.h index c5a93576cc..ac41b00cae 100644 --- a/src/clientiface.h +++ b/src/clientiface.h @@ -482,8 +482,11 @@ public: /* get list of client player names */ const std::vector &getPlayerNames() const { return m_clients_names; } - /* send message to client */ - void send(session_t peer_id, u8 channelnum, NetworkPacket *pkt, bool reliable); + /* send to one client */ + 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); diff --git a/src/network/clientopcodes.cpp b/src/network/clientopcodes.cpp index c3dbe57a99..d426d3fe78 100644 --- a/src/network/clientopcodes.cpp +++ b/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 }; diff --git a/src/network/clientopcodes.h b/src/network/clientopcodes.h index 5c5a31315b..683d3534df 100644 --- a/src/network/clientopcodes.h +++ b/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, diff --git a/src/network/networkprotocol.h b/src/network/networkprotocol.h index 44d5e80fcc..76d1a11708 100644 --- a/src/network/networkprotocol.h +++ b/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, //TODO: Multiple blocks + TOCLIENT_BLOCKDATA = 0x20, 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 diff --git a/src/network/serveropcodes.cpp b/src/network/serveropcodes.cpp index 1165bb0656..1cb4134927 100644 --- a/src/network/serveropcodes.cpp +++ b/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 diff --git a/src/network/serveropcodes.h b/src/network/serveropcodes.h index dfaa52a0f1..c461da44a4 100644 --- a/src/network/serveropcodes.h +++ b/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); }; diff --git a/src/server.cpp b/src/server.cpp index 26b58245b9..fee0f557f9 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -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) { - // Do nothing + } catch (con::PeerNotFoundException &e) { + infostream << "Server: PeerNotFoundException" << std::endl; + } catch (ClientNotFoundException &e) { + infostream << "Server: ClientNotFoundException" << std::endl; } } } @@ -1189,7 +1191,7 @@ void Server::ProcessData(NetworkPacket *pkt) // Command must be handled into ToServerCommandHandler if (command >= TOSERVER_NUM_MSG_TYPES) { infostream << "Server: Ignoring unknown command " - << command << std::endl; + << static_cast(command) << std::endl; return; } @@ -1201,9 +1203,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" - " serialization format invalid or not initialized." - " Skipping incoming command=" << command << std::endl; + errorstream << "Server: Peer serialization format invalid. " + "Skipping incoming command " + << static_cast(command) << std::endl; return; } @@ -1216,9 +1218,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 " - << peer_id << " but client isn't active yet. Dropping packet " - << std::endl; + errorstream << "Server: Got packet command " + << static_cast(command) + << " for peer id " << peer_id + << " but client isn't active yet. Dropping packet." << std::endl; return; } @@ -1346,15 +1349,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, - clientCommandFactoryTable[pkt->getCommand()].channel, - pkt, - clientCommandFactoryTable[pkt->getCommand()].reliable); + m_clients.send(peer_id, pkt); } void Server::SendMovement(session_t peer_id) @@ -2137,9 +2138,8 @@ void Server::SendActiveObjectMessages(session_t peer_id, const std::string &data pkt.putRawString(datas.c_str(), datas.size()); - m_clients.send(pkt.getPeerId(), - reliable ? clientCommandFactoryTable[pkt.getCommand()].channel : 1, - &pkt, reliable); + auto &ccf = clientCommandFactoryTable[pkt.getCommand()]; + m_clients.sendCustom(pkt.getPeerId(), reliable ? ccf.channel : 1, &pkt, reliable); } void Server::SendCSMRestrictionFlags(session_t peer_id) @@ -2237,12 +2237,12 @@ s32 Server::playSound(ServerPlayingSound ¶ms, 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 +2261,7 @@ void Server::stopSound(s32 handle) pkt << handle; for (session_t peer_id : psound.clients) { - // Send as reliable - m_clients.send(peer_id, 0, &pkt, true); + Send(peer_id, &pkt); } // Remove sound reference @@ -2282,8 +2281,7 @@ void Server::fadeSound(s32 handle, float step, float gain) pkt << handle << step << gain; for (session_t peer_id : psound.clients) { - // Send as reliable - m_clients.send(peer_id, 0, &pkt, true); + Send(peer_id, &pkt); } // Remove sound reference @@ -2340,8 +2338,7 @@ void Server::sendNodeChangePkt(NetworkPacket &pkt, v3s16 block_pos, continue; } - // Send as reliable - m_clients.send(client_id, 0, &pkt, true); + Send(client_id, &pkt); } } @@ -3670,8 +3667,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.send(peer_id, 0, &legacy_pkt, true); + m_clients.sendCustom(peer_id, 1, &legacy_pkt, true); + m_clients.sendCustom(peer_id, 0, &legacy_pkt, true); } else { waiting.emplace(peer_id); Send(peer_id, &pkt); From c0f852e0167f23d7e8e9f1a1532a2314216b604d Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 24 Jan 2024 18:36:43 +0100 Subject: [PATCH 08/11] Change NetworkPacket to reserve instead of resize also make the bool serialization clearer and move the constructor to the header file --- src/network/networkpacket.cpp | 35 ++++++++--------------------------- src/network/networkpacket.h | 24 +++++++++++++++--------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/src/network/networkpacket.cpp b/src/network/networkpacket.cpp index c99b987cf2..5b7b51391f 100644 --- a/src/network/networkpacket.cpp +++ b/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): -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) +void NetworkPacket::checkReadOffset(u32 from_offset, u32 field_size) const { 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) { - m_datasize = m_read_offset + len; - m_data.resize(m_datasize); - } + checkDataSize(len); 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) diff --git a/src/network/networkpacket.h b/src/network/networkpacket.h index b9c39f332a..5f768f51af 100644 --- a/src/network/networkpacket.h +++ b/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 class NetworkPacket { - public: - NetworkPacket(u16 command, u32 datasize, session_t peer_id); - NetworkPacket(u16 command, u32 datasize); + NetworkPacket(u16 command, u32 preallocate, session_t peer_id) : + 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 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) { From 5dbc1d4c086031606f03d6d57b6504d1d888563e Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 24 Jan 2024 19:13:03 +0100 Subject: [PATCH 09/11] Move some files to src/server/ --- src/CMakeLists.txt | 4 ---- src/client/clientlauncher.cpp | 1 - src/script/lua_api/l_mainmenu.cpp | 1 - src/server.cpp | 6 +++--- src/server.h | 2 +- src/server/CMakeLists.txt | 4 ++++ src/{ => server}/ban.cpp | 0 src/{ => server}/ban.h | 0 src/{ => server}/clientiface.cpp | 0 src/{ => server}/clientiface.h | 0 src/{ => server}/rollback.cpp | 0 src/{ => server}/rollback.h | 0 src/{ => server}/serverlist.cpp | 0 src/{ => server}/serverlist.h | 4 +++- src/unittest/test_ban.cpp | 2 +- 15 files changed, 12 insertions(+), 12 deletions(-) rename src/{ => server}/ban.cpp (100%) rename src/{ => server}/ban.h (100%) rename src/{ => server}/clientiface.cpp (100%) rename src/{ => server}/clientiface.h (100%) rename src/{ => server}/rollback.cpp (100%) rename src/{ => server}/rollback.h (100%) rename src/{ => server}/serverlist.cpp (100%) rename src/{ => server}/serverlist.h (93%) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c6f998a81d..6879f2e86d 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/client/clientlauncher.cpp b/src/client/clientlauncher.cpp index eb4362348a..aaaf5c4837 100644 --- a/src/client/clientlauncher.cpp +++ b/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" diff --git a/src/script/lua_api/l_mainmenu.cpp b/src/script/lua_api/l_mainmenu.cpp index eef3cd6351..5bcf0f228e 100644 --- a/src/script/lua_api/l_mainmenu.cpp +++ b/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" diff --git a/src/server.cpp b/src/server.cpp index fee0f557f9..177144c094 100644 --- a/src/server.cpp +++ b/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" diff --git a/src/server.h b/src/server.h index 2915c88cd3..1c2394fd3d 100644 --- a/src/server.h +++ b/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" diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt index 0a5a8f3a75..d4afbc55bc 100644 --- a/src/server/CMakeLists.txt +++ b/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) diff --git a/src/ban.cpp b/src/server/ban.cpp similarity index 100% rename from src/ban.cpp rename to src/server/ban.cpp diff --git a/src/ban.h b/src/server/ban.h similarity index 100% rename from src/ban.h rename to src/server/ban.h diff --git a/src/clientiface.cpp b/src/server/clientiface.cpp similarity index 100% rename from src/clientiface.cpp rename to src/server/clientiface.cpp diff --git a/src/clientiface.h b/src/server/clientiface.h similarity index 100% rename from src/clientiface.h rename to src/server/clientiface.h diff --git a/src/rollback.cpp b/src/server/rollback.cpp similarity index 100% rename from src/rollback.cpp rename to src/server/rollback.cpp diff --git a/src/rollback.h b/src/server/rollback.h similarity index 100% rename from src/rollback.h rename to src/server/rollback.h diff --git a/src/serverlist.cpp b/src/server/serverlist.cpp similarity index 100% rename from src/serverlist.cpp rename to src/server/serverlist.cpp diff --git a/src/serverlist.h b/src/server/serverlist.h similarity index 93% rename from src/serverlist.h rename to src/server/serverlist.h index 1466eeaf65..2e2b8590fb 100644 --- a/src/serverlist.h +++ b/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 +} diff --git a/src/unittest/test_ban.cpp b/src/unittest/test_ban.cpp index 2b9ccc218e..6b36211ab5 100644 --- a/src/unittest/test_ban.cpp +++ b/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 { From 89f3502b566d590b6e608bfce73657e984c15f12 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 24 Jan 2024 19:39:28 +0100 Subject: [PATCH 10/11] Move Server ban check to different point --- src/network/serverpackethandler.cpp | 3 ++ src/server.cpp | 46 +++++++++++++---------------- src/server.h | 1 + 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index 253dcdba8d..3c6b635008 100644 --- a/src/network/serverpackethandler.cpp +++ b/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; diff --git a/src/server.cpp b/src/server.cpp index 177144c094..baacce3f6b 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1160,31 +1160,6 @@ 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(); @@ -3287,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) @@ -3299,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 diff --git a/src/server.h b/src/server.h index 1c2394fd3d..c57dde0d73 100644 --- a/src/server.h +++ b/src/server.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); From fbec168e91026b70fd989e7f46bea7140328076e Mon Sep 17 00:00:00 2001 From: grorp Date: Sat, 27 Jan 2024 14:37:00 +0100 Subject: [PATCH 11/11] Only pause rendering if the Android activity is stopped (#14211) --- src/client/game.cpp | 2 +- src/client/renderingengine.h | 11 ----------- src/gui/guiEngine.cpp | 5 ++--- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/client/game.cpp b/src/client/game.cpp index 1e62b4b7c4..546dfa7ec3 100644 --- a/src/client/game.cpp +++ b/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 ==================== diff --git a/src/client/renderingengine.h b/src/client/renderingengine.h index 06c3157d56..7d2291172d 100644 --- a/src/client/renderingengine.h +++ b/src/client/renderingengine.h @@ -141,17 +141,6 @@ public: const irr::core::dimension2d 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; diff --git a/src/gui/guiEngine.cpp b/src/gui/guiEngine.cpp index 4bb7b041fd..1ae463e37e 100644 --- a/src/gui/guiEngine.cpp +++ b/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"));