Represent cItem::m_Lore as an AStringVector (#3882)

* Replace cItem::m_Lore with AStringVector

* Reword deprecation warning

* Fix lua bindings

Server/Plugins/APIDump/APIDesc.lua

@@ -6693,7 +6693,7 @@ These ItemGrids are available in the API and can be manipulated by the plugins,
 							},
 							{
 								Name = "Lore",
-								Type = "string",
+								Type = "table",
 								IsOptional = true,
 							},
 						},
@@ -6947,10 +6947,10 @@ These ItemGrids are available in the API and can be manipulated by the plugins,
 					Type = "number",
 					Notes = "The item type. One of E_ITEM_ or E_BLOCK_ constants",
 				},
-				m_Lore =
+				m_LoreTable =
 				{
-					Type = "string",
+					Type = "table",
-					Notes = "The lore for an item. Line breaks are represented by the ` character.",
+					Notes = "The lore for an item. Represented as an array table of lines.",
 				},
 				m_RepairCost =
 				{

src/Bindings/DeprecatedBindings.cpp

@@ -347,6 +347,58 @@ static int tolua_set_cBlockInfo_m_PlaceSound(lua_State * tolua_S)
 
 
 
+static int tolua_get_cItem_m_Lore(lua_State * tolua_S)
+{
+	// Maintain legacy m_Lore variable as Lore table split by ` (grave-accent)
+	cLuaState L(tolua_S);
+	if (!L.CheckParamSelf("const cItem"))
+	{
+		return 0;
+	}
+
+	const cItem * Self = nullptr;
+	L.GetStackValue(1, Self);
+
+	AString LoreString = StringJoin(Self->m_LoreTable, "`");
+
+	L.Push(LoreString);
+
+	LOGWARNING("cItem.m_Lore is deprecated, use cItem.m_LoreTable instead");
+	L.LogStackTrace(0);
+	return 1;
+}
+
+
+
+
+
+static int tolua_set_cItem_m_Lore(lua_State * tolua_S)
+{
+	// Maintain legacy m_Lore variable as Lore table split by ` (grave-accent)
+	cLuaState L(tolua_S);
+	if (
+		!L.CheckParamSelf("cItem") ||
+		!L.CheckParamString(2)
+		)
+	{
+		return 0;
+	}
+
+	cItem * Self = nullptr;
+	AString LoreString;
+	L.GetStackValues(1, Self, LoreString);
+
+	Self->m_LoreTable = StringSplit(LoreString, "`");
+
+	LOGWARNING("cItem.m_Lore is deprecated, use cItem.m_LoreTable instead");
+	L.LogStackTrace(0);
+	return 0;
+}
+
+
+
+
+
 /* method: Trace of class  cTracer */
 static int tolua_cTracer_Trace(lua_State * a_LuaState)
 {
@@ -500,6 +552,10 @@ void DeprecatedBindings::Bind(lua_State * tolua_S)
 		tolua_variable(tolua_S, "m_PlaceSound", tolua_get_cBlockInfo_m_PlaceSound, tolua_set_cBlockInfo_m_PlaceSound);
 	tolua_endmodule(tolua_S);
 
+	tolua_beginmodule(tolua_S, "cItem");
+		tolua_variable(tolua_S, "m_Lore", tolua_get_cItem_m_Lore, tolua_set_cItem_m_Lore);
+	tolua_endmodule(tolua_S);
+
 	tolua_beginmodule(tolua_S, "cTracer");
 		tolua_function(tolua_S, "Trace", tolua_cTracer_Trace);
 	tolua_endmodule(tolua_S);

src/Bindings/LuaState.cpp

@@ -1151,6 +1151,37 @@ bool cLuaState::GetStackValue(int a_StackPos, AStringMap & a_Value)
 
 
 
+bool cLuaState::GetStackValue(int a_StackPos, AStringVector & a_Value)
+{
+	// Retrieve all values in an array of string table:
+	if (!lua_istable(m_LuaState, a_StackPos))
+	{
+		return false;
+	}
+	cStackTable tbl(*this, a_StackPos);
+	bool isValid = true;
+	tbl.ForEachArrayElement([&](cLuaState & a_LuaState, int a_Index)
+		{
+			AString tempStr;
+			if (a_LuaState.GetStackValue(-1, tempStr))
+			{
+				a_Value.push_back(std::move(tempStr));
+			}
+			else
+			{
+				isValid = false;
+				return true;
+			}
+			return false;
+		}
+	);
+	return isValid;
+}
+
+
+
+
+
 bool cLuaState::GetStackValue(int a_StackPos, bool & a_ReturnedVal)
 {
 	a_ReturnedVal = (tolua_toboolean(m_LuaState, a_StackPos, a_ReturnedVal ? 1 : 0) > 0);

src/Bindings/LuaState.h

@@ -640,6 +640,7 @@ public:
 	// Enum values are checked for their allowed values and fail if the value is not assigned.
 	bool GetStackValue(int a_StackPos, AString & a_Value);
 	bool GetStackValue(int a_StackPos, AStringMap & a_Value);
+	bool GetStackValue(int a_StackPos, AStringVector & a_Value);
 	bool GetStackValue(int a_StackPos, bool & a_Value);
 	bool GetStackValue(int a_StackPos, cCallback & a_Callback);
 	bool GetStackValue(int a_StackPos, cCallbackPtr & a_Callback);

src/Bindings/ManualBindings.cpp

@@ -11,11 +11,6 @@
 #include "PluginLua.h"
 #include "PluginManager.h"
 #include "LuaWindow.h"
-#include "../Root.h"
-#include "../World.h"
-#include "../Entities/Player.h"
-#include "../WebAdmin.h"
-#include "../ClientHandle.h"
 #include "../BlockArea.h"
 #include "../BlockEntities/BeaconEntity.h"
 #include "../BlockEntities/BrewingstandEntity.h"
@@ -28,14 +23,20 @@
 #include "../BlockEntities/NoteEntity.h"
 #include "../BlockEntities/MobHeadEntity.h"
 #include "../BlockEntities/FlowerPotEntity.h"
-#include "../Generating/ChunkDesc.h"
-#include "../LineBlockTracer.h"
-#include "../CompositeChat.h"
-#include "../StringCompression.h"
-#include "../CommandOutput.h"
-#include "../BuildInfo.h"
-#include "../HTTP/UrlParser.h"
 #include "../BoundingBox.h"
+#include "../BuildInfo.h"
+#include "../ClientHandle.h"
+#include "../CommandOutput.h"
+#include "../CompositeChat.h"
+#include "../Entities/Player.h"
+#include "../Generating/ChunkDesc.h"
+#include "../HTTP/UrlParser.h"
+#include "../Item.h"
+#include "../LineBlockTracer.h"
+#include "../Root.h"
+#include "../StringCompression.h"
+#include "../WebAdmin.h"
+#include "../World.h"
 
 
 
@@ -2557,6 +2558,57 @@ static int tolua_cMojangAPI_MakeUUIDShort(lua_State * L)
 
 
 
+static int tolua_get_cItem_m_LoreTable(lua_State * tolua_S)
+{
+	// Check params:
+	cLuaState L(tolua_S);
+	if (!L.CheckParamSelf("const cItem"))
+	{
+		return 0;
+	}
+
+	// Get the params:
+	const cItem * Self = nullptr;
+	L.GetStackValue(1, Self);
+
+	// Push the result:
+	L.Push(Self->m_LoreTable);
+	return 1;
+}
+
+
+
+
+
+static int tolua_set_cItem_m_LoreTable(lua_State * tolua_S)
+{
+	// Check params:
+	cLuaState L(tolua_S);
+	if (
+		!L.CheckParamSelf("cItem") ||
+		!L.CheckParamTable(2)
+	)
+	{
+		return 0;
+	}
+
+	// Get the params:
+	cItem * Self = nullptr;
+	L.GetStackValue(1, Self);
+
+	// Set the value:
+	Self->m_LoreTable.clear();
+	if (!L.GetStackValue(2, Self->m_LoreTable))
+	{
+		return L.ApiParamError("cItem.m_LoreTable: Could not read value as an array of strings");
+	}
+	return 0;
+}
+
+
+
+
+
 static int Lua_ItemGrid_GetSlotCoords(lua_State * L)
 {
 	tolua_Error tolua_err;
@@ -3798,6 +3850,10 @@ void cManualBindings::Bind(lua_State * tolua_S)
 			tolua_function(tolua_S, "GetOutputBlockPos", tolua_cHopperEntity_GetOutputBlockPos);
 		tolua_endmodule(tolua_S);
 
+		tolua_beginmodule(tolua_S, "cItem");
+			tolua_variable(tolua_S, "m_LoreTable", tolua_get_cItem_m_LoreTable, tolua_set_cItem_m_LoreTable);
+		tolua_endmodule(tolua_S);
+
 		tolua_beginmodule(tolua_S, "cItemGrid");
 			tolua_function(tolua_S, "GetSlotCoords", Lua_ItemGrid_GetSlotCoords);
 		tolua_endmodule(tolua_S);

src/Item.cpp

@@ -158,7 +158,12 @@ void cItem::GetJson(Json::Value & a_OutValue) const
 		}
 		if (!IsLoreEmpty())
 		{
-			a_OutValue["Lore"] = m_Lore;
+			auto & LoreArray = (a_OutValue["Lore"] = Json::Value(Json::arrayValue));
+
+			for (const auto & Line : m_LoreTable)
+			{
+				LoreArray.append(Line);
+			}
 		}
 
 		if (m_ItemColor.IsValid())
@@ -196,7 +201,11 @@ void cItem::FromJson(const Json::Value & a_Value)
 		m_Enchantments.Clear();
 		m_Enchantments.AddFromString(a_Value.get("ench", "").asString());
 		m_CustomName = a_Value.get("Name", "").asString();
-		m_Lore = a_Value.get("Lore", "").asString();
+		auto Lore = a_Value.get("Lore", Json::arrayValue);
+		for (auto & Line : Lore)
+		{
+			m_LoreTable.push_back(Line.asString());
+		}
 
 		int red = a_Value.get("Color_Red", -1).asInt();
 		int green = a_Value.get("Color_Green", -1).asInt();

src/Item.h

@@ -41,7 +41,6 @@ public:
 		m_ItemCount(0),
 		m_ItemDamage(0),
 		m_CustomName(""),
-		m_Lore(""),
 		m_RepairCost(0),
 		m_FireworkItem(),
 		m_ItemColor()
@@ -56,14 +55,14 @@ public:
 		short a_ItemDamage = 0,
 		const AString & a_Enchantments = "",
 		const AString & a_CustomName = "",
-		const AString & a_Lore = ""
+		const AStringVector & a_LoreTable = {}
 	) :
 		m_ItemType    (a_ItemType),
 		m_ItemCount   (a_ItemCount),
 		m_ItemDamage  (a_ItemDamage),
 		m_Enchantments(a_Enchantments),
 		m_CustomName  (a_CustomName),
-		m_Lore        (a_Lore),
+		m_LoreTable   (a_LoreTable),
 		m_RepairCost  (0),
 		m_FireworkItem(),
 		m_ItemColor()
@@ -106,7 +105,7 @@ public:
 		m_ItemDamage = 0;
 		m_Enchantments.Clear();
 		m_CustomName = "";
-		m_Lore = "";
+		m_LoreTable.clear();
 		m_RepairCost = 0;
 		m_FireworkItem.EmptyData();
 		m_ItemColor.Clear();
@@ -137,7 +136,7 @@ public:
 			(m_ItemDamage == a_Item.m_ItemDamage) &&
 			(m_Enchantments == a_Item.m_Enchantments) &&
 			(m_CustomName == a_Item.m_CustomName) &&
-			(m_Lore == a_Item.m_Lore) &&
+			(m_LoreTable == a_Item.m_LoreTable) &&
 			m_FireworkItem.IsEqualTo(a_Item.m_FireworkItem)
 		);
 	}
@@ -151,12 +150,12 @@ public:
 
 	bool IsBothNameAndLoreEmpty(void) const
 	{
-		return (m_CustomName.empty() && m_Lore.empty());
+		return (m_CustomName.empty() && m_LoreTable.empty());
 	}
 
 
 	bool IsCustomNameEmpty(void) const { return (m_CustomName.empty()); }
-	bool IsLoreEmpty(void) const { return (m_Lore.empty()); }
+	bool IsLoreEmpty(void) const { return (m_LoreTable.empty()); }
 
 	/** Returns a copy of this item with m_ItemCount set to 1. Useful to preserve enchantments etc. on stacked items */
 	cItem CopyOne(void) const;
@@ -221,7 +220,12 @@ public:
 	short          m_ItemDamage;
 	cEnchantments  m_Enchantments;
 	AString        m_CustomName;
-	AString        m_Lore;
+
+	// tolua_end
+
+	AStringVector  m_LoreTable;  // Exported in ManualBindings.cpp
+
+	// tolua_begin
 
 	int            m_RepairCost;
 	cFireworkItem  m_FireworkItem;

src/Protocol/Protocol_1_8.cpp

@@ -2883,14 +2883,10 @@ void cProtocol_1_8_0::ParseItemMetadata(cItem & a_Item, const AString & a_Metada
 						}
 						else if ((NBT.GetType(displaytag) == TAG_List) && (NBT.GetName(displaytag) == "Lore"))  // Lore tag
 						{
-							AString Lore;
-
 							for (int loretag = NBT.GetFirstChild(displaytag); loretag >= 0; loretag = NBT.GetNextSibling(loretag))  // Loop through array of strings
 							{
-								AppendPrintf(Lore, "%s`", NBT.GetString(loretag).c_str());  // Append the lore with a grave accent / backtick, used internally by MCS to display a new line in the client; don't forget to c_str ;)
+								a_Item.m_LoreTable.push_back(NBT.GetString(loretag));
 							}
-
-							a_Item.m_Lore = Lore;
 						}
 						else if ((NBT.GetType(displaytag) == TAG_Int) && (NBT.GetName(displaytag) == "color"))
 						{
@@ -3079,15 +3075,9 @@ void cProtocol_1_8_0::WriteItem(cPacketizer & a_Pkt, const cItem & a_Item)
 		{
 			Writer.BeginList("Lore", TAG_String);
 
-			AStringVector Decls = StringSplit(a_Item.m_Lore, "`");
+			for (const auto & Line : a_Item.m_LoreTable)
-			for (AStringVector::const_iterator itr = Decls.begin(), end = Decls.end(); itr != end; ++itr)
 			{
-				if (itr->empty())
+				Writer.AddString("", Line);
-				{
-					// The decl is empty (two `s), ignore
-					continue;
-				}
-				Writer.AddString("", itr->c_str());
 			}
 
 			Writer.EndList();

src/Protocol/Protocol_1_9.cpp

@@ -2999,14 +2999,11 @@ void cProtocol_1_9_0::ParseItemMetadata(cItem & a_Item, const AString & a_Metada
 						}
 						else if ((NBT.GetType(displaytag) == TAG_List) && (NBT.GetName(displaytag) == "Lore"))  // Lore tag
 						{
-							AString Lore;
+							a_Item.m_LoreTable.clear();
-
 							for (int loretag = NBT.GetFirstChild(displaytag); loretag >= 0; loretag = NBT.GetNextSibling(loretag))  // Loop through array of strings
 							{
-								AppendPrintf(Lore, "%s`", NBT.GetString(loretag).c_str());  // Append the lore with a grave accent / backtick, used internally by MCS to display a new line in the client; don't forget to c_str ;)
+								a_Item.m_LoreTable.push_back(NBT.GetString(loretag));
 							}
-
-							a_Item.m_Lore = Lore;
 						}
 						else if ((NBT.GetType(displaytag) == TAG_Int) && (NBT.GetName(displaytag) == "color"))
 						{
@@ -3342,15 +3339,9 @@ void cProtocol_1_9_0::WriteItem(cPacketizer & a_Pkt, const cItem & a_Item)
 		{
 			Writer.BeginList("Lore", TAG_String);
 
-			AStringVector Decls = StringSplit(a_Item.m_Lore, "`");
+			for (const auto & Line : a_Item.m_LoreTable)
-			for (AStringVector::const_iterator itr = Decls.begin(), end = Decls.end(); itr != end; ++itr)
 			{
-				if (itr->empty())
+				Writer.AddString("", Line);
-				{
-					// The decl is empty (two `s), ignore
-					continue;
-				}
-				Writer.AddString("", itr->c_str());
 			}
 
 			Writer.EndList();

src/StringUtils.cpp

@@ -228,6 +228,42 @@ AStringVector StringSplitWithQuotes(const AString & str, const AString & delim)
 
 
 
+
+AString StringJoin(const AStringVector & a_Strings, const AString & a_Delimeter)
+{
+	if (a_Strings.empty())
+	{
+		return {};
+	}
+
+	// Do a dry run to gather the size
+	const auto DelimSize = a_Delimeter.size();
+	size_t ResultSize = a_Strings[0].size();
+	std::for_each(a_Strings.begin() + 1, a_Strings.end(),
+		[&](const AString & a_String)
+		{
+			ResultSize += DelimSize;
+			ResultSize += a_String.size();
+		}
+	);
+
+	// Now do the actual join
+	AString Result;
+	Result.reserve(ResultSize);
+	Result.append(a_Strings[0]);
+	std::for_each(a_Strings.begin() + 1, a_Strings.end(),
+		[&](const AString & a_String)
+		{
+			Result += a_Delimeter;
+			Result += a_String;
+		}
+	);
+	return Result;
+}
+
+
+
+
 AStringVector StringSplitAndTrim(const AString & str, const AString & delim)
 {
 	AStringVector results;

src/StringUtils.h

@@ -47,6 +47,9 @@ Resolves issue #490
 Return the splitted strings as a stringvector. */
 extern AStringVector StringSplitWithQuotes(const AString & str, const AString & delim);
 
+/** Join a list of strings with the given delimiter between entries. */
+AString StringJoin(const AStringVector & a_Strings, const AString & a_Delimiter);
+
 /** Split the string at any of the listed delimiters and trim each value.
 Returns the splitted strings as a stringvector. */
 extern AStringVector StringSplitAndTrim(const AString & str, const AString & delim);

src/WorldStorage/NBTChunkSerializer.cpp

@@ -107,7 +107,7 @@ void cNBTChunkSerializer::AddItem(const cItem & a_Item, int a_Slot, const AStrin
 		((a_Item.m_ItemType == E_ITEM_FIREWORK_ROCKET) || (a_Item.m_ItemType == E_ITEM_FIREWORK_STAR)) ||
 		(a_Item.m_RepairCost > 0) ||
 		(a_Item.m_CustomName != "") ||
-		(a_Item.m_Lore != "")
+		(!a_Item.m_LoreTable.empty())
 	)
 	{
 		m_Writer.BeginCompound("tag");
@@ -116,16 +116,23 @@ void cNBTChunkSerializer::AddItem(const cItem & a_Item, int a_Slot, const AStrin
 				m_Writer.AddInt("RepairCost", a_Item.m_RepairCost);
 			}
 
-			if ((a_Item.m_CustomName != "") || (a_Item.m_Lore != ""))
+			if ((a_Item.m_CustomName != "") || (!a_Item.m_LoreTable.empty()))
 			{
 				m_Writer.BeginCompound("display");
 				if (a_Item.m_CustomName != "")
 				{
 					m_Writer.AddString("Name", a_Item.m_CustomName);
 				}
-				if (a_Item.m_Lore != "")
+				if (!a_Item.m_LoreTable.empty())
 				{
-					m_Writer.AddString("Lore", a_Item.m_Lore);
+					m_Writer.BeginList("Lore", TAG_String);
+
+					for (const auto & Line : a_Item.m_LoreTable)
+					{
+						m_Writer.AddString("", Line);
+					}
+
+					m_Writer.EndList();
 				}
 				m_Writer.EndCompound();
 			}

src/WorldStorage/WSSAnvil.cpp

@@ -804,7 +804,17 @@ bool cWSSAnvil::LoadItemFromNBT(cItem & a_Item, const cParsedNBT & a_NBT, int a_
 		int Lore = a_NBT.FindChildByName(DisplayTag, "Lore");
 		if ((Lore > 0) && (a_NBT.GetType(Lore) == TAG_String))
 		{
-			a_Item.m_Lore = a_NBT.GetString(Lore);
+			// Legacy string lore
+			a_Item.m_LoreTable = StringSplit(a_NBT.GetString(Lore), "`");
+		}
+		else if ((Lore > 0) && (a_NBT.GetType(Lore) == TAG_List))
+		{
+			// Lore table
+			a_Item.m_LoreTable.clear();
+			for (int loretag = a_NBT.GetFirstChild(Lore); loretag >= 0; loretag = a_NBT.GetNextSibling(loretag))  // Loop through array of strings
+			{
+				a_Item.m_LoreTable.push_back(a_NBT.GetString(loretag));
+			}
 		}
 	}