From 4af592ec0247579c00f163c06d1ffde6fe8e7738 Mon Sep 17 00:00:00 2001 From: cx384 Date: Mon, 27 Jan 2025 12:27:06 +0100 Subject: [PATCH 1/3] Replace object visual by enum --- src/client/content_cao.cpp | 48 ++++++++++++++++++++------------- src/object_properties.cpp | 28 ++++++++++++++++--- src/object_properties.h | 17 +++++++++++- src/script/common/c_content.cpp | 10 +++++-- src/server/player_sao.cpp | 2 +- 5 files changed, 79 insertions(+), 26 deletions(-) diff --git a/src/client/content_cao.cpp b/src/client/content_cao.cpp index 5c044ddeffdb..18c590d03efd 100644 --- a/src/client/content_cao.cpp +++ b/src/client/content_cao.cpp @@ -597,7 +597,7 @@ void GenericCAO::addToScene(ITextureSource *tsrc, scene::ISceneManager *smgr) if (!m_prop.is_visible) return; - infostream << "GenericCAO::addToScene(): " << m_prop.visual << std::endl; + infostream << "GenericCAO::addToScene(): " << enum_to_string(es_ObjectVisual, m_prop.visual) << std::endl; m_material_type_param = 0.5f; // May cut off alpha < 128 depending on m_material_type @@ -634,7 +634,8 @@ void GenericCAO::addToScene(ITextureSource *tsrc, scene::ISceneManager *smgr) node->forEachMaterial(setMaterial); }; - if (m_prop.visual == "sprite") { + switch(m_prop.visual) { + case OBJECTVISUAL_SPRITE: { grabMatrixNode(); m_spritenode = m_smgr->addBillboardSceneNode( m_matrixnode, v2f(1, 1), v3f(0,0,0), -1); @@ -654,7 +655,8 @@ void GenericCAO::addToScene(ITextureSource *tsrc, scene::ISceneManager *smgr) setBillboardTextureMatrix(m_spritenode, txs, tys, 0, 0); } - } else if (m_prop.visual == "upright_sprite") { + break; + } case OBJECTVISUAL_UPRIGHT_SPRITE: { grabMatrixNode(); auto mesh = make_irr(); f32 dx = BS * m_prop.visual_size.X / 2; @@ -696,7 +698,8 @@ void GenericCAO::addToScene(ITextureSource *tsrc, scene::ISceneManager *smgr) mesh->recalculateBoundingBox(); m_meshnode = m_smgr->addMeshSceneNode(mesh.get(), m_matrixnode); m_meshnode->grab(); - } else if (m_prop.visual == "cube") { + break; + } case OBJECTVISUAL_CUBE: { grabMatrixNode(); scene::IMesh *mesh = createCubeMesh(v3f(BS,BS,BS)); m_meshnode = m_smgr->addMeshSceneNode(mesh, m_matrixnode); @@ -710,7 +713,8 @@ void GenericCAO::addToScene(ITextureSource *tsrc, scene::ISceneManager *smgr) m_meshnode->forEachMaterial([this] (auto &mat) { mat.BackfaceCulling = m_prop.backface_culling; }); - } else if (m_prop.visual == "mesh") { + break; + } case OBJECTVISUAL_MESH: { grabMatrixNode(); scene::IAnimatedMesh *mesh = m_client->getMesh(m_prop.mesh, true); if (mesh) { @@ -737,7 +741,10 @@ void GenericCAO::addToScene(ITextureSource *tsrc, scene::ISceneManager *smgr) }); } else errorstream<<"GenericCAO::addToScene(): Could not load mesh "<setItem(item, m_client, - (m_prop.visual == "wielditem")); + (m_prop.visual == OBJECTVISUAL_WIELDITEM)); m_wield_meshnode->setScale(m_prop.visual_size / 2.0f); - } else { - infostream<<"GenericCAO::addToScene(): \""<getMesh()->setHardwareMappingHint(scene::EHM_STATIC); if (m_animated_meshnode) { @@ -873,7 +883,7 @@ void GenericCAO::updateLight(u32 day_night_ratio) void GenericCAO::setNodeLight(const video::SColor &light_color) { - if (m_prop.visual == "wielditem" || m_prop.visual == "item") { + if (m_prop.visual == OBJECTVISUAL_WIELDITEM || m_prop.visual == OBJECTVISUAL_ITEM) { if (m_wield_meshnode) m_wield_meshnode->setNodeLightColor(light_color); return; @@ -1259,7 +1269,7 @@ void GenericCAO::updateTexturePos() } else if (m_meshnode) { - if (m_prop.visual == "upright_sprite") { + if (m_prop.visual == OBJECTVISUAL_UPRIGHT_SPRITE) { int row = m_tx_basepos.Y; int col = m_tx_basepos.X; @@ -1293,7 +1303,7 @@ void GenericCAO::updateTextures(std::string mod) m_current_texture_modifier = mod; if (m_spritenode) { - if (m_prop.visual == "sprite") { + if (m_prop.visual == OBJECTVISUAL_SPRITE) { std::string texturestring = "no_texture.png"; if (!m_prop.textures.empty()) texturestring = m_prop.textures[0]; @@ -1312,7 +1322,7 @@ void GenericCAO::updateTextures(std::string mod) } else if (m_animated_meshnode) { - if (m_prop.visual == "mesh") { + if (m_prop.visual == OBJECTVISUAL_MESH) { for (u32 i = 0; i < m_animated_meshnode->getMaterialCount(); ++i) { const auto texture_idx = m_animated_meshnode->getMesh()->getTextureSlot(i); if (texture_idx >= m_prop.textures.size()) @@ -1350,7 +1360,7 @@ void GenericCAO::updateTextures(std::string mod) } else if (m_meshnode) { - if(m_prop.visual == "cube") + if(m_prop.visual == OBJECTVISUAL_CUBE) { for (u32 i = 0; i < 6; ++i) { @@ -1371,7 +1381,7 @@ void GenericCAO::updateTextures(std::string mod) use_anisotropic_filter); }); } - } else if (m_prop.visual == "upright_sprite") { + } else if (m_prop.visual == OBJECTVISUAL_UPRIGHT_SPRITE) { scene::IMesh *mesh = m_meshnode->getMesh(); { std::string tname = "no_texture.png"; @@ -1535,7 +1545,7 @@ bool GenericCAO::visualExpiryRequired(const ObjectProperties &new_) const */ bool uses_legacy_texture = new_.wield_item.empty() && - (new_.visual == "wielditem" || new_.visual == "item"); + (new_.visual == OBJECTVISUAL_WIELDITEM || new_.visual == OBJECTVISUAL_ITEM); // Ordered to compare primitive types before std::vectors return old.backface_culling != new_.backface_culling || old.is_visible != new_.is_visible || @@ -1926,7 +1936,7 @@ void GenericCAO::updateMeshCulling() if (!node) return; - if (m_prop.visual == "upright_sprite") { + if (m_prop.visual == OBJECTVISUAL_UPRIGHT_SPRITE) { // upright sprite has no backface culling node->forEachMaterial([=] (auto &mat) { mat.FrontfaceCulling = hidden; diff --git a/src/object_properties.cpp b/src/object_properties.cpp index 63a0ef01bc43..06cf72111835 100644 --- a/src/object_properties.cpp +++ b/src/object_properties.cpp @@ -8,11 +8,24 @@ #include "exceptions.h" #include "log.h" #include "util/serialize.h" +#include "util/enum_string.h" #include #include static const video::SColor NULL_BGCOLOR{0, 1, 1, 1}; +const struct EnumString es_ObjectVisual[] = +{ + {OBJECTVISUAL_UNKNOWN, "unknown"}, + {OBJECTVISUAL_SPRITE, "sprite"}, + {OBJECTVISUAL_UPRIGHT_SPRITE, "upright_sprite"}, + {OBJECTVISUAL_CUBE, "cube"}, + {OBJECTVISUAL_MESH, "mesh"}, + {OBJECTVISUAL_ITEM, "item"}, + {OBJECTVISUAL_WIELDITEM, "wielditem"}, + {0, nullptr}, +}; + ObjectProperties::ObjectProperties() { textures.emplace_back("no_texture.png"); @@ -27,7 +40,7 @@ std::string ObjectProperties::dump() const os << ", physical=" << physical; os << ", collideWithObjects=" << collideWithObjects; os << ", collisionbox=" << collisionbox.MinEdge << "," << collisionbox.MaxEdge; - os << ", visual=" << visual; + os << ", visual=" << enum_to_string(es_ObjectVisual, visual); os << ", mesh=" << mesh; os << ", visual_size=" << visual_size; os << ", textures=["; @@ -136,7 +149,10 @@ void ObjectProperties::serialize(std::ostream &os) const writeV3F32(os, selectionbox.MinEdge); writeV3F32(os, selectionbox.MaxEdge); Pointabilities::serializePointabilityType(os, pointable); - os << serializeString16(visual); + + // Convert to string for compatibility + os << serializeString16(enum_to_string(es_ObjectVisual, visual)); + writeV3F32(os, visual_size); writeU16(os, textures.size()); for (const std::string &texture : textures) { @@ -197,7 +213,13 @@ void ObjectProperties::deSerialize(std::istream &is) selectionbox.MinEdge = readV3F32(is); selectionbox.MaxEdge = readV3F32(is); pointable = Pointabilities::deSerializePointabilityType(is); - visual = deSerializeString16(is); + + int result; + if (string_to_enum(es_ObjectVisual, result, deSerializeString16(is))) + visual = static_cast(result); + else + visual = OBJECTVISUAL_UNKNOWN; + visual_size = readV3F32(is); textures.clear(); u32 texture_count = readU16(is); diff --git a/src/object_properties.h b/src/object_properties.h index d97b4997eb36..329e1ceac2d7 100644 --- a/src/object_properties.h +++ b/src/object_properties.h @@ -11,6 +11,21 @@ #include #include "util/pointabilities.h" +struct EnumString; + +enum ObjectVisual : u8 { + OBJECTVISUAL_UNKNOWN, + OBJECTVISUAL_SPRITE, + OBJECTVISUAL_UPRIGHT_SPRITE, + OBJECTVISUAL_CUBE, + OBJECTVISUAL_MESH, + OBJECTVISUAL_ITEM, + OBJECTVISUAL_WIELDITEM, +}; + +extern const EnumString es_ObjectVisual[]; + + struct ObjectProperties { /* member variables ordered roughly by size */ @@ -21,7 +36,7 @@ struct ObjectProperties aabb3f collisionbox = aabb3f(-0.5f, -0.5f, -0.5f, 0.5f, 0.5f, 0.5f); // Values are BS=1 aabb3f selectionbox = aabb3f(-0.5f, -0.5f, -0.5f, 0.5f, 0.5f, 0.5f); - std::string visual = "sprite"; + ObjectVisual visual = OBJECTVISUAL_SPRITE; std::string mesh; std::string damage_texture_modifier = "^[brighten"; std::string nametag; diff --git a/src/script/common/c_content.cpp b/src/script/common/c_content.cpp index 146609185441..f9dbd90ed5fe 100644 --- a/src/script/common/c_content.cpp +++ b/src/script/common/c_content.cpp @@ -351,7 +351,13 @@ void read_object_properties(lua_State *L, int index, } lua_pop(L, 1); - getstringfield(L, -1, "visual", prop->visual); + // Don't set if nil + std::string visual; + if (getstringfield(L, -1, "visual", visual)) { + int result; + prop->visual = string_to_enum(es_ObjectVisual, result, visual) ? + static_cast(result) : OBJECTVISUAL_UNKNOWN; + } getstringfield(L, -1, "mesh", prop->mesh); @@ -490,7 +496,7 @@ void push_object_properties(lua_State *L, const ObjectProperties *prop) lua_setfield(L, -2, "selectionbox"); push_pointability_type(L, prop->pointable); lua_setfield(L, -2, "pointable"); - lua_pushlstring(L, prop->visual.c_str(), prop->visual.size()); + lua_pushstring(L, enum_to_string(es_ObjectVisual, prop->visual)); lua_setfield(L, -2, "visual"); lua_pushlstring(L, prop->mesh.c_str(), prop->mesh.size()); lua_setfield(L, -2, "mesh"); diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp index 5d6b891fa95f..60d6eadca4fa 100644 --- a/src/server/player_sao.cpp +++ b/src/server/player_sao.cpp @@ -26,7 +26,7 @@ PlayerSAO::PlayerSAO(ServerEnvironment *env_, RemotePlayer *player_, session_t p m_prop.selectionbox = aabb3f(-0.3f, 0.0f, -0.3f, 0.3f, 1.77f, 0.3f); m_prop.pointable = PointabilityType::POINTABLE; // Start of default appearance, this should be overwritten by Lua - m_prop.visual = "upright_sprite"; + m_prop.visual = OBJECTVISUAL_UPRIGHT_SPRITE; m_prop.visual_size = v3f(1, 2, 1); m_prop.textures.clear(); m_prop.textures.emplace_back("player.png"); From d5916ec00c6790aad06b21c19bfdb2f89d757c8c Mon Sep 17 00:00:00 2001 From: cx384 Date: Mon, 27 Jan 2025 17:28:45 +0100 Subject: [PATCH 2/3] Improve handling of unsupported ObjectVisual --- src/client/content_cao.cpp | 3 --- src/object_properties.cpp | 8 ++++++-- src/script/common/c_content.cpp | 8 ++++++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/client/content_cao.cpp b/src/client/content_cao.cpp index 18c590d03efd..ae32c0e51bbf 100644 --- a/src/client/content_cao.cpp +++ b/src/client/content_cao.cpp @@ -767,9 +767,6 @@ void GenericCAO::addToScene(ITextureSource *tsrc, scene::ISceneManager *smgr) m_wield_meshnode->setScale(m_prop.visual_size / 2.0f); break; } default: - infostream << "GenericCAO::addToScene(): \"" - << enum_to_string(es_ObjectVisual, m_prop.visual) - << "\" not supported"<(result); - else + else { + infostream << "ObjectProperties::deSerialize() ObjectVisual: \"" << visual_string + << "\" not supported" << std::endl; visual = OBJECTVISUAL_UNKNOWN; + } visual_size = readV3F32(is); textures.clear(); diff --git a/src/script/common/c_content.cpp b/src/script/common/c_content.cpp index f9dbd90ed5fe..6b0ad4cd228c 100644 --- a/src/script/common/c_content.cpp +++ b/src/script/common/c_content.cpp @@ -355,8 +355,12 @@ void read_object_properties(lua_State *L, int index, std::string visual; if (getstringfield(L, -1, "visual", visual)) { int result; - prop->visual = string_to_enum(es_ObjectVisual, result, visual) ? - static_cast(result) : OBJECTVISUAL_UNKNOWN; + if (string_to_enum(es_ObjectVisual, result, visual)) + prop->visual = static_cast(result); + else { + script_log_unique(L, "Unsupported ObjectVisual: " + visual, warningstream); + prop->visual = OBJECTVISUAL_UNKNOWN; + } } getstringfield(L, -1, "mesh", prop->mesh); From 24fac822e0749dd0786d733c182c4ebae07fe099 Mon Sep 17 00:00:00 2001 From: cx384 Date: Mon, 27 Jan 2025 18:25:12 +0100 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: sfan5 --- src/object_properties.cpp | 8 ++++---- src/script/common/c_content.cpp | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/object_properties.cpp b/src/object_properties.cpp index 9582128e30d0..b54fc815c6fd 100644 --- a/src/object_properties.cpp +++ b/src/object_properties.cpp @@ -215,11 +215,11 @@ void ObjectProperties::deSerialize(std::istream &is) pointable = Pointabilities::deSerializePointabilityType(is); int result; - std::string visual_string{deSerializeString16(is)}; - if (string_to_enum(es_ObjectVisual, result, visual_string)) + std::string visual_str = deSerializeString16(is); + if (string_to_enum(es_ObjectVisual, result, visual_str)) { visual = static_cast(result); - else { - infostream << "ObjectProperties::deSerialize() ObjectVisual: \"" << visual_string + } else { + infostream << "ObjectProperties::deSerialize() visual \"" << visual_str << "\" not supported" << std::endl; visual = OBJECTVISUAL_UNKNOWN; } diff --git a/src/script/common/c_content.cpp b/src/script/common/c_content.cpp index 6b0ad4cd228c..9692f3c4f85d 100644 --- a/src/script/common/c_content.cpp +++ b/src/script/common/c_content.cpp @@ -355,9 +355,9 @@ void read_object_properties(lua_State *L, int index, std::string visual; if (getstringfield(L, -1, "visual", visual)) { int result; - if (string_to_enum(es_ObjectVisual, result, visual)) + if (string_to_enum(es_ObjectVisual, result, visual)) { prop->visual = static_cast(result); - else { + } else { script_log_unique(L, "Unsupported ObjectVisual: " + visual, warningstream); prop->visual = OBJECTVISUAL_UNKNOWN; }