From 14a00d0051d9c0f97105145bd2ecf7679bb015a3 Mon Sep 17 00:00:00 2001 From: 12xx12 <44411062+12xx12@users.noreply.github.com> Date: Wed, 11 Nov 2020 22:38:51 +0100 Subject: [PATCH] changing cComposite Chat to newer c++ standart (#5028) * upgraded to new C++ for loops and fixed errors * readded delete instruction * now using unique ptr * added test for text only (that was causing an error for me) * using unique ptr constructor * added move constructor and deleted copy constructor * fixed deconstuctor http prefixes are constexpr and std::string_view * fixed whitespace Co-authored-by: 12xx12 <12xx12100@gmail.com> --- src/CompositeChat.cpp | 117 +++++++++++----------- src/CompositeChat.h | 11 +- tests/CompositeChat/CompositeChatTest.cpp | 14 +++ 3 files changed, 79 insertions(+), 63 deletions(-) diff --git a/src/CompositeChat.cpp b/src/CompositeChat.cpp index 08e1973ce..7def15550 100644 --- a/src/CompositeChat.cpp +++ b/src/CompositeChat.cpp @@ -45,10 +45,6 @@ cCompositeChat::~cCompositeChat() void cCompositeChat::Clear(void) { - for (cParts::iterator itr = m_Parts.begin(), end = m_Parts.end(); itr != end; ++itr) - { - delete *itr; - } // for itr - m_Parts[] m_Parts.clear(); } @@ -58,7 +54,7 @@ void cCompositeChat::Clear(void) void cCompositeChat::AddTextPart(const AString & a_Message, const AString & a_Style) { - m_Parts.push_back(new cTextPart(a_Message, a_Style)); + m_Parts.emplace_back(std::make_unique(a_Message, a_Style)); } @@ -67,7 +63,7 @@ void cCompositeChat::AddTextPart(const AString & a_Message, const AString & a_St void cCompositeChat::AddClientTranslatedPart(const AString & a_TranslationID, const AStringVector & a_Parameters, const AString & a_Style) { - m_Parts.push_back(new cClientTranslatedPart(a_TranslationID, a_Parameters, a_Style)); + m_Parts.emplace_back(std::make_unique(a_TranslationID, a_Parameters, a_Style)); } @@ -76,7 +72,7 @@ void cCompositeChat::AddClientTranslatedPart(const AString & a_TranslationID, co void cCompositeChat::AddUrlPart(const AString & a_Text, const AString & a_Url, const AString & a_Style) { - m_Parts.push_back(new cUrlPart(a_Text, a_Url, a_Style)); + m_Parts.emplace_back(std::make_unique(a_Text, a_Url, a_Style)); } @@ -85,7 +81,7 @@ void cCompositeChat::AddUrlPart(const AString & a_Text, const AString & a_Url, c void cCompositeChat::AddRunCommandPart(const AString & a_Text, const AString & a_Command, const AString & a_Style) { - m_Parts.push_back(new cRunCommandPart(a_Text, a_Command, a_Style)); + m_Parts.emplace_back(std::make_unique(a_Text, a_Command, a_Style)); } @@ -94,7 +90,7 @@ void cCompositeChat::AddRunCommandPart(const AString & a_Text, const AString & a void cCompositeChat::AddSuggestCommandPart(const AString & a_Text, const AString & a_SuggestedCommand, const AString & a_Style) { - m_Parts.push_back(new cSuggestCommandPart(a_Text, a_SuggestedCommand, a_Style)); + m_Parts.emplace_back(std::make_unique(a_Text, a_SuggestedCommand, a_Style)); } @@ -103,7 +99,7 @@ void cCompositeChat::AddSuggestCommandPart(const AString & a_Text, const AString void cCompositeChat::AddShowAchievementPart(const AString & a_PlayerName, const AString & a_Achievement, const AString & a_Style) { - m_Parts.push_back(new cShowAchievementPart(a_PlayerName, a_Achievement, a_Style)); + m_Parts.emplace_back(std::make_unique(a_PlayerName, a_Achievement, a_Style)); } @@ -149,7 +145,7 @@ void cCompositeChat::ParseText(const AString & a_ParseText) } if (!CurrentText.empty()) { - m_Parts.push_back(new cTextPart(CurrentText, CurrentStyle)); + m_Parts.emplace_back(std::make_unique(CurrentText, CurrentStyle)); CurrentText.clear(); } AddStyle(CurrentStyle, a_ParseText.substr(i - 1, 2)); @@ -159,17 +155,19 @@ void cCompositeChat::ParseText(const AString & a_ParseText) case ':': { - const char * LinkPrefixes[] = + static const constexpr std::array LinkPrefixes = { - "http", - "https" + { + "http", + "https" + } }; - for (size_t Prefix = 0; Prefix < ARRAYCOUNT(LinkPrefixes); Prefix++) + for (const auto & Prefix : LinkPrefixes) { - size_t PrefixLen = strlen(LinkPrefixes[Prefix]); + size_t PrefixLen = Prefix.size(); if ( (i >= first + PrefixLen) && // There is enough space in front of the colon for the prefix - (strncmp(a_ParseText.c_str() + i - PrefixLen, LinkPrefixes[Prefix], PrefixLen) == 0) // the prefix matches + (std::string_view(a_ParseText).substr(i - PrefixLen, PrefixLen) == Prefix) // the prefix matches ) { // Add everything before this as a text part: @@ -223,13 +221,13 @@ void cCompositeChat::SetMessageType(eMessageType a_MessageType, const AString & void cCompositeChat::UnderlineUrls(void) { - for (cParts::iterator itr = m_Parts.begin(), end = m_Parts.end(); itr != end; ++itr) + for (auto & Part : m_Parts) { - if ((*itr)->m_PartType == ptUrl) + if (Part->m_PartType == ptUrl) { - (*itr)->m_Style.append("u"); + Part->m_Style.append("u"); } - } // for itr - m_Parts[] + } } @@ -239,21 +237,21 @@ void cCompositeChat::UnderlineUrls(void) AString cCompositeChat::ExtractText(void) const { AString Msg; - for (cParts::const_iterator itr = m_Parts.begin(), end = m_Parts.end(); itr != end; ++itr) + for (const auto & Part : m_Parts) { - switch ((*itr)->m_PartType) + switch (Part->m_PartType) { case ptText: case ptClientTranslated: case ptRunCommand: case ptSuggestCommand: { - Msg.append((*itr)->m_Text); + Msg.append(Part->m_Text); break; } case ptUrl: { - Msg.append((static_cast(*itr))->m_Url); + Msg.append(static_cast(Part.get())->m_Url); break; } case ptShowAchievement: @@ -318,98 +316,97 @@ void cCompositeChat::AddStyle(AString & a_Style, const AString & a_AddStyle) AString cCompositeChat::CreateJsonString(bool a_ShouldUseChatPrefixes) const { - Json::Value msg; - msg["text"] = cClientHandle::FormatMessageType(a_ShouldUseChatPrefixes, GetMessageType(), GetAdditionalMessageTypeData()); // The client crashes without this field being present - const cCompositeChat::cParts & Parts = GetParts(); - for (cCompositeChat::cParts::const_iterator itr = Parts.begin(), end = Parts.end(); itr != end; ++itr) + Json::Value Message; + Message["text"] = cClientHandle::FormatMessageType(a_ShouldUseChatPrefixes, GetMessageType(), GetAdditionalMessageTypeData()); // The client crashes without this field being present + for (const auto & Part : m_Parts) { - Json::Value Part; - switch ((*itr)->m_PartType) + Json::Value JsonPart; + switch (Part->m_PartType) { case cCompositeChat::ptText: { - Part["text"] = (*itr)->m_Text; - AddChatPartStyle(Part, (*itr)->m_Style); + JsonPart["text"] = Part->m_Text; + AddChatPartStyle(JsonPart, Part->m_Style); break; } case cCompositeChat::ptClientTranslated: { - const cCompositeChat::cClientTranslatedPart & p = static_cast(**itr); - Part["translate"] = p.m_Text; + const auto TranslatedPart = static_cast(Part.get()); + JsonPart["translate"] = TranslatedPart->m_Text; Json::Value With; - for (AStringVector::const_iterator itrW = p.m_Parameters.begin(), endW = p.m_Parameters.end(); itrW != endW; ++itr) + for (const auto & Parameter : TranslatedPart->m_Parameters) { - With.append(*itrW); + With.append(Parameter); } - if (!p.m_Parameters.empty()) + if (!TranslatedPart->m_Parameters.empty()) { - Part["with"] = With; + JsonPart["with"] = With; } - AddChatPartStyle(Part, p.m_Style); + AddChatPartStyle(JsonPart, TranslatedPart->m_Style); break; } case cCompositeChat::ptUrl: { - const cCompositeChat::cUrlPart & p = static_cast(**itr); - Part["text"] = p.m_Text; + const auto UrlPart = static_cast(Part.get()); + JsonPart["text"] = UrlPart->m_Text; Json::Value Url; Url["action"] = "open_url"; - Url["value"] = p.m_Url; - Part["clickEvent"] = Url; - AddChatPartStyle(Part, p.m_Style); + Url["value"] = UrlPart->m_Url; + JsonPart["clickEvent"] = Url; + AddChatPartStyle(JsonPart, UrlPart->m_Style); break; } case cCompositeChat::ptSuggestCommand: case cCompositeChat::ptRunCommand: { - const cCompositeChat::cCommandPart & p = static_cast(**itr); - Part["text"] = p.m_Text; + const auto CommandPart = static_cast(Part.get()); + JsonPart["text"] = CommandPart->m_Text; Json::Value Cmd; - Cmd["action"] = (p.m_PartType == cCompositeChat::ptRunCommand) ? "run_command" : "suggest_command"; - Cmd["value"] = p.m_Command; - Part["clickEvent"] = Cmd; - AddChatPartStyle(Part, p.m_Style); + Cmd["action"] = (CommandPart->m_PartType == cCompositeChat::ptRunCommand) ? "run_command" : "suggest_command"; + Cmd["value"] = CommandPart->m_Command; + JsonPart["clickEvent"] = Cmd; + AddChatPartStyle(JsonPart, CommandPart->m_Style); break; } case cCompositeChat::ptShowAchievement: { - const cCompositeChat::cShowAchievementPart & p = static_cast(**itr); - Part["translate"] = "chat.type.achievement"; + const auto AchievementPart = static_cast(Part.get()); + JsonPart["translate"] = "chat.type.achievement"; Json::Value Ach; Ach["action"] = "show_achievement"; - Ach["value"] = p.m_Text; + Ach["value"] = AchievementPart->m_Text; Json::Value AchColourAndName; AchColourAndName["color"] = "green"; - AchColourAndName["translate"] = p.m_Text; + AchColourAndName["translate"] = AchievementPart->m_Text; AchColourAndName["hoverEvent"] = Ach; Json::Value Extra; Extra.append(AchColourAndName); Json::Value Name; - Name["text"] = p.m_PlayerName; + Name["text"] = AchievementPart->m_PlayerName; Json::Value With; With.append(Name); With.append(Extra); - Part["with"] = With; - AddChatPartStyle(Part, p.m_Style); + JsonPart["with"] = With; + AddChatPartStyle(JsonPart, AchievementPart->m_Style); break; } } - msg["extra"].append(Part); + Message["extra"].append(JsonPart); } // for itr - Parts[] #if 1 // Serialize as machine-readable string (no whitespace): - return JsonUtils::WriteFastString(msg); + return JsonUtils::WriteFastString(Message); #else // Serialize as human-readable string (pretty-printed): return JsonUtils::WriteStyledString(msg); diff --git a/src/CompositeChat.h b/src/CompositeChat.h index 675837548..b52b76bc5 100644 --- a/src/CompositeChat.h +++ b/src/CompositeChat.h @@ -152,9 +152,8 @@ public: } ; - - using cParts = std::vector; - + /** the parts have to be allocated with new else the part specific parts are not saved (only the cBasePart members). */ + using cParts = std::vector>; /** Creates a new empty chat message. Exported manually due to the other overload needing a manual export. */ @@ -166,6 +165,12 @@ public: Exported manually due to ToLua++ generating extra output parameter. */ cCompositeChat(const AString & a_ParseText, eMessageType a_MessageType = mtCustom); + cCompositeChat(cCompositeChat && a_Other) = default; + + /** Copy constructor is explicitly deleted because m_Parts is not copyable. */ + cCompositeChat(cCompositeChat & a_Other) = delete; + cCompositeChat(const cCompositeChat & a_Other) = delete; + ~cCompositeChat(); // tolua_export // The following are exported in ManualBindings in order to support chaining - they return "self" in Lua (#755) diff --git a/tests/CompositeChat/CompositeChatTest.cpp b/tests/CompositeChat/CompositeChatTest.cpp index a6ac86921..08733d661 100644 --- a/tests/CompositeChat/CompositeChatTest.cpp +++ b/tests/CompositeChat/CompositeChatTest.cpp @@ -96,6 +96,19 @@ static void TestParser5(void) +static void TestParser6(void) +{ + cCompositeChat Msg; + Msg.ParseText("Hello World"); + const cCompositeChat::cParts & Parts = Msg.GetParts(); + TEST_EQUAL(Parts.size(), 1); + TEST_EQUAL(Parts[0]->m_PartType, cCompositeChat::ptText); + TEST_EQUAL(Parts[0]->m_Style, ""); +} + + + + IMPLEMENT_TEST_MAIN("CompositeChat", TestParser1(); @@ -103,4 +116,5 @@ IMPLEMENT_TEST_MAIN("CompositeChat", TestParser3(); TestParser4(); TestParser5(); + TestParser6(); )