From 452160cd000ab77dd35c80ae185966cf7dc28757 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Tue, 13 May 2025 17:43:30 +0200 Subject: [PATCH 1/4] Clean up read_tiledef and related parts a bit --- src/client/tile.h | 2 - src/script/common/c_content.cpp | 85 ++++++++++++++++++++------------- src/script/cpp_api/s_node.h | 1 - 3 files changed, 51 insertions(+), 37 deletions(-) diff --git a/src/client/tile.h b/src/client/tile.h index ffbe78bac..3293b4dd1 100644 --- a/src/client/tile.h +++ b/src/client/tile.h @@ -191,8 +191,6 @@ struct TileSpec bool world_aligned = false; //! Tile rotation. TileRotation rotation = TileRotation::None; - //! This much light does the tile emit. - u8 emissive_light = 0; //! The first is base texture, the second is overlay. TileLayer layers[MAX_TILE_LAYERS]; }; diff --git a/src/script/common/c_content.cpp b/src/script/common/c_content.cpp index e4d94b3fc..91aa5b405 100644 --- a/src/script/common/c_content.cpp +++ b/src/script/common/c_content.cpp @@ -56,9 +56,16 @@ void read_item_definition(lua_State* L, int index, if (index < 0) index = lua_gettop(L) + 1 + index; - def.type = (ItemType)getenumfield(L, index, "type", - es_ItemType, ITEM_NONE); + def.name.clear(); getstringfield(L, index, "name", def.name); + + { + auto str = getstringfield_default(L, index, "type", ""); + if (!string_to_enum(es_ItemType, def.type, str)) + warningstream << "Item " << def.name + << " has unknown type \"" << str << '"' << std::endl; + } + getstringfield(L, index, "description", def.description); getstringfield(L, index, "short_description", def.short_description); getstringfield(L, index, "inventory_image", def.inventory_image); @@ -605,9 +612,6 @@ TileDef read_tiledef(lua_State *L, int index, u8 drawtype, bool special) case NDT_PLANTLIKE: case NDT_FIRELIKE: default_tiling = false; - // "break" is omitted here intentionaly, as PLANTLIKE - // FIRELIKE drawtype both should default to having - // backface_culling to false. [[fallthrough]]; case NDT_MESH: case NDT_LIQUID: @@ -621,7 +625,6 @@ TileDef read_tiledef(lua_State *L, int index, u8 drawtype, bool special) break; } - // key at index -2 and value at index if(lua_isstring(L, index)){ // "default_lava.png" tiledef.name = lua_tostring(L, index); @@ -634,7 +637,10 @@ TileDef read_tiledef(lua_State *L, int index, u8 drawtype, bool special) // name="default_lava.png" tiledef.name.clear(); getstringfield(L, index, "name", tiledef.name); - getstringfield(L, index, "image", tiledef.name); // MaterialSpec compat. + warn_if_field_exists(L, index, "image", "TileDef", + "Deprecated: new name is \"name\"."); + getstringfield(L, index, "image", tiledef.name); + tiledef.backface_culling = getboolfield_default( L, index, "backface_culling", default_culling); tiledef.tileable_horizontal = getboolfield_default( @@ -659,6 +665,9 @@ TileDef read_tiledef(lua_State *L, int index, u8 drawtype, bool special) lua_getfield(L, index, "animation"); tiledef.animation = read_animation_definition(L, -1); lua_pop(L, 1); + } else if (!lua_isnil(L, index)) { + // TODO: should be an error + errorstream << "TileDef: Invalid type! (expected string or table)" << std::endl; } return tiledef; @@ -672,13 +681,13 @@ void read_content_features(lua_State *L, ContentFeatures &f, int index) /* Cache existence of some callbacks */ lua_getfield(L, index, "on_construct"); - if(!lua_isnil(L, -1)) f.has_on_construct = true; + f.has_on_construct = !lua_isnil(L, -1); lua_pop(L, 1); lua_getfield(L, index, "on_destruct"); - if(!lua_isnil(L, -1)) f.has_on_destruct = true; + f.has_on_destruct = !lua_isnil(L, -1); lua_pop(L, 1); lua_getfield(L, index, "after_destruct"); - if(!lua_isnil(L, -1)) f.has_after_destruct = true; + f.has_after_destruct = !lua_isnil(L, -1); lua_pop(L, 1); lua_getfield(L, index, "on_rightclick"); @@ -695,8 +704,13 @@ void read_content_features(lua_State *L, ContentFeatures &f, int index) /* Visual definition */ - f.drawtype = (NodeDrawType)getenumfield(L, index, "drawtype", - ScriptApiNode::es_DrawType,NDT_NORMAL); + { + auto str = getstringfield_default(L, index, "drawtype", ""); + if (!string_to_enum(ScriptApiNode::es_DrawType, f.drawtype, str)) + warningstream << "Node " << f.name + << " has unknown drawtype \"" << str << '"' << std::endl; + } + getfloatfield(L, index, "visual_scale", f.visual_scale); /* Meshnode model filename */ @@ -796,10 +810,7 @@ void read_content_features(lua_State *L, ContentFeatures &f, int index) if (lua_toboolean(L, -1)) f.alpha = (f.drawtype == NDT_NORMAL) ? ALPHAMODE_CLIP : ALPHAMODE_BLEND; } else if (check_field_or_nil(L, -1, LUA_TSTRING, "use_texture_alpha")) { - int result = f.alpha; - string_to_enum(ScriptApiNode::es_TextureAlphaMode, result, - std::string(lua_tostring(L, -1))); - f.alpha = static_cast(result); + string_to_enum(ScriptApiNode::es_TextureAlphaMode, f.alpha, lua_tostring(L, -1)); } lua_pop(L, 1); @@ -817,10 +828,18 @@ void read_content_features(lua_State *L, ContentFeatures &f, int index) getboolfield(L, index, "post_effect_color_shaded", f.post_effect_color_shaded); - f.param_type = (ContentParamType)getenumfield(L, index, "paramtype", - ScriptApiNode::es_ContentParamType, CPT_NONE); - f.param_type_2 = (ContentParamType2)getenumfield(L, index, "paramtype2", - ScriptApiNode::es_ContentParamType2, CPT2_NONE); + { + auto str = getstringfield_default(L, index, "paramtype", ""); + if (!string_to_enum(ScriptApiNode::es_ContentParamType, f.param_type, str)) + warningstream << "Node " << f.name + << " has unknown paramtype \"" << str << '"' << std::endl; + } + { + auto str = getstringfield_default(L, index, "paramtype2", ""); + if (!string_to_enum(ScriptApiNode::es_ContentParamType2, f.param_type_2, str)) + warningstream << "Node " << f.name + << " has unknown paramtype2 \"" << str << '"' << std::endl; + } if (!f.palette_name.empty() && !(f.param_type_2 == CPT2_COLOR || @@ -855,8 +874,12 @@ void read_content_features(lua_State *L, ContentFeatures &f, int index) // Liquids flow into and replace node getboolfield(L, index, "floodable", f.floodable); // Whether the node is non-liquid, source liquid or flowing liquid - f.liquid_type = (LiquidType)getenumfield(L, index, "liquidtype", - ScriptApiNode::es_LiquidType, LIQUID_NONE); + { + auto str = getstringfield_default(L, index, "liquidtype", ""); + if (!string_to_enum(ScriptApiNode::es_LiquidType, f.liquid_type, str)) + warningstream << "Node " << f.name + << " has unknown liquidtype \"" << str << '"' << std::endl; + } // If the content is liquid, this is the flowing version of the liquid. getstringfield(L, index, "liquid_alternative_flowing", f.liquid_alternative_flowing); @@ -915,7 +938,7 @@ void read_content_features(lua_State *L, ContentFeatures &f, int index) lua_pushnil(L); while (lua_next(L, table) != 0) { // Value at -1 - std::string side(lua_tostring(L, -1)); + std::string_view side(lua_tostring(L, -1)); // Note faces are flipped to make checking easier if (side == "top") f.connect_sides |= 2; @@ -986,6 +1009,7 @@ void read_content_features(lua_State *L, ContentFeatures &f, int index) } else if(lua_isnil(L, -1)) { f.liquid_move_physics = f.liquid_type != LIQUID_NONE; } else { + // TODO: should be an error errorstream << "Field \"liquid_move_physics\": Invalid type!" << std::endl; } lua_pop(L, 1); @@ -1805,10 +1829,8 @@ WearBarParams read_wear_bar_params( auto blend = WearBarParams::BLEND_MODE_CONSTANT; lua_getfield(L, stack_idx, "blend"); if (check_field_or_nil(L, -1, LUA_TSTRING, "blend")) { - int blendInt; - if (!string_to_enum(WearBarParams::es_BlendMode, blendInt, std::string(lua_tostring(L, -1)))) + if (!string_to_enum(WearBarParams::es_BlendMode, blend, lua_tostring(L, -1))) throw LuaError("Invalid wear bar color blend mode"); - blend = static_cast(blendInt); } lua_pop(L, 1); @@ -2395,14 +2417,9 @@ void push_hud_element(lua_State *L, HudElement *elem) bool read_hud_change(lua_State *L, HudElementStat &stat, HudElement *elem, void **value) { std::string statstr = lua_tostring(L, 3); - { - int statint; - if (!string_to_enum(es_HudElementStat, statint, statstr)) { - script_log_unique(L, "Unknown HUD stat type: " + statstr, warningstream); - return false; - } - - stat = static_cast(statint); + if (!string_to_enum(es_HudElementStat, stat, statstr)) { + script_log_unique(L, "Unknown HUD stat type: " + statstr, warningstream); + return false; } switch (stat) { diff --git a/src/script/cpp_api/s_node.h b/src/script/cpp_api/s_node.h index bf4dc6af5..2c7133df1 100644 --- a/src/script/cpp_api/s_node.h +++ b/src/script/cpp_api/s_node.h @@ -38,7 +38,6 @@ public: static struct EnumString es_ContentParamType[]; static struct EnumString es_ContentParamType2[]; static struct EnumString es_LiquidType[]; - static struct EnumString es_LiquidMoveType[]; static struct EnumString es_NodeBoxType[]; static struct EnumString es_TextureAlphaMode[]; }; From 1214a1d4a6d3023e45feb5be034a83ca30b3c732 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sat, 24 May 2025 22:49:29 +0200 Subject: [PATCH 2/4] Refactor ITextureSource use in main menu (#16135) --- irr/include/IVideoDriver.h | 4 ++-- src/client/clientlauncher.cpp | 19 +++++++++++++++---- src/client/shadows/dynamicshadowsrender.cpp | 2 +- src/gui/guiEngine.cpp | 19 +++++-------------- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/irr/include/IVideoDriver.h b/irr/include/IVideoDriver.h index f392eb636..02c9a2d4f 100644 --- a/irr/include/IVideoDriver.h +++ b/irr/include/IVideoDriver.h @@ -172,7 +172,7 @@ public: \return Pointer to the texture, or 0 if the texture could not be loaded. This pointer should not be dropped. See IReferenceCounted::drop() for more information. */ - virtual ITexture *getTexture(const io::path &filename) = 0; + [[deprecated]] virtual ITexture *getTexture(const io::path &filename) = 0; //! Get access to a named texture. /** Loads the texture from disk if it is not @@ -184,7 +184,7 @@ public: \return Pointer to the texture, or 0 if the texture could not be loaded. This pointer should not be dropped. See IReferenceCounted::drop() for more information. */ - virtual ITexture *getTexture(io::IReadFile *file) = 0; + [[deprecated]] virtual ITexture *getTexture(io::IReadFile *file) = 0; //! Returns amount of textures currently loaded /** \return Amount of textures currently loaded */ diff --git a/src/client/clientlauncher.cpp b/src/client/clientlauncher.cpp index a770dbcae..64f2e8f51 100644 --- a/src/client/clientlauncher.cpp +++ b/src/client/clientlauncher.cpp @@ -326,6 +326,18 @@ void ClientLauncher::setting_changed_callback(const std::string &name, void *dat static_cast(data)->config_guienv(); } +static video::ITexture *loadTexture(video::IVideoDriver *driver, const char *path) +{ + // FIXME?: it would be cleaner to do this through a ITextureSource, but we don't have one + video::ITexture *texture = nullptr; + verbosestream << "Loading texture " << path << std::endl; + if (auto *image = driver->createImageFromFile(path); image) { + texture = driver->addTexture(fs::GetFilenameFromPath(path), image); + image->drop(); + } + return texture; +} + void ClientLauncher::config_guienv() { gui::IGUISkin *skin = guienv->getSkin(); @@ -364,10 +376,9 @@ void ClientLauncher::config_guienv() if (cached_id != sprite_ids.end()) { skin->setIcon(gui::EGDI_CHECK_BOX_CHECKED, cached_id->second); } else { - gui::IGUISpriteBank *sprites = skin->getSpriteBank(); - video::IVideoDriver *driver = m_rendering_engine->get_video_driver(); - video::ITexture *texture = driver->getTexture(path.c_str()); - s32 id = sprites->addTextureAsSprite(texture); + auto *driver = m_rendering_engine->get_video_driver(); + auto *texture = loadTexture(driver, path.c_str()); + s32 id = skin->getSpriteBank()->addTextureAsSprite(texture); if (id != -1) { skin->setIcon(gui::EGDI_CHECK_BOX_CHECKED, id); sprite_ids.emplace(path, id); diff --git a/src/client/shadows/dynamicshadowsrender.cpp b/src/client/shadows/dynamicshadowsrender.cpp index 17260e21d..dbabf6dd7 100644 --- a/src/client/shadows/dynamicshadowsrender.cpp +++ b/src/client/shadows/dynamicshadowsrender.cpp @@ -410,7 +410,7 @@ video::ITexture *ShadowRenderer::getSMTexture(const std::string &shadow_map_name shadow_map_name.c_str(), texture_format); } - return m_driver->getTexture(shadow_map_name.c_str()); + return m_driver->findTexture(shadow_map_name.c_str()); } void ShadowRenderer::renderShadowMap(video::ITexture *target, diff --git a/src/gui/guiEngine.cpp b/src/gui/guiEngine.cpp index 8cc9954fc..aef76aec7 100644 --- a/src/gui/guiEngine.cpp +++ b/src/gui/guiEngine.cpp @@ -74,6 +74,7 @@ video::ITexture *MenuTextureSource::getTexture(const std::string &name, u32 *id) if (retval) return retval; + verbosestream << "MenuTextureSource: loading " << name << std::endl; video::IImage *image = m_driver->createImageFromFile(name.c_str()); if (!image) return NULL; @@ -597,26 +598,16 @@ void GUIEngine::drawFooter(video::IVideoDriver *driver) bool GUIEngine::setTexture(texture_layer layer, const std::string &texturepath, bool tile_image, unsigned int minsize) { - video::IVideoDriver *driver = m_rendering_engine->get_video_driver(); + m_textures[layer].texture = nullptr; - if (m_textures[layer].texture) { - driver->removeTexture(m_textures[layer].texture); - m_textures[layer].texture = NULL; - } - - if (texturepath.empty() || !fs::PathExists(texturepath)) { + if (texturepath.empty() || !fs::PathExists(texturepath)) return false; - } - m_textures[layer].texture = driver->getTexture(texturepath.c_str()); + m_textures[layer].texture = m_texture_source->getTexture(texturepath); m_textures[layer].tile = tile_image; m_textures[layer].minsize = minsize; - if (!m_textures[layer].texture) { - return false; - } - - return true; + return m_textures[layer].texture != nullptr; } /******************************************************************************/ From e9b32843a5989026b6ee49b525c965577d0ab17d Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Thu, 22 May 2025 17:02:22 -0500 Subject: [PATCH 3/4] Make MTP server shutdown flag atomic I noticed this potential data race while reading the code. I have not detected it with TSan in practice. --- src/network/mtp/impl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/mtp/impl.h b/src/network/mtp/impl.h index 3f81fe27b..b8059f202 100644 --- a/src/network/mtp/impl.h +++ b/src/network/mtp/impl.h @@ -12,6 +12,7 @@ #include "util/numeric.h" #include "porting.h" #include "network/networkprotocol.h" +#include #include #include #include @@ -301,7 +302,7 @@ private: // Backwards compatibility PeerHandler *m_bc_peerhandler; - bool m_shutting_down = false; + std::atomic m_shutting_down = false; }; } // namespace From fa0c09d202537acbd267a38958f16c883359baca Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Thu, 22 May 2025 17:03:11 -0500 Subject: [PATCH 4/4] Do not modify peer timeout on shutdown Shortening the peer timeout was supposedly necessary at some point to work around an unknown bug. I was not able to reproduce the bug running a headless Luanti server on WSL Tumbleweed and connecting with a client on the Windows host. That is not enough to say the issue no longer exists. This commit may cause a regression. The access to change the peer timeout was unsynchronized and done by a different thread than the sending thread, so it was detected by TSan to be a data race. Since this patch deletes the code performing the write, the data race is no longer a concern and no synchronization must be added. --- src/network/mtp/impl.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/network/mtp/impl.cpp b/src/network/mtp/impl.cpp index 07bdfc735..d54c116a2 100644 --- a/src/network/mtp/impl.cpp +++ b/src/network/mtp/impl.cpp @@ -1314,11 +1314,6 @@ Connection::~Connection() m_sendThread->stop(); m_receiveThread->stop(); - //TODO for some unkonwn reason send/receive threads do not exit as they're - // supposed to be but wait on peer timeout. To speed up shutdown we reduce - // timeout to half a second. - m_sendThread->setPeerTimeout(0.5); - // wait for threads to finish m_sendThread->wait(); m_receiveThread->wait();