Refactor texture overrides and add new features (#9600)

* Refactor texture overrides, and add new features:

- Texture overrides can support multiple targets in one line
- Texture override files can have comment lines
- Item images/wield images can be overridden

* Formatting changes

* Address soime feedback

- Pass vectors by const reference
- Log syntax errors as warnings
- Remove 'C' prefix from TextureOverrideSource

* Simplify override target checks with an inline helper function

* make linter happy

* Apply feedback suggestions

Co-Authored-By: rubenwardy <rw@rubenwardy.com>

* Remove remaining != 0 checks

* Update copyright notice

Co-authored-by: sfan5 <sfan5@live.de>
Co-authored-by: rubenwardy <rw@rubenwardy.com>

doc/texture_packs.txt

@@ -145,34 +145,51 @@ are placeholders intended to be overwritten by the game.
 Texture Overrides
 -----------------
 
-You can override the textures of a node from a texture pack using
+You can override the textures of nodes and items from a
-texture overrides. To do this, create a file in a texture pack
+texture pack using texture overrides. To do this, create one or
-called override.txt
+more files in a texture pack called override.txt
 
 Each line in an override.txt file is a rule. It consists of
 
-	nodename face-selector texture
+	itemname target texture
 
 For example,
 
 	default:dirt_with_grass sides default_stone.png
 
-You can use ^ operators as usual:
+or
+
+	default:sword_steel inventory my_steel_sword.png
+
+You can list multiple targets on one line as a comma-separated list:
+
+	default:tree top,bottom my_special_tree.png
+
+You can use texture modifiers, as usual:
 
 	default:dirt_with_grass sides default_stone.png^[brighten
 
-Here are face selectors you can choose from:
+Finally, if a line is empty or starts with '#' it will be considered
+a comment and not read as a rule. You can use this to better organize
+your override.txt files.
 
-| face-selector | behavior                                          |
+Here are targets you can choose from:
+
+| target        | behavior                                          |
 |---------------|---------------------------------------------------|
-| left          | x-                                                |
+| left          | x- face                                           |
-| right         | x+                                                |
+| right         | x+ face                                           |
-| front         | z-                                                |
+| front         | z- face                                           |
-| back          | z+                                                |
+| back          | z+ face                                           |
-| top           | y+                                                |
+| top           | y+ face                                           |
-| bottom        | y-                                                |
+| bottom        | y- face                                           |
-| sides         | x-, x+, z-, z+                                    |
+| sides         | x-, x+, z-, z+ faces                              |
 | all           | All faces. You can also use '*' instead of 'all'. |
+| inventory     | The inventory texture                             |
+| wield         | The texture used when held by the player          |
+
+Nodes support all targets, but other items only support 'inventory'
+and 'wield'
 
 Designing leaves textures for the leaves rendering options
 ----------------------------------------------------------

src/CMakeLists.txt

@@ -423,6 +423,7 @@ set(common_SRCS
 	settings.cpp
 	staticobject.cpp
 	terminal_chat_console.cpp
+	texture_override.cpp
 	tileanimation.cpp
 	tool.cpp
 	translation.cpp

src/client/client.cpp

@@ -1742,8 +1742,11 @@ void Client::afterContentReceived()
 	text = wgettext("Initializing nodes...");
 	RenderingEngine::draw_load_screen(text, guienv, m_tsrc, 0, 72);
 	m_nodedef->updateAliases(m_itemdef);
-	for (const auto &path : getTextureDirs())
+	for (const auto &path : getTextureDirs()) {
-		m_nodedef->applyTextureOverrides(path + DIR_DELIM + "override.txt");
+		TextureOverrideSource override_source(path + DIR_DELIM + "override.txt");
+		m_nodedef->applyTextureOverrides(override_source.getNodeTileOverrides());
+		m_itemdef->applyTextureOverrides(override_source.getItemTextureOverrides());
+	}
 	m_nodedef->setNodeRegistrationStatus(true);
 	m_nodedef->runNodeResolveCallbacks();
 	delete[] text;

src/itemdef.cpp

@@ -422,6 +422,25 @@ public:
 		return get(stack.name).color;
 	}
 #endif
+	void applyTextureOverrides(const std::vector<TextureOverride> &overrides)
+	{
+		infostream << "ItemDefManager::applyTextureOverrides(): Applying "
+			"overrides to textures" << std::endl;
+
+		for (const TextureOverride& texture_override : overrides) {
+			if (m_item_definitions.find(texture_override.id) == m_item_definitions.end()) {
+				continue; // Ignore unknown item
+			}
+
+			ItemDefinition* itemdef = m_item_definitions[texture_override.id];
+
+			if (texture_override.hasTarget(OverrideTarget::INVENTORY))
+				itemdef->inventory_image = texture_override.texture;
+
+			if (texture_override.hasTarget(OverrideTarget::WIELD))
+				itemdef->wield_image = texture_override.texture;
+		}
+	}
 	void clear()
 	{
 		for(std::map<std::string, ItemDefinition*>::const_iterator

src/itemdef.h

@@ -26,6 +26,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <set>
 #include "itemgroup.h"
 #include "sound.h"
+#include "texture_override.h" // TextureOverride
 class IGameDef;
 class Client;
 struct ToolCapabilities;
@@ -157,6 +158,10 @@ public:
 		Client *client) const=0;
 #endif
 
+	// Replace the textures of registered nodes with the ones specified in
+	// the texture pack's override.txt files
+	virtual void applyTextureOverrides(const std::vector<TextureOverride> &overrides)=0;
+
 	// Remove all registered item and node definitions and aliases
 	// Then re-add the builtin item definitions
 	virtual void clear()=0;

src/nodedef.cpp

@@ -1304,60 +1304,35 @@ void NodeDefManager::updateAliases(IItemDefManager *idef)
 	}
 }
 
-void NodeDefManager::applyTextureOverrides(const std::string &override_filepath)
+void NodeDefManager::applyTextureOverrides(const std::vector<TextureOverride> &overrides)
 {
 	infostream << "NodeDefManager::applyTextureOverrides(): Applying "
-		"overrides to textures from " << override_filepath << std::endl;
+		"overrides to textures" << std::endl;
-
-	std::ifstream infile(override_filepath.c_str());
-	std::string line;
-	int line_c = 0;
-	while (std::getline(infile, line)) {
-		line_c++;
-		// Also trim '\r' on DOS-style files
-		line = trim(line);
-		if (line.empty())
-			continue;
-
-		std::vector<std::string> splitted = str_split(line, ' ');
-		if (splitted.size() != 3) {
-			errorstream << override_filepath
-				<< ":" << line_c << " Could not apply texture override \""
-				<< line << "\": Syntax error" << std::endl;
-			continue;
-		}
 
+	for (const TextureOverride& texture_override : overrides) {
 		content_t id;
-		if (!getId(splitted[0], id))
+		if (!getId(texture_override.id, id))
 			continue; // Ignore unknown node
 
 		ContentFeatures &nodedef = m_content_features[id];
 
-		if (splitted[1] == "top")
+		if (texture_override.hasTarget(OverrideTarget::TOP))
-			nodedef.tiledef[0].name = splitted[2];
+			nodedef.tiledef[0].name = texture_override.texture;
-		else if (splitted[1] == "bottom")
+
-			nodedef.tiledef[1].name = splitted[2];
+		if (texture_override.hasTarget(OverrideTarget::BOTTOM))
-		else if (splitted[1] == "right")
+			nodedef.tiledef[1].name = texture_override.texture;
-			nodedef.tiledef[2].name = splitted[2];
+
-		else if (splitted[1] == "left")
+		if (texture_override.hasTarget(OverrideTarget::RIGHT))
-			nodedef.tiledef[3].name = splitted[2];
+			nodedef.tiledef[2].name = texture_override.texture;
-		else if (splitted[1] == "back")
+
-			nodedef.tiledef[4].name = splitted[2];
+		if (texture_override.hasTarget(OverrideTarget::LEFT))
-		else if (splitted[1] == "front")
+			nodedef.tiledef[3].name = texture_override.texture;
-			nodedef.tiledef[5].name = splitted[2];
+
-		else if (splitted[1] == "all" || splitted[1] == "*")
+		if (texture_override.hasTarget(OverrideTarget::BACK))
-			for (TileDef &i : nodedef.tiledef)
+			nodedef.tiledef[4].name = texture_override.texture;
-				i.name = splitted[2];
+
-		else if (splitted[1] == "sides")
+		if (texture_override.hasTarget(OverrideTarget::FRONT))
-			for (int i = 2; i < 6; i++)
+			nodedef.tiledef[5].name = texture_override.texture;
-				nodedef.tiledef[i].name = splitted[2];
-		else {
-			errorstream << override_filepath
-				<< ":" << line_c << " Could not apply texture override \""
-				<< line << "\": Unknown node side \""
-				<< splitted[1] << "\"" << std::endl;
-			continue;
-		}
 	}
 }
 

src/nodedef.h

@@ -33,6 +33,7 @@ class Client;
 #include "itemgroup.h"
 #include "sound.h" // SimpleSoundSpec
 #include "constants.h" // BS
+#include "texture_override.h" // TextureOverride
 #include "tileanimation.h"
 
 // PROTOCOL_VERSION >= 37
@@ -583,15 +584,12 @@ public:
 	void updateAliases(IItemDefManager *idef);
 
 	/*!
-	 * Reads the used texture pack's override.txt, and replaces the textures
+	 * Replaces the textures of registered nodes with the ones specified in
-	 * of registered nodes with the ones specified there.
+	 * the texturepack's override.txt file
 	 *
-	 * Format of the input file: in each line
+	 * @param overrides the texture overrides
-	 * `node_name top|bottom|right|left|front|back|all|*|sides texture_name.png`
-	 *
-	 * @param override_filepath path to 'texturepack/override.txt'
 	 */
-	void applyTextureOverrides(const std::string &override_filepath);
+	void applyTextureOverrides(const std::vector<TextureOverride> &overrides);
 
 	/*!
 	 * Only the client uses this. Loads textures and shaders required for

src/server.cpp

@@ -373,8 +373,11 @@ void Server::init()
 	std::vector<std::string> paths;
 	fs::GetRecursiveDirs(paths, g_settings->get("texture_path"));
 	fs::GetRecursiveDirs(paths, m_gamespec.path + DIR_DELIM + "textures");
-	for (const std::string &path : paths)
+	for (const std::string &path : paths) {
-		m_nodedef->applyTextureOverrides(path + DIR_DELIM + "override.txt");
+		TextureOverrideSource override_source(path + DIR_DELIM + "override.txt");
+		m_nodedef->applyTextureOverrides(override_source.getNodeTileOverrides());
+		m_itemdef->applyTextureOverrides(override_source.getItemTextureOverrides());
+	}
 
 	m_nodedef->setNodeRegistrationStatus(true);
 

src/texture_override.cpp

@@ -0,0 +1,120 @@
+/*
+Minetest
+Copyright (C) 2020 Hugues Ross <hugues.ross@gmail.com>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#include "texture_override.h"
+
+#include "log.h"
+#include "util/string.h"
+#include <algorithm>
+#include <fstream>
+
+TextureOverrideSource::TextureOverrideSource(std::string filepath)
+{
+	std::ifstream infile(filepath.c_str());
+	std::string line;
+	int line_index = 0;
+	while (std::getline(infile, line)) {
+		line_index++;
+
+		// Also trim '\r' on DOS-style files
+		line = trim(line);
+
+		// Ignore empty lines and comments
+		if (line.empty() || line[0] == '#')
+			continue;
+
+		std::vector<std::string> splitted = str_split(line, ' ');
+		if (splitted.size() != 3) {
+			warningstream << filepath << ":" << line_index
+					<< " Syntax error in texture override \"" << line
+					<< "\": Expected 3 arguments, got " << splitted.size()
+					<< std::endl;
+			continue;
+		}
+
+		TextureOverride texture_override = {};
+		texture_override.id = splitted[0];
+		texture_override.texture = splitted[2];
+
+		// Parse the target mask
+		std::vector<std::string> targets = str_split(splitted[1], ',');
+		for (const std::string &target : targets) {
+			if (target == "top")
+				texture_override.target |= static_cast<u8>(OverrideTarget::TOP);
+			else if (target == "bottom")
+				texture_override.target |= static_cast<u8>(OverrideTarget::BOTTOM);
+			else if (target == "left")
+				texture_override.target |= static_cast<u8>(OverrideTarget::LEFT);
+			else if (target == "right")
+				texture_override.target |= static_cast<u8>(OverrideTarget::RIGHT);
+			else if (target == "front")
+				texture_override.target |= static_cast<u8>(OverrideTarget::FRONT);
+			else if (target == "back")
+				texture_override.target |= static_cast<u8>(OverrideTarget::BACK);
+			else if (target == "inventory")
+				texture_override.target |= static_cast<u8>(OverrideTarget::INVENTORY);
+			else if (target == "wield")
+				texture_override.target |= static_cast<u8>(OverrideTarget::WIELD);
+			else if (target == "sides")
+				texture_override.target |= static_cast<u8>(OverrideTarget::SIDES);
+			else if (target == "all" || target == "*")
+				texture_override.target |= static_cast<u8>(OverrideTarget::ALL_FACES);
+			else {
+				// Report invalid target
+				warningstream << filepath << ":" << line_index
+						<< " Syntax error in texture override \"" << line
+						<< "\": Unknown target \"" << target << "\""
+						<< std::endl;
+			}
+		}
+
+		// If there are no valid targets, skip adding this override
+		if (texture_override.target == static_cast<u8>(OverrideTarget::INVALID)) {
+			continue;
+		}
+
+		m_overrides.push_back(texture_override);
+	}
+}
+
+//! Get all overrides that apply to item definitions
+std::vector<TextureOverride> TextureOverrideSource::getItemTextureOverrides()
+{
+	std::vector<TextureOverride> found_overrides;
+
+	for (const TextureOverride &texture_override : m_overrides) {
+		if (texture_override.hasTarget(OverrideTarget::ITEM_TARGETS))
+			found_overrides.push_back(texture_override);
+	}
+
+	return found_overrides;
+}
+
+//! Get all overrides that apply to node definitions
+std::vector<TextureOverride> TextureOverrideSource::getNodeTileOverrides()
+{
+	std::vector<TextureOverride> found_overrides;
+
+	for (const TextureOverride &texture_override : m_overrides) {
+		if (texture_override.hasTarget(OverrideTarget::ALL_FACES))
+			found_overrides.push_back(texture_override);
+	}
+
+	return found_overrides;
+}

src/texture_override.h

@@ -0,0 +1,72 @@
+/*
+Minetest
+Copyright (C) 2020 Hugues Ross <hugues.ross@gmail.com>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#pragma once
+
+#include "irrlichttypes.h"
+#include <string>
+#include <vector>
+
+//! Bitmask enum specifying what a texture override should apply to
+enum class OverrideTarget : u8
+{
+	INVALID = 0,
+	TOP = 1 << 0,
+	BOTTOM = 1 << 1,
+	LEFT = 1 << 2,
+	RIGHT = 1 << 3,
+	FRONT = 1 << 4,
+	BACK = 1 << 5,
+	INVENTORY = 1 << 6,
+	WIELD = 1 << 7,
+
+	SIDES = LEFT | RIGHT | FRONT | BACK,
+	ALL_FACES = TOP | BOTTOM | SIDES,
+	ITEM_TARGETS = INVENTORY | WIELD,
+};
+
+struct TextureOverride
+{
+	std::string id;
+	std::string texture;
+	u8 target;
+
+	// Helper function for checking if an OverrideTarget is found in
+	// a TextureOverride without casting
+	inline bool hasTarget(OverrideTarget overrideTarget) const
+	{
+		return (target & static_cast<u8>(overrideTarget)) != 0;
+	}
+};
+
+//! Class that provides texture override information from a texture pack
+class TextureOverrideSource
+{
+public:
+	TextureOverrideSource(std::string filepath);
+
+	//! Get all overrides that apply to item definitions
+	std::vector<TextureOverride> getItemTextureOverrides();
+
+	//! Get all overrides that apply to node definitions
+	std::vector<TextureOverride> getNodeTileOverrides();
+
+private:
+	std::vector<TextureOverride> m_overrides;
+};

util/travis/clang-format-whitelist.txt

@@ -423,6 +423,7 @@ src/subgame.cpp
 src/subgame.h
 src/terminal_chat_console.cpp
 src/terminal_chat_console.h
+src/texture_override.cpp
 src/threading/atomic.h
 src/threading/event.cpp
 src/threading/mutex_auto_lock.h