From dd8a78c18465f148c5686049a03a00c6b5bb6175 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Thu, 22 Oct 2026 23:40:20 +0200 Subject: [PATCH 01/26] fix(server-list): a server of a group the list does not have no longer leaks it The connect server can send a server whose group (server ID / 20) is not in the server list. Such a server is not shown, but InsertServerGroup had already built a CServerGroup for it, which nobody deleted. The group is a unique_ptr now until the map owns it. --- src/source/Network/Server/ServerListManager.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/source/Network/Server/ServerListManager.cpp b/src/source/Network/Server/ServerListManager.cpp index 6ad7aaee6d..75bba9e28b 100644 --- a/src/source/Network/Server/ServerListManager.cpp +++ b/src/source/Network/Server/ServerListManager.cpp @@ -6,6 +6,8 @@ #include "ServerListManager.h" #include "I18N/All.h" +#include + CServerListManager::CServerListManager() { m_iTotalServer = 0; @@ -120,11 +122,12 @@ void CServerListManager::InsertServerGroup(int iConnectIndex, int iServerPercent } else { - pServerGroup = new CServerGroup; - - if (MakeServerGroup(iConnectIndex / MAX_SERVER_PER_GROUP, pServerGroup) == false) + // A server of a group the list does not have is not shown. + auto pNewServerGroup = std::make_unique(); + if (MakeServerGroup(iConnectIndex / MAX_SERVER_PER_GROUP, pNewServerGroup.get()) == false) return; + pServerGroup = pNewServerGroup.release(); m_mapServerGroup.insert(type_mapServerGroup::value_type(pServerGroup->m_iSequence, pServerGroup)); } From 99541e61518a21986cc70239ffa279761a6ef740 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Thu, 22 Oct 2026 23:41:08 +0200 Subject: [PATCH 02/26] feat(server-list): the login screen's server groups come from ServerList.json The server groups of the login screen (name, order, test server, channel types) are read from Data/Local/ServerList.json instead of the encrypted ServerList.bmd: { "formatVersion": 1, "groups": [ { "id": 0, "name": "Valhalla", "channels": {"1": "gold", "5": "goldPvp", "6": "nonPvp"} } ] } "id" is the server ID / 20, the list order is the button order, and a channel not listed is normal. Errors stop the start with a message, as broken item data does; every problem goes to the log. The file is written with LF like the item and effect data, since the editor writes it. The shipped file was converted from ServerList.bmd. A one-time test reads the bmd the way the old loader did and compares each group with what the JSON gives the login screen: name, order, position and all 20 channel types. The bmd and that test go in the next commit. Two things change on the way: - The old loader read each group's description but never kept it, so the login screen's description panel was always empty. The description is shown now; the shipped groups have none. - Channel types covered only the first 15 of the 20 channels of a group. --- .gitattributes | 2 + src/bin/Data/Local/ServerList.json | 20 + .../GameData/ServerListData/ServerGroup.h | 46 ++ .../ServerListData/ServerListJson.cpp | 396 ++++++++++++++++++ .../GameData/ServerListData/ServerListJson.h | 62 +++ src/source/Engine/Object/ZzzOpenData.cpp | 15 +- .../Network/Server/ServerListManager.cpp | 105 +++-- src/source/Network/Server/ServerListManager.h | 24 +- tests/data/CMakeLists.txt | 13 + .../data/test_server_list_bmd_comparison.cpp | 98 +++++ tests/data/test_server_list_json.cpp | 296 +++++++++++++ 11 files changed, 1008 insertions(+), 69 deletions(-) create mode 100644 src/bin/Data/Local/ServerList.json create mode 100644 src/source/Data/GameData/ServerListData/ServerGroup.h create mode 100644 src/source/Data/GameData/ServerListData/ServerListJson.cpp create mode 100644 src/source/Data/GameData/ServerListData/ServerListJson.h create mode 100644 tests/data/test_server_list_bmd_comparison.cpp create mode 100644 tests/data/test_server_list_json.cpp diff --git a/.gitattributes b/.gitattributes index 1dcfd03d52..c2a0d29c5f 100644 --- a/.gitattributes +++ b/.gitattributes @@ -16,6 +16,8 @@ src/shaders/compiled/*.msl text eol=lf whitespace=-blank-at-eof src/bin/Data/Items/**/*.json text eol=lf # The effect catalogue is checked against the format its writer produces (LF). src/bin/Data/Effects/*.json text eol=lf +# The server list is written by the editor's Server List window with LF. +src/bin/Data/Local/ServerList.json text eol=lf # The creation baseline of the effect recorder is compared line by line on every platform. tests/effects/baseline/*.txt text eol=lf diff --git a/src/bin/Data/Local/ServerList.json b/src/bin/Data/Local/ServerList.json new file mode 100644 index 0000000000..1b738575c9 --- /dev/null +++ b/src/bin/Data/Local/ServerList.json @@ -0,0 +1,20 @@ +{ + "formatVersion": 1, + "groups": [ + { + "id": 0, + "name": "Valhalla", + "channels": {"1": "gold", "2": "gold", "3": "gold", "4": "gold", "5": "goldPvp", "6": "nonPvp", "7": "nonPvp", "8": "nonPvp"} + }, + { + "id": 1, + "name": "Helheim", + "channels": {"1": "gold", "2": "gold", "3": "gold", "4": "gold", "5": "goldPvp", "6": "nonPvp", "7": "nonPvp", "8": "nonPvp"} + }, + { + "id": 2, + "name": "Midgard", + "channels": {"1": "gold", "2": "gold", "3": "gold", "4": "gold", "5": "goldPvp", "6": "nonPvp", "7": "nonPvp", "8": "nonPvp"} + } + ] +} diff --git a/src/source/Data/GameData/ServerListData/ServerGroup.h b/src/source/Data/GameData/ServerListData/ServerGroup.h new file mode 100644 index 0000000000..97611a2b12 --- /dev/null +++ b/src/source/Data/GameData/ServerListData/ServerGroup.h @@ -0,0 +1,46 @@ +#pragma once + +#include "Core/Globals/_define.h" + +#include +#include +#include + +namespace Data::ServerList +{ +// What the login screen writes after a channel's number, and its color there. +// The values are the ones the client always had: bit 0 is "no PvP", bit 1 "gold". +enum class ChannelType : std::uint8_t +{ + Normal = 0, // "Valhalla-1", gray + NonPvp = 1, // "Valhalla-1(Non-PVP)" + GoldPvp = 2, // "Valhalla-1(Gold PVP)", orange + Gold = 3, // "Valhalla-1(Gold)", orange +}; + +inline constexpr int ChannelTypeCount = 4; + +// A server sends its ID; the group is ID / MAX_SERVER_PER_GROUP and the +// channel ID % MAX_SERVER_PER_GROUP + 1. +inline constexpr int MaxChannelsPerGroup = MAX_SERVER_PER_GROUP; +// Server IDs are 16 bit. +inline constexpr int MaxServerGroupId = 0xFFFF / MAX_SERVER_PER_GROUP; +// The width of the group buttons on the login screen. +inline constexpr int MaxServerGroupNameLength = 32; +inline constexpr int MaxServerGroupDescriptionLength = MAX_TEXT_LENGTH - 1; + +// One group of game servers on the login screen: its button, and the name +// its channels get. +struct ServerGroup +{ + int id = 0; + std::string name; // UTF-8 + std::string description; // UTF-8, shown below the channels of the selected group + // A test server group: shown first, and only one of them. + bool testServer = false; + // Index: channel number - 1. + std::array channels{}; + + bool operator==(const ServerGroup&) const = default; +}; +} // namespace Data::ServerList diff --git a/src/source/Data/GameData/ServerListData/ServerListJson.cpp b/src/source/Data/GameData/ServerListData/ServerListJson.cpp new file mode 100644 index 0000000000..8d28bf3b4b --- /dev/null +++ b/src/source/Data/GameData/ServerListData/ServerListJson.cpp @@ -0,0 +1,396 @@ +#include "stdafx.h" + +#include "ServerListJson.h" + +#include "Data/GameData/ItemData/ItemJsonCommon.h" + +#include +#include +#include +#include +#include + +namespace Data::ServerList +{ +namespace +{ +using Items::ItemDataIssue; +using Items::ItemDataIssueSeverity; +using Items::Json::OrderedJson; + +constexpr const char* GroupsKey = "groups"; +constexpr const char* IdKey = "id"; +constexpr const char* NameKey = "name"; +constexpr const char* DescriptionKey = "description"; +constexpr const char* TestServerKey = "testServer"; +constexpr const char* ChannelsKey = "channels"; +// A save writes this file first, then puts it in the old one's place. +constexpr const char* TemporaryExtension = ".tmp"; + +// In the order of the ChannelType values. +constexpr std::array ChannelTypeNames = {"normal", "nonPvp", "goldPvp", "gold"}; + +// The bits of a UTF-8 byte that tell whether it continues a character. +constexpr unsigned char Utf8ContinuationMask = 0xC0; +constexpr unsigned char Utf8ContinuationBits = 0x80; + +class IssueList +{ +public: + IssueList(const std::string& source, std::vector& issues) : m_source(source), m_issues(issues) {} + + void Error(const std::string& field, const std::string& message) + { + Items::Json::AddFileIssue(m_issues, m_source, ItemDataIssue::NoItem, field, message); + } + + void Warning(const std::string& field, const std::string& message) + { + m_issues.push_back( + {ItemDataIssueSeverity::Warning, m_source, ItemDataIssue::NoItem, ItemDataIssue::NoItem, field, message}); + } + +private: + const std::string& m_source; + std::vector& m_issues; +}; + +std::string GroupField(size_t index) +{ + return std::string(GroupsKey) + "[" + std::to_string(index) + "]"; +} + +std::string Field(const std::string& object, const char* key) +{ + return object + "." + key; +} + +bool IsContinuationByte(char byte) +{ + return (static_cast(byte) & Utf8ContinuationMask) == Utf8ContinuationBits; +} + +bool ReadId(const OrderedJson& json, const std::string& field, int& id, IssueList& issues) +{ + const auto value = json.find(IdKey); + long long number = 0; + if (value == json.end() || !value->is_number_integer() || !Items::Json::ReadWholeNumber(*value, number) || + number < 0 || number > MaxServerGroupId) + { + issues.Error(Field(field, IdKey), + "missing or not a whole number from 0 to " + std::to_string(MaxServerGroupId)); + return false; + } + id = static_cast(number); + return true; +} + +// A text of the group; one longer than maxCharacters is cut. +bool ReadText(const OrderedJson& json, const std::string& field, const char* key, size_t maxCharacters, + std::string& text, IssueList& issues) +{ + const auto value = json.find(key); + if (value == json.end()) + { + return true; + } + if (!value->is_string()) + { + issues.Error(Field(field, key), "must be a text"); + return false; + } + text = value->get(); + if (CutToCharacters(text, maxCharacters)) + { + issues.Warning(Field(field, key), "longer than " + std::to_string(maxCharacters) + " characters, cut"); + } + return true; +} + +bool ReadName(const OrderedJson& json, const std::string& field, std::string& name, IssueList& issues) +{ + if (!ReadText(json, field, NameKey, MaxServerGroupNameLength, name, issues)) + { + return false; + } + if (name.empty()) + { + issues.Error(Field(field, NameKey), "missing or empty"); + return false; + } + return true; +} + +bool ReadTestServer(const OrderedJson& json, const std::string& field, bool& testServer, IssueList& issues) +{ + const auto value = json.find(TestServerKey); + if (value == json.end()) + { + return true; + } + if (!value->is_boolean()) + { + issues.Error(Field(field, TestServerKey), "must be true or false"); + return false; + } + testServer = value->get(); + return true; +} + +// "1" to "20"; nothing for anything else. +std::optional ParseChannelNumber(std::string_view text) +{ + int number = 0; + const char* end = text.data() + text.size(); + const auto [parsedEnd, error] = std::from_chars(text.data(), end, number); + if (error != std::errc{} || parsedEnd != end || number < 1 || number > MaxChannelsPerGroup) + { + return std::nullopt; + } + return number; +} + +bool ReadChannel(const std::string& key, const OrderedJson& value, const std::string& field, ServerGroup& group, + IssueList& issues) +{ + const std::optional number = ParseChannelNumber(key); + if (!number) + { + issues.Error(field, "a channel number is 1 to " + std::to_string(MaxChannelsPerGroup)); + return false; + } + const std::optional type = + value.is_string() ? FindChannelType(value.get()) : std::nullopt; + if (!type) + { + issues.Error(field, "must be normal, nonPvp, goldPvp or gold"); + return false; + } + group.channels[*number - 1] = *type; + return true; +} + +bool ReadChannels(const OrderedJson& json, const std::string& field, ServerGroup& group, IssueList& issues) +{ + const auto channels = json.find(ChannelsKey); + if (channels == json.end()) + { + return true; + } + if (!channels->is_object()) + { + issues.Error(Field(field, ChannelsKey), "must be an object of channel numbers and types"); + return false; + } + bool valid = true; + for (const auto& [key, value] : channels->items()) + { + valid = ReadChannel(key, value, Field(Field(field, ChannelsKey), key.c_str()), group, issues) && valid; + } + return valid; +} + +std::optional ReadGroup(const OrderedJson& json, const std::string& field, IssueList& issues) +{ + if (!json.is_object()) + { + issues.Error(field, "must be an object"); + return std::nullopt; + } + ServerGroup group; + bool valid = ReadId(json, field, group.id, issues); + valid = ReadName(json, field, group.name, issues) && valid; + valid = ReadText(json, field, DescriptionKey, MaxServerGroupDescriptionLength, group.description, issues) && valid; + valid = ReadTestServer(json, field, group.testServer, issues) && valid; + valid = ReadChannels(json, field, group, issues) && valid; + Items::Json::WarnAboutUnknownKeys(json, {IdKey, NameKey, DescriptionKey, TestServerKey, ChannelsKey}, field + ".", + [&](const std::string& unknown, const std::string& message) + { issues.Warning(unknown, message); }); + return valid ? std::optional(std::move(group)) : std::nullopt; +} + +// The login screen shows the first test server group only. +void WarnAboutMoreTestServers(const std::vector& groups, IssueList& issues) +{ + const auto first = + std::find_if(groups.begin(), groups.end(), [](const ServerGroup& group) { return group.testServer; }); + if (first == groups.end()) + { + return; + } + for (auto group = std::next(first); group != groups.end(); ++group) + { + if (group->testServer) + { + issues.Warning(TestServerKey, "group " + std::to_string(group->id) + + " is not shown: only the first test server group is, which is group " + + std::to_string(first->id)); + } + } +} + +OrderedJson WriteChannels(const ServerGroup& group) +{ + OrderedJson channels = OrderedJson::object(); + for (size_t index = 0; index < group.channels.size(); ++index) + { + if (group.channels[index] != ChannelType::Normal) + { + channels[std::to_string(index + 1)] = GetChannelTypeName(group.channels[index]); + } + } + return channels; +} + +OrderedJson WriteGroup(const ServerGroup& group) +{ + OrderedJson json = OrderedJson::object(); + json[IdKey] = group.id; + json[NameKey] = group.name; + if (!group.description.empty()) + { + json[DescriptionKey] = group.description; + } + if (group.testServer) + { + json[TestServerKey] = true; + } + OrderedJson channels = WriteChannels(group); + if (!channels.empty()) + { + json[ChannelsKey] = std::move(channels); + } + return json; +} +} // namespace + +bool CutToCharacters(std::string& text, size_t maxCharacters) +{ + size_t characters = 0; + for (size_t i = 0; i < text.size(); ++i) + { + if (!IsContinuationByte(text[i]) && characters++ == maxCharacters) + { + text.resize(i); + return true; + } + } + return false; +} + +const char* GetChannelTypeName(ChannelType type) +{ + return ChannelTypeNames[static_cast(type)]; +} + +std::optional FindChannelType(std::string_view name) +{ + const auto found = std::find(ChannelTypeNames.begin(), ChannelTypeNames.end(), name); + if (found == ChannelTypeNames.end()) + { + return std::nullopt; + } + return static_cast(std::distance(ChannelTypeNames.begin(), found)); +} + +void ReadServerListJson(std::string_view text, const std::string& source, std::vector& groups, + std::vector& issues) +{ + IssueList list(source, issues); + OrderedJson root; + if (!Items::Json::ReadFileVersion(text, source, ServerListFormatVersion, root, issues)) + { + return; + } + Items::Json::WarnAboutUnknownKeys(root, {Items::Json::Keys::FormatVersion, GroupsKey}, "", + [&](const std::string& unknown, const std::string& message) + { list.Warning(unknown, message); }); + const auto entries = root.find(GroupsKey); + if (entries == root.end() || !entries->is_array()) + { + list.Error(GroupsKey, "missing or not a list of server groups"); + return; + } + std::set ids; + for (size_t index = 0; index < entries->size(); ++index) + { + const std::string field = GroupField(index); + std::optional group = ReadGroup((*entries)[index], field, list); + if (!group) + { + continue; + } + if (!ids.insert(group->id).second) + { + list.Error(Field(field, IdKey), "group " + std::to_string(group->id) + " is in the list more than once"); + continue; + } + groups.push_back(std::move(*group)); + } + WarnAboutMoreTestServers(groups, list); +} + +std::string WriteServerListJson(std::span groups) +{ + OrderedJson entries = OrderedJson::array(); + for (const ServerGroup& group : groups) + { + entries.push_back(WriteGroup(group)); + } + OrderedJson root = OrderedJson::object(); + root[Items::Json::Keys::FormatVersion] = ServerListFormatVersion; + root[GroupsKey] = std::move(entries); + const std::string text = root.dump(Items::Json::Indent, ' ', false, OrderedJson::error_handler_t::replace); + return Items::Json::PutObjectsOnOneLine(text, ChannelsKey) + "\n"; +} + +ServerListLoadResult LoadServerListFile(const std::filesystem::path& file) +{ + ServerListLoadResult result; + const std::string source = file.generic_string(); + const std::optional text = Items::Json::ReadTextFile(file); + if (!text) + { + IssueList(source, result.issues).Error("", "could not be read"); + return result; + } + ReadServerListJson(*text, source, result.groups, result.issues); + return result; +} + +bool SaveServerListFile(const std::filesystem::path& file, std::span groups, + std::vector& issues) +{ + const std::string source = file.generic_string(); + const std::string text = WriteServerListJson(groups); + std::vector readBack; + ReadServerListJson(text, source, readBack, issues); + if (Items::HasErrors(issues)) + { + return false; + } + + std::filesystem::path temporaryFile = file; + temporaryFile += TemporaryExtension; + std::error_code error; + std::filesystem::create_directories(file.parent_path(), error); + std::ofstream stream(temporaryFile, std::ios::binary | std::ios::trunc); + stream << text; + // Closing flushes; checking only afterwards also catches a failing final write. + stream.close(); + if (!stream) + { + IssueList(source, issues).Error("", "could not be written"); + std::filesystem::remove(temporaryFile, error); + return false; + } + std::filesystem::rename(temporaryFile, file, error); + if (error) + { + IssueList(source, issues).Error("", "could not be replaced: " + error.message()); + std::filesystem::remove(temporaryFile, error); + return false; + } + return true; +} +} // namespace Data::ServerList diff --git a/src/source/Data/GameData/ServerListData/ServerListJson.h b/src/source/Data/GameData/ServerListData/ServerListJson.h new file mode 100644 index 0000000000..22cb53708d --- /dev/null +++ b/src/source/Data/GameData/ServerListData/ServerListJson.h @@ -0,0 +1,62 @@ +#pragma once + +#include "Data/GameData/ItemData/ItemDataIssue.h" +#include "Data/GameData/ServerListData/ServerGroup.h" + +#include +#include +#include +#include +#include +#include + +// The server groups of the login screen (Data/Local/ServerList.json): +// +// { "formatVersion": 1, +// "groups": [ +// { "id": 0, "name": "Valhalla", "channels": {"1": "gold", "5": "goldPvp", "6": "nonPvp"} }, +// { "id": 1, "name": "Helheim", "description": "PvP everywhere" }, +// { "id": 9, "name": "Test", "testServer": true } ] } +// +// The list order is the order of the buttons. "id" is the server ID / 20: the +// connect server's servers 0 to 19 are group 0, 20 to 39 group 1. A server the +// list has no group for is not shown. "channels" gives the type of a channel +// by its number (server ID % 20 + 1): normal (the default), nonPvp, goldPvp or +// gold. "description" and "testServer" are optional. +namespace Data::ServerList +{ +constexpr int ServerListFormatVersion = 1; + +// The file, relative to the client folder. +constexpr const char* ServerListFile = "Data/Local/ServerList.json"; + +// The name of a channel type in the file ("nonPvp"), and back. +const char* GetChannelTypeName(ChannelType type); +std::optional FindChannelType(std::string_view name); + +// Reads the groups of the file into `groups`, in their order. A group with an +// error is left out; a name or description that is too long is cut (a warning). +void ReadServerListJson(std::string_view text, const std::string& source, std::vector& groups, + std::vector& issues); + +// The text of the file with these groups, in their order. +std::string WriteServerListJson(std::span groups); + +// Cuts UTF-8 text after maxCharacters characters; false when it is not longer. +bool CutToCharacters(std::string& text, size_t maxCharacters); + +struct ServerListLoadResult +{ + std::vector groups; + std::vector issues; +}; + +// Reads the file; one that cannot be read is an error. +ServerListLoadResult LoadServerListFile(const std::filesystem::path& file); + +// Writes the groups to the file when they read back without errors; the +// problems found are added to `issues`. The old file stays when the new one +// cannot be written whole. +bool SaveServerListFile(const std::filesystem::path& file, std::span groups, + std::vector& issues); +} // namespace Data::ServerList diff --git a/src/source/Engine/Object/ZzzOpenData.cpp b/src/source/Engine/Object/ZzzOpenData.cpp index 2f1980b15a..d1cb012ace 100644 --- a/src/source/Engine/Object/ZzzOpenData.cpp +++ b/src/source/Engine/Object/ZzzOpenData.cpp @@ -370,7 +370,7 @@ void OpenPlayerTextures() } } -// The game cannot run with broken item or effect data: shows the errors and quits. +// The game cannot run with broken item, effect or server list data: shows the errors and quits. static void StopOnDataError(const char* title, const std::string& errorMessage) { const std::wstring message = Core::Text::FromUtf8(errorMessage); @@ -384,6 +384,17 @@ static void StopOnItemDataError(const std::string& errorMessage) StopOnDataError("Item data error", errorMessage); } +// Loads the server groups of the login screen (Data/Local/ServerList.json); +// errors stop the game. +static void OpenServerList() +{ + std::string errorMessage; + if (!g_ServerListManager->LoadServerListScript(errorMessage)) + { + StopOnDataError("Server list error", errorMessage); + } +} + // Loads the effect types (Data/Effects) and builds the effect registry from // their creation values; errors stop the game. static void OpenEffectTypeData() @@ -4392,7 +4403,7 @@ void OpenBasicData(HDC hDC) wchar_t Text[100]; - g_ServerListManager->LoadServerListScript(); + OpenServerList(); // NPC dialog text used to live in Data\Local\\Dialog_.bmd // and was loaded into g_DialogScript here. The text and answer labels diff --git a/src/source/Network/Server/ServerListManager.cpp b/src/source/Network/Server/ServerListManager.cpp index 75bba9e28b..a2813d6e65 100644 --- a/src/source/Network/Server/ServerListManager.cpp +++ b/src/source/Network/Server/ServerListManager.cpp @@ -6,8 +6,29 @@ #include "ServerListManager.h" #include "I18N/All.h" +#include "Core/Text/Utf8.h" +#include "Data/DataHandler/DataIssueReport.h" +#include "Data/GameData/ServerListData/ServerListJson.h" + +#include +#include +#include #include +namespace +{ +constexpr const char* ServerListName = "server list"; +// The bit of a channel type that turns PvP off. +constexpr BYTE NonPvpBit = 0x01; + +template void CopyText(wchar_t (&destination)[Capacity], const std::wstring& source) +{ + const size_t length = std::min(source.size(), Capacity - 1); + std::wmemcpy(destination, source.c_str(), length); + destination[length] = L'\0'; +} +} // namespace + CServerListManager::CServerListManager() { m_iTotalServer = 0; @@ -38,60 +59,33 @@ void CServerListManager::Release() m_iTotalServer = 0; } -void CServerListManager::LoadServerListScript() +bool CServerListManager::LoadServerListScript(std::string& errorMessage) { - FILE* fp = ::_wfopen(L"Data\\Local\\ServerList.bmd", L"rb"); - - if (fp == NULL) + const std::filesystem::path file = Data::ServerList::ServerListFile; + Data::ServerList::ServerListLoadResult result = Data::ServerList::LoadServerListFile(file); + Data::LogDataIssues("Server list:", result.issues); + if (Data::Items::HasErrors(result.issues)) { - wchar_t szMessage[256]; - ::mu_swprintf(szMessage, L"Data\\Local\\ServerList.bmd file not found.\r\n"); - g_ErrorReport.Write(szMessage); - ::MessageBox(g_hWnd, szMessage, NULL, MB_OK); - ::PostMessage(g_hWnd, WM_DESTROY, 0, 0); - return; + errorMessage = Data::DescribeDataErrors(ServerListName, file, result.issues); + return false; } + SetServerGroups(result.groups); + return true; +} -#pragma pack(push, 1) - typedef struct _SERVER_GROUP_INFO - { - WORD m_wIndex; - char m_szName[SLM_MAX_SERVER_NAME_LENGTH]; - BYTE m_byPos; - BYTE m_bySequence; - BYTE m_abyNonPVP[SLM_MAX_SERVER_COUNT]; - short m_nDescriptLen; - } SERVER_GROUP_INFO; -#pragma pack(pop) - - int nSize = sizeof(SERVER_GROUP_INFO); - SERVER_GROUP_INFO sServerGroupScript; - char szDescript[1024]; - SServerGroupInfo sServerGroupInfo; - int i; - - while (0 != ::fread(&sServerGroupScript, nSize, 1, fp)) +void CServerListManager::SetServerGroups(std::span groups) +{ + m_mapServerListScript.clear(); + int sequence = 0; + for (const Data::ServerList::ServerGroup& group : groups) { - BuxConvert((BYTE*)&sServerGroupScript, nSize); - ::fread(szDescript, sServerGroupScript.m_nDescriptLen, 1, fp); - BuxConvert((BYTE*)szDescript, sServerGroupScript.m_nDescriptLen); - - CMultiLanguage::ConvertFromUtf8(sServerGroupInfo.m_szName, sServerGroupScript.m_szName); - - sServerGroupInfo.m_byPos = sServerGroupScript.m_byPos; - sServerGroupInfo.m_bySequence = sServerGroupScript.m_bySequence; - for (i = 0; i < SLM_MAX_SERVER_COUNT; ++i) - sServerGroupInfo.m_abyNonPVP[i] = sServerGroupScript.m_abyNonPVP[i]; - - m_mapServerListScript.insert(std::make_pair(sServerGroupScript.m_wIndex, sServerGroupInfo)); + m_mapServerListScript[group.id] = {group, ++sequence}; } - - ::fclose(fp); } -const SServerGroupInfo* CServerListManager::GetServerGroupInfoInScript(WORD wServerGroupIndex) +const SServerGroupInfo* CServerListManager::GetServerGroupInfoInScript(int iServerGroupIndex) { - ServerListScriptMap::const_iterator iter = m_mapServerListScript.find(wServerGroupIndex); + ServerListScriptMap::const_iterator iter = m_mapServerListScript.find(iServerGroupIndex); if (iter == m_mapServerListScript.end()) return NULL; @@ -142,21 +136,19 @@ bool CServerListManager::MakeServerGroup(IN int iServerGroupIndex, OUT CServerGr if (NULL == pServerGroupInfo) return false; - ::wcscpy(pServerGroup->m_szName, pServerGroupInfo->m_szName); - ::wcscpy(pServerGroup->m_szDescription, pServerGroupInfo->m_strDescript.c_str()); - pServerGroup->m_iSequence = (int)pServerGroupInfo->m_bySequence; - pServerGroup->m_iWidthPos = (int)pServerGroupInfo->m_byPos; + const Data::ServerList::ServerGroup& group = pServerGroupInfo->group; + CopyText(pServerGroup->m_szName, Core::Text::FromUtf8(group.name)); + CopyText(pServerGroup->m_szDescription, Core::Text::FromUtf8(group.description)); + pServerGroup->m_iSequence = pServerGroupInfo->sequence; + pServerGroup->m_iWidthPos = group.testServer ? CServerGroup::SBP_CENTER : CServerGroup::SBP_LEFT; pServerGroup->m_iServerIndex = iServerGroupIndex; pServerGroup->m_bPvPServer = true; - int i; - for (i = 0; i < SLM_MAX_SERVER_COUNT; ++i) + for (int i = 0; i < MAX_SERVER_PER_GROUP; ++i) { - pServerGroup->m_abyNonPvpServer[i] = pServerGroupInfo->m_abyNonPVP[i]; - if (0x01 & pServerGroup->m_abyNonPvpServer[i]) + pServerGroup->m_abyNonPvpServer[i] = static_cast(group.channels[i]); + if (NonPvpBit & pServerGroup->m_abyNonPvpServer[i]) pServerGroup->m_bPvPServer = false; } - for (; i < MAX_SERVER_PER_GROUP; ++i) - pServerGroup->m_abyNonPvpServer[i] = 0; return true; } @@ -268,7 +260,6 @@ int CServerListManager::GetSelectServerIndex() return m_iSelectServerIndex; } - BYTE CServerListManager::GetNonPVPInfo() { return m_byNonPvP; @@ -276,7 +267,7 @@ BYTE CServerListManager::GetNonPVPInfo() bool CServerListManager::IsNonPvP() { - return bool(0x01 & GetNonPVPInfo()); + return bool(NonPvpBit & GetNonPVPInfo()); } void CServerListManager::SetTotalServer(int iTotalServer) diff --git a/src/source/Network/Server/ServerListManager.h b/src/source/Network/Server/ServerListManager.h index 7355791a58..80e97ae7ed 100644 --- a/src/source/Network/Server/ServerListManager.h +++ b/src/source/Network/Server/ServerListManager.h @@ -4,19 +4,19 @@ #pragma once #include "ServerGroup.h" -#define SLM_MAX_SERVER_NAME_LENGTH 32 -#define SLM_MAX_SERVER_COUNT 15 +#include "Data/GameData/ServerListData/ServerGroup.h" +#include +#include + +// A group of Data/Local/ServerList.json, with its place in the list. struct SServerGroupInfo { - wchar_t m_szName[SLM_MAX_SERVER_NAME_LENGTH + 1]; - BYTE m_byPos; - BYTE m_bySequence; - BYTE m_abyNonPVP[SLM_MAX_SERVER_COUNT]; - std::wstring m_strDescript; + Data::ServerList::ServerGroup group; + int sequence = 0; }; -typedef std::map ServerListScriptMap; +typedef std::map ServerListScriptMap; typedef std::map type_mapServerGroup; class CServerListManager @@ -32,7 +32,11 @@ class CServerListManager void InsertServerGroup(int iConnectIndex, int iServerPercent); void Release(); - void LoadServerListScript(); + // Reads Data/Local/ServerList.json. False when it has errors; they are in + // errorMessage then, for the player. + bool LoadServerListScript(std::string& errorMessage); + // The groups the login screen names the servers with, in their order. + void SetServerGroups(std::span groups); void SetFirst(); bool GetNext(OUT CServerGroup*& pServerGroup); CServerGroup* GetServerGroupByBtnPos(int iBtnPos); @@ -48,7 +52,7 @@ class CServerListManager int GetTotalServer(); protected: - const SServerGroupInfo* GetServerGroupInfoInScript(WORD wServerGroupIndex); + const SServerGroupInfo* GetServerGroupInfoInScript(int iServerGroupIndex); bool MakeServerGroup(IN int iServerGroupIndex, OUT CServerGroup* pServerGroup); void InsertServer(CServerGroup* pServerGroup, int iConnectIndex, int iServerPercent); diff --git a/tests/data/CMakeLists.txt b/tests/data/CMakeLists.txt index 33c8a9a9f4..38f4cbdc5d 100644 --- a/tests/data/CMakeLists.txt +++ b/tests/data/CMakeLists.txt @@ -34,6 +34,19 @@ target_compile_definitions(light_mood_data_tests PRIVATE MU_TEST_DATA_DIR="${CMAKE_SOURCE_DIR}/src/bin/Data" ) +# The server groups of the login screen (Data/Local/ServerList.json). +mu_add_test( + NAME server_list_data_tests + SOURCES + test_server_list_bmd_comparison.cpp + test_server_list_json.cpp + LINK_LIBS + MuClient +) +target_compile_definitions(server_list_data_tests PRIVATE + MU_TEST_DATA_DIR="${CMAKE_SOURCE_DIR}/src/bin/Data" +) + # The look profile of the Enhanced rendering features (Data/Render/Look.json) and the player's own # tuning of it ([EnhancedTuning] of config.ini). mu_add_test( diff --git a/tests/data/test_server_list_bmd_comparison.cpp b/tests/data/test_server_list_bmd_comparison.cpp new file mode 100644 index 0000000000..d3c87fadba --- /dev/null +++ b/tests/data/test_server_list_bmd_comparison.cpp @@ -0,0 +1,98 @@ +#include "stdafx.h" + +#include "doctest.h" + +#include "Data/GameData/ServerListData/ServerListJson.h" + +#include +#include +#include +#include +#include +#include + +// One-time check that Data/Local/ServerList.json holds what ServerList.bmd +// held: the bmd is read here the way CServerListManager::LoadServerListScript +// read it, and each group is compared with what the JSON gives the login screen. +namespace +{ +constexpr int BmdNameLength = 32; +constexpr int BmdChannelCount = 15; +constexpr int BmdDescriptionCapacity = 1024; + +#pragma pack(push, 1) +struct BmdServerGroup +{ + WORD index; + char name[BmdNameLength]; + BYTE position; + BYTE sequence; + BYTE nonPvp[BmdChannelCount]; + short descriptionLength; +}; +#pragma pack(pop) + +struct OldGroup +{ + std::string name; + BYTE position = 0; + BYTE sequence = 0; + BYTE nonPvp[BmdChannelCount] = {}; +}; + +std::map ReadBmd(const std::filesystem::path& file) +{ + std::map groups; + FILE* fp = std::fopen(file.string().c_str(), "rb"); + REQUIRE(fp != nullptr); + BmdServerGroup entry{}; + char description[BmdDescriptionCapacity]; + while (std::fread(&entry, sizeof(entry), 1, fp) != 0) + { + BuxConvert(reinterpret_cast(&entry), sizeof(entry)); + std::fread(description, entry.descriptionLength, 1, fp); + OldGroup group; + group.name.assign(entry.name, strnlen(entry.name, BmdNameLength)); + group.position = entry.position; + group.sequence = entry.sequence; + std::memcpy(group.nonPvp, entry.nonPvp, sizeof(group.nonPvp)); + groups.insert({entry.index, group}); + } + std::fclose(fp); + return groups; +} +} // namespace + +TEST_CASE("ServerList.json holds what ServerList.bmd held") +{ + const std::filesystem::path local = std::filesystem::path(MU_TEST_DATA_DIR) / "Local"; + const std::map old = ReadBmd(local / "ServerList.bmd"); + const Data::ServerList::ServerListLoadResult loaded = + Data::ServerList::LoadServerListFile(local / "ServerList.json"); + + REQUIRE(loaded.issues.empty()); + REQUIRE(loaded.groups.size() == old.size()); + for (size_t index = 0; index < loaded.groups.size(); ++index) + { + const Data::ServerList::ServerGroup& group = loaded.groups[index]; + CAPTURE(group.id); + const auto found = old.find(group.id); + REQUIRE(found != old.end()); + const OldGroup& before = found->second; + CHECK(group.name == before.name); + // The login screen orders the groups by their sequence; the JSON by the list. + CHECK(static_cast(index + 1) == before.sequence); + // Only the center position (test server) is drawn apart. + CHECK(group.testServer == (before.position == 2)); + for (int channel = 0; channel < Data::ServerList::MaxChannelsPerGroup; ++channel) + { + CAPTURE(channel); + // The old loader set the channels after the 15th to 0. + const BYTE expected = channel < BmdChannelCount ? before.nonPvp[channel] : 0; + CHECK(static_cast(group.channels[channel]) == expected); + } + // The old loader read the description but never kept it: the login + // screen showed none. + CHECK(group.description.empty()); + } +} diff --git a/tests/data/test_server_list_json.cpp b/tests/data/test_server_list_json.cpp new file mode 100644 index 0000000000..a1cd9b993a --- /dev/null +++ b/tests/data/test_server_list_json.cpp @@ -0,0 +1,296 @@ +#include "stdafx.h" + +#include "doctest.h" + +#include "Data/GameData/ItemData/ItemJsonCommon.h" +#include "Data/GameData/ServerListData/ServerListJson.h" + +#include +#include +#include +#include +#include + +using Data::Items::HasErrors; +using Data::Items::ItemDataIssue; +using Data::Items::ItemDataIssueSeverity; +using Data::ServerList::ChannelType; +using Data::ServerList::ServerGroup; + +namespace +{ +struct ReadResult +{ + std::vector groups; + std::vector issues; +}; + +ReadResult Read(const std::string& text) +{ + ReadResult result; + Data::ServerList::ReadServerListJson(text, "ServerList.json", result.groups, result.issues); + return result; +} + +std::string Messages(const std::vector& issues) +{ + std::string text; + for (const ItemDataIssue& issue : issues) + { + text += issue.ToString() + "\n"; + } + return text; +} + +bool HasIssueAbout(const std::vector& issues, const std::string& field, const std::string& message) +{ + return std::any_of(issues.begin(), issues.end(), [&](const ItemDataIssue& issue) + { return issue.field == field && issue.message.find(message) != std::string::npos; }); +} + +bool HasWarnings(const std::vector& issues) +{ + return std::any_of(issues.begin(), issues.end(), + [](const ItemDataIssue& issue) { return issue.severity == ItemDataIssueSeverity::Warning; }); +} + +// One group with nothing else around it. +std::string GroupFile(const std::string& group) +{ + return R"({ "formatVersion": 1, "groups": [ )" + group + " ] }"; +} + +const std::string ValidFile = R"({ + "formatVersion": 1, + "groups": [ + { "id": 0, "name": "Valhalla", "channels": {"1": "gold", "5": "goldPvp", "6": "nonPvp", "20": "normal"} }, + { "id": 3, "name": "Kanturu", "description": "No PvP here", "testServer": true } + ] +})"; +} // namespace + +TEST_CASE("ServerListJson: reads the groups in their order") +{ + const ReadResult read = Read(ValidFile); + + CHECK_MESSAGE(read.issues.empty(), Messages(read.issues)); + REQUIRE(read.groups.size() == 2); + + const ServerGroup& valhalla = read.groups[0]; + CHECK(valhalla.id == 0); + CHECK(valhalla.name == "Valhalla"); + CHECK(valhalla.description.empty()); + CHECK_FALSE(valhalla.testServer); + CHECK(valhalla.channels[0] == ChannelType::Gold); + CHECK(valhalla.channels[1] == ChannelType::Normal); + CHECK(valhalla.channels[4] == ChannelType::GoldPvp); + CHECK(valhalla.channels[5] == ChannelType::NonPvp); + CHECK(valhalla.channels[19] == ChannelType::Normal); + + const ServerGroup& kanturu = read.groups[1]; + CHECK(kanturu.id == 3); + CHECK(kanturu.description == "No PvP here"); + CHECK(kanturu.testServer); + CHECK(std::all_of(kanturu.channels.begin(), kanturu.channels.end(), + [](ChannelType type) { return type == ChannelType::Normal; })); +} + +TEST_CASE("ServerListJson: writes what it reads") +{ + const ReadResult read = Read(ValidFile); + const std::string written = Data::ServerList::WriteServerListJson(read.groups); + const ReadResult readBack = Read(written); + + CHECK_MESSAGE(readBack.issues.empty(), Messages(readBack.issues)); + CHECK(readBack.groups == read.groups); + // Normal channels and empty or false fields are left out. + CHECK(written.find("\"20\"") == std::string::npos); + CHECK(written.find("\"description\": \"\"") == std::string::npos); + CHECK(written.find("\"testServer\": false") == std::string::npos); + // The channels of a group are on one line. + CHECK(written.find(R"("channels": {"1": "gold", "5": "goldPvp", "6": "nonPvp"})") != std::string::npos); +} + +TEST_CASE("ServerListJson: an empty list is a valid file") +{ + const ReadResult read = Read(R"({ "formatVersion": 1, "groups": [] })"); + + CHECK(read.issues.empty()); + CHECK(read.groups.empty()); + CHECK(Data::ServerList::WriteServerListJson(read.groups) == "{\n \"formatVersion\": 1,\n \"groups\": []\n}\n"); +} + +TEST_CASE("ServerListJson: the file needs its version and a list of groups") +{ + CHECK(HasErrors(Read("not json").issues)); + CHECK(HasErrors(Read(R"({ "groups": [] })").issues)); + CHECK(HasErrors(Read(R"({ "formatVersion": 2, "groups": [] })").issues)); + + const ReadResult noList = Read(R"({ "formatVersion": 1, "groups": {} })"); + CHECK(HasIssueAbout(noList.issues, "groups", "not a list")); +} + +TEST_CASE("ServerListJson: a group needs an id and a name") +{ + const ReadResult noId = Read(GroupFile(R"({ "name": "Valhalla" })")); + CHECK(HasIssueAbout(noId.issues, "groups[0].id", "missing")); + CHECK(noId.groups.empty()); + + const ReadResult badId = Read(GroupFile(R"({ "id": 3277, "name": "Valhalla" })")); + CHECK(HasIssueAbout(badId.issues, "groups[0].id", "from 0 to 3276")); + + const ReadResult negativeId = Read(GroupFile(R"({ "id": -1, "name": "Valhalla" })")); + CHECK(HasErrors(negativeId.issues)); + + const ReadResult noName = Read(GroupFile(R"({ "id": 0 })")); + CHECK(HasIssueAbout(noName.issues, "groups[0].name", "missing or empty")); + + const ReadResult emptyName = Read(GroupFile(R"({ "id": 0, "name": "" })")); + CHECK(HasIssueAbout(emptyName.issues, "groups[0].name", "missing or empty")); + + const ReadResult notAnObject = Read(GroupFile("5")); + CHECK(HasIssueAbout(notAnObject.issues, "groups[0]", "must be an object")); +} + +TEST_CASE("ServerListJson: a group id is in the list once") +{ + const ReadResult read = Read(R"({ "formatVersion": 1, "groups": [ + { "id": 1, "name": "Helheim" }, { "id": 1, "name": "Midgard" } ] })"); + + CHECK(HasIssueAbout(read.issues, "groups[1].id", "more than once")); + REQUIRE(read.groups.size() == 1); + CHECK(read.groups[0].name == "Helheim"); +} + +TEST_CASE("ServerListJson: channels are numbered 1 to 20 and have a known type") +{ + const ReadResult zero = Read(GroupFile(R"({ "id": 0, "name": "A", "channels": {"0": "gold"} })")); + CHECK(HasIssueAbout(zero.issues, "groups[0].channels.0", "1 to 20")); + + const ReadResult tooHigh = Read(GroupFile(R"({ "id": 0, "name": "A", "channels": {"21": "gold"} })")); + CHECK(HasIssueAbout(tooHigh.issues, "groups[0].channels.21", "1 to 20")); + + const ReadResult notANumber = Read(GroupFile(R"({ "id": 0, "name": "A", "channels": {"1a": "gold"} })")); + CHECK(HasErrors(notANumber.issues)); + + const ReadResult unknownType = Read(GroupFile(R"({ "id": 0, "name": "A", "channels": {"1": "silver"} })")); + CHECK(HasIssueAbout(unknownType.issues, "groups[0].channels.1", "normal, nonPvp, goldPvp or gold")); + + const ReadResult notAnObject = Read(GroupFile(R"({ "id": 0, "name": "A", "channels": ["gold"] })")); + CHECK(HasIssueAbout(notAnObject.issues, "groups[0].channels", "object")); +} + +TEST_CASE("ServerListJson: a name or description that is too long is cut") +{ + const std::string longName(40, 'x'); + const std::string longDescription(300, 'y'); + const ReadResult read = + Read(GroupFile(R"({ "id": 0, "name": ")" + longName + R"(", "description": ")" + longDescription + R"(" })")); + + CHECK_FALSE(HasErrors(read.issues)); + CHECK(HasIssueAbout(read.issues, "groups[0].name", "longer than 32 characters")); + CHECK(HasIssueAbout(read.issues, "groups[0].description", "longer than 254 characters")); + REQUIRE(read.groups.size() == 1); + CHECK(read.groups[0].name == std::string(32, 'x')); + CHECK(read.groups[0].description.size() == 254); +} + +TEST_CASE("ServerListJson: names are counted in characters, not bytes") +{ + // 32 two-byte characters fit; the cut never splits one. + std::string name; + for (int i = 0; i < 33; ++i) + { + name += "\xC3\xA4"; // a-umlaut + } + const ReadResult read = Read(GroupFile(R"({ "id": 0, "name": ")" + name + R"(" })")); + + REQUIRE(read.groups.size() == 1); + CHECK(read.groups[0].name == name.substr(0, 64)); +} + +TEST_CASE("ServerListJson: unknown fields and a second test server are warnings") +{ + const ReadResult read = Read(R"({ "formatVersion": 1, "color": "red", "groups": [ + { "id": 0, "name": "A", "testServer": true, "order": 3 }, + { "id": 1, "name": "B", "testServer": true } ] })"); + + CHECK_FALSE(HasErrors(read.issues)); + CHECK(HasWarnings(read.issues)); + CHECK(HasIssueAbout(read.issues, "color", "unknown field")); + CHECK(HasIssueAbout(read.issues, "groups[0].order", "unknown field")); + CHECK(HasIssueAbout(read.issues, "testServer", "only the first test server group")); + CHECK(read.groups.size() == 2); +} + +TEST_CASE("ServerListJson: wrong types are errors") +{ + CHECK(HasIssueAbout(Read(GroupFile(R"({ "id": 0, "name": 5 })")).issues, "groups[0].name", "must be a text")); + CHECK(HasIssueAbout(Read(GroupFile(R"({ "id": 0, "name": "A", "testServer": 1 })")).issues, "groups[0].testServer", + "true or false")); + CHECK(HasIssueAbout(Read(GroupFile(R"({ "id": "0", "name": "A" })")).issues, "groups[0].id", "whole number")); +} + +TEST_CASE("ServerListJson: channel type names") +{ + for (const ChannelType type : {ChannelType::Normal, ChannelType::NonPvp, ChannelType::GoldPvp, ChannelType::Gold}) + { + CHECK(Data::ServerList::FindChannelType(Data::ServerList::GetChannelTypeName(type)) == type); + } + CHECK_FALSE(Data::ServerList::FindChannelType("Gold").has_value()); +} + +TEST_CASE("ServerListJson: the shipped file reads without problems and is written as the editor writes it") +{ + // MU_TEST_DATA_DIR is the client folder's Data; the file name starts with it. + const std::filesystem::path file = + std::filesystem::path(MU_TEST_DATA_DIR).parent_path() / Data::ServerList::ServerListFile; + const Data::ServerList::ServerListLoadResult loaded = Data::ServerList::LoadServerListFile(file); + + CHECK_MESSAGE(loaded.issues.empty(), Messages(loaded.issues)); + REQUIRE(loaded.groups.size() == 3); + CHECK(loaded.groups[0].name == "Valhalla"); + CHECK(loaded.groups[1].name == "Helheim"); + CHECK(loaded.groups[2].name == "Midgard"); + + const std::optional text = Data::Items::Json::ReadTextFile(file); + REQUIRE(text.has_value()); + CHECK(Data::ServerList::WriteServerListJson(loaded.groups) == *text); +} + +TEST_CASE("ServerListJson: a missing file is an error") +{ + const Data::ServerList::ServerListLoadResult loaded = + Data::ServerList::LoadServerListFile("does/not/exist/ServerList.json"); + + CHECK(HasIssueAbout(loaded.issues, "", "could not be read")); + CHECK(loaded.groups.empty()); +} + +TEST_CASE("ServerListJson: save writes the file and refuses a list with errors") +{ + const std::filesystem::path folder = std::filesystem::temp_directory_path() / "mu_server_list_save_test"; + std::filesystem::remove_all(folder); + const std::filesystem::path file = folder / "Local" / "ServerList.json"; + const ReadResult read = Read(ValidFile); + + std::vector issues; + REQUIRE(Data::ServerList::SaveServerListFile(file, read.groups, issues)); + CHECK(issues.empty()); + const Data::ServerList::ServerListLoadResult saved = Data::ServerList::LoadServerListFile(file); + CHECK(saved.issues.empty()); + CHECK(saved.groups == read.groups); + + std::vector broken = read.groups; + broken[1].name.clear(); + broken.push_back(read.groups[0]); + std::vector refused; + CHECK_FALSE(Data::ServerList::SaveServerListFile(file, broken, refused)); + CHECK(HasIssueAbout(refused, "groups[1].name", "missing or empty")); + CHECK(HasIssueAbout(refused, "groups[2].id", "more than once")); + // The file stays as it was. + CHECK(Data::ServerList::LoadServerListFile(file).groups == read.groups); + CHECK_FALSE(std::filesystem::exists(file.string() + ".tmp")); + + std::filesystem::remove_all(folder); +} From de59075046984e9f615a7bd2175d93e7d9e7b0c4 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Thu, 22 Oct 2026 23:41:15 +0200 Subject: [PATCH 03/26] chore(server-list): remove ServerList.bmd and its one-time comparison The client reads ServerList.json since the last commit, whose test showed the JSON holds what the bmd held. Neither the bmd nor that test is needed any more. --- src/bin/Data/Local/ServerList.bmd | 1 - tests/data/CMakeLists.txt | 1 - .../data/test_server_list_bmd_comparison.cpp | 98 ------------------- 3 files changed, 100 deletions(-) delete mode 100644 src/bin/Data/Local/ServerList.bmd delete mode 100644 tests/data/test_server_list_bmd_comparison.cpp diff --git a/src/bin/Data/Local/ServerList.bmd b/src/bin/Data/Local/ServerList.bmd deleted file mode 100644 index 3993621209..0000000000 --- a/src/bin/Data/Local/ServerList.bmd +++ /dev/null @@ -1 +0,0 @@ -ÝǝϫϫϫϫϫϫϫϫϪ̨ͪΫϫϫ㙣Ùϫϫϫϫϫϫϫϫϩ̨ͪΫϫϫ敫̝ϫϫϫϫϫϫϫϫϨ̨ͪΫϫϫ \ No newline at end of file diff --git a/tests/data/CMakeLists.txt b/tests/data/CMakeLists.txt index 38f4cbdc5d..a1f2494cc1 100644 --- a/tests/data/CMakeLists.txt +++ b/tests/data/CMakeLists.txt @@ -38,7 +38,6 @@ target_compile_definitions(light_mood_data_tests PRIVATE mu_add_test( NAME server_list_data_tests SOURCES - test_server_list_bmd_comparison.cpp test_server_list_json.cpp LINK_LIBS MuClient diff --git a/tests/data/test_server_list_bmd_comparison.cpp b/tests/data/test_server_list_bmd_comparison.cpp deleted file mode 100644 index d3c87fadba..0000000000 --- a/tests/data/test_server_list_bmd_comparison.cpp +++ /dev/null @@ -1,98 +0,0 @@ -#include "stdafx.h" - -#include "doctest.h" - -#include "Data/GameData/ServerListData/ServerListJson.h" - -#include -#include -#include -#include -#include -#include - -// One-time check that Data/Local/ServerList.json holds what ServerList.bmd -// held: the bmd is read here the way CServerListManager::LoadServerListScript -// read it, and each group is compared with what the JSON gives the login screen. -namespace -{ -constexpr int BmdNameLength = 32; -constexpr int BmdChannelCount = 15; -constexpr int BmdDescriptionCapacity = 1024; - -#pragma pack(push, 1) -struct BmdServerGroup -{ - WORD index; - char name[BmdNameLength]; - BYTE position; - BYTE sequence; - BYTE nonPvp[BmdChannelCount]; - short descriptionLength; -}; -#pragma pack(pop) - -struct OldGroup -{ - std::string name; - BYTE position = 0; - BYTE sequence = 0; - BYTE nonPvp[BmdChannelCount] = {}; -}; - -std::map ReadBmd(const std::filesystem::path& file) -{ - std::map groups; - FILE* fp = std::fopen(file.string().c_str(), "rb"); - REQUIRE(fp != nullptr); - BmdServerGroup entry{}; - char description[BmdDescriptionCapacity]; - while (std::fread(&entry, sizeof(entry), 1, fp) != 0) - { - BuxConvert(reinterpret_cast(&entry), sizeof(entry)); - std::fread(description, entry.descriptionLength, 1, fp); - OldGroup group; - group.name.assign(entry.name, strnlen(entry.name, BmdNameLength)); - group.position = entry.position; - group.sequence = entry.sequence; - std::memcpy(group.nonPvp, entry.nonPvp, sizeof(group.nonPvp)); - groups.insert({entry.index, group}); - } - std::fclose(fp); - return groups; -} -} // namespace - -TEST_CASE("ServerList.json holds what ServerList.bmd held") -{ - const std::filesystem::path local = std::filesystem::path(MU_TEST_DATA_DIR) / "Local"; - const std::map old = ReadBmd(local / "ServerList.bmd"); - const Data::ServerList::ServerListLoadResult loaded = - Data::ServerList::LoadServerListFile(local / "ServerList.json"); - - REQUIRE(loaded.issues.empty()); - REQUIRE(loaded.groups.size() == old.size()); - for (size_t index = 0; index < loaded.groups.size(); ++index) - { - const Data::ServerList::ServerGroup& group = loaded.groups[index]; - CAPTURE(group.id); - const auto found = old.find(group.id); - REQUIRE(found != old.end()); - const OldGroup& before = found->second; - CHECK(group.name == before.name); - // The login screen orders the groups by their sequence; the JSON by the list. - CHECK(static_cast(index + 1) == before.sequence); - // Only the center position (test server) is drawn apart. - CHECK(group.testServer == (before.position == 2)); - for (int channel = 0; channel < Data::ServerList::MaxChannelsPerGroup; ++channel) - { - CAPTURE(channel); - // The old loader set the channels after the 15th to 0. - const BYTE expected = channel < BmdChannelCount ? before.nonPvp[channel] : 0; - CHECK(static_cast(group.channels[channel]) == expected); - } - // The old loader read the description but never kept it: the login - // screen showed none. - CHECK(group.description.empty()); - } -} From d3d360d9dc0b0fd93902f5197ded2656447c85e8 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Thu, 22 Oct 2026 23:42:09 +0200 Subject: [PATCH 04/26] feat(editor): a Server List window for the login screen's server groups Windows > Server List shows the groups of Data/Local/ServerList.json in their order (Add, Remove, Move up, Move down) and the selected group: its ID with the server IDs it covers, name, description, the test server mark and the type of each of its 20 channels, with the name the login screen gives the channel. - The login screen shows each change right away: the server list manager keeps the last list the connect server sent (editor builds only) and builds the groups from it again. - Groups the connect server sent that the list does not have are listed with an Add button; the channel table shows which channels the connect server sent, with their load. - Save writes the file and refuses errors (a group without a name, an ID twice), Revert reads it again, Undo and Redo (Ctrl+Z, Ctrl+Y) go through the changes of the session; a text field's whole edit is one step. - Texts in English and German. --- src/Localization/Editor.de.resx | 87 +++ src/Localization/Editor.en.resx | 87 +++ src/MuEditor/Core/MuEditorCore.cpp | 5 + src/MuEditor/README.md | 4 + src/MuEditor/UI/Common/MuEditorLayout.cpp | 5 +- src/MuEditor/UI/Common/MuEditorToolWindows.h | 1 + src/MuEditor/UI/Common/MuEditorUI.cpp | 1 + src/MuEditor/UI/Common/MuEditorWindowIds.h | 1 + src/MuEditor/UI/ServerList/MuServerListUI.cpp | 535 ++++++++++++++++++ src/MuEditor/UI/ServerList/MuServerListUI.h | 61 ++ .../UI/ServerList/ServerListEdits.cpp | 116 ++++ src/MuEditor/UI/ServerList/ServerListEdits.h | 82 +++ .../Network/Server/ServerListManager.cpp | 20 + src/source/Network/Server/ServerListManager.h | 21 + tests/data/CMakeLists.txt | 1 + tests/data/test_server_list_edits.cpp | 191 +++++++ 16 files changed, 1216 insertions(+), 2 deletions(-) create mode 100644 src/MuEditor/UI/ServerList/MuServerListUI.cpp create mode 100644 src/MuEditor/UI/ServerList/MuServerListUI.h create mode 100644 src/MuEditor/UI/ServerList/ServerListEdits.cpp create mode 100644 src/MuEditor/UI/ServerList/ServerListEdits.h create mode 100644 tests/data/test_server_list_edits.cpp diff --git a/src/Localization/Editor.de.resx b/src/Localization/Editor.de.resx index 820ef1746e..2ac85aa705 100644 --- a/src/Localization/Editor.de.resx +++ b/src/Localization/Editor.de.resx @@ -1852,4 +1852,91 @@ Erst die Änderungen anwenden oder zurücksetzen. + + Serverliste + + + Speichern + + + Liest {0} neu ein; ungespeicherte Änderungen gehen verloren. + + + Ungespeicherte Änderungen + + + In {0} gespeichert + + + Nicht gespeichert, siehe die Probleme unten. + + + Probleme in {0}: + + + Neue Gruppe + + + Gruppen-ID + + + Server {0} bis {1} + + + Eine andere Gruppe hat die ID {0}. + + + Eine Gruppe braucht einen Namen. + + + Beschreibung + + + Steht unter den Kanälen, solange die Gruppe gewählt ist. + + + Testserver + + + Steht oben; der Anmeldebildschirm zeigt nur die erste Testserver-Gruppe. + + + Kanal + + + Art + + + Auf dem Anmeldebildschirm + + + Connect-Server + + + Normal + + + Kein PvP + + + Gold-PvP + + + Gold + + + Gemeldet, {0} % Last + + + Der Connect-Server meldet Server von Gruppen, die die Liste nicht hat; der Anmeldebildschirm zeigt sie nicht: + + + Gruppe {0}: Server {1} + + + Wählen Sie eine Gruppe in der Liste. + + + Die Reihenfolge der Liste ist die der Schaltflächen auf dem Anmeldebildschirm, der jede Änderung sofort zeigt. Speichern schreibt {0}. + diff --git a/src/Localization/Editor.en.resx b/src/Localization/Editor.en.resx index cd0b111810..59a4a4dff9 100644 --- a/src/Localization/Editor.en.resx +++ b/src/Localization/Editor.en.resx @@ -1873,4 +1873,91 @@ Apply or revert the changes first. + + Server List + + + Save + + + Reads {0} again; unsaved changes are lost. + + + Unsaved changes + + + Saved to {0} + + + Not saved, see the problems below. + + + Problems in {0}: + + + New Group + + + Group ID + + + Servers {0} to {1} + + + Another group has the ID {0}. + + + A group needs a name. + + + Description + + + Shown below the channels while the group is selected. + + + Test server + + + Shown first; the login screen shows only the first test server group. + + + Channel + + + Type + + + On the login screen + + + Connect server + + + Normal + + + Non-PvP + + + Gold PvP + + + Gold + + + Sent, {0}% load + + + The connect server sent servers of groups the list does not have; the login screen does not show them: + + + Group {0}: servers {1} + + + Pick a group in the list. + + + The order of the list is the order of the buttons on the login screen, which shows each change right away. Save writes {0}. + diff --git a/src/MuEditor/Core/MuEditorCore.cpp b/src/MuEditor/Core/MuEditorCore.cpp index 8a98f83899..301999fb5b 100644 --- a/src/MuEditor/Core/MuEditorCore.cpp +++ b/src/MuEditor/Core/MuEditorCore.cpp @@ -33,6 +33,7 @@ #include "../MuEditor/UI/MapEditor/MapEditorUI.h" #include "../MuEditor/UI/EffectBrowser/MuEffectBrowserUI.h" #include "../MuEditor/UI/ItemTranslations/MuItemTranslationsUI.h" +#include "../MuEditor/UI/ServerList/MuServerListUI.h" #include "../UI/Common/MuEditorUI.h" #include "../UI/Console/MuEditorConsoleUI.h" #include "I18N/All.h" @@ -876,6 +877,10 @@ void CMuEditorCore::Render() { g_MuItemTranslationsUI.Render(&m_windows.itemTranslations); } + if (m_windows.serverList) + { + g_MuServerListUI.Render(&m_windows.serverList); + } RenderItemTools(); // Render console (if enabled) diff --git a/src/MuEditor/README.md b/src/MuEditor/README.md index 3f52ec1fec..bd6a59bccd 100644 --- a/src/MuEditor/README.md +++ b/src/MuEditor/README.md @@ -53,6 +53,10 @@ stores it when a field is left. ### Item Translations - **MuItemTranslationsUI.h/cpp** - Item names as a grid of items and languages, with missing counts (docs/translation-system.md) +### Server List (UI/ServerList) +- **MuServerListUI.h/cpp** - The server groups of the login screen (Data/Local/ServerList.json): order, names, test server and channel types, shown on the login screen right away (docs/server-list.md) +- **ServerListEdits.h/cpp** - The groups being changed, what the file has, and the window's undo + ### Input Management - **MuInputBlocker.h/cpp** - Blocks game input when hovering over ImGui UI diff --git a/src/MuEditor/UI/Common/MuEditorLayout.cpp b/src/MuEditor/UI/Common/MuEditorLayout.cpp index ea5c3efae0..80c7d45535 100644 --- a/src/MuEditor/UI/Common/MuEditorLayout.cpp +++ b/src/MuEditor/UI/Common/MuEditorLayout.cpp @@ -47,8 +47,9 @@ void BuildDefaultLayout(ImGuiID dockSpace, ImVec2 size) { ImGui::DockBuilderDockWindow(window, otherTools); } - for (const char* window : {MuEditor::WindowIds::SkillEditor, MuEditor::WindowIds::DevEditor, - MuEditor::WindowIds::MapEditor, MuEditor::WindowIds::EffectBrowser}) + for (const char* window : + {MuEditor::WindowIds::SkillEditor, MuEditor::WindowIds::DevEditor, MuEditor::WindowIds::MapEditor, + MuEditor::WindowIds::EffectBrowser, MuEditor::WindowIds::ServerList}) { ImGui::DockBuilderDockWindow(window, otherTools); } diff --git a/src/MuEditor/UI/Common/MuEditorToolWindows.h b/src/MuEditor/UI/Common/MuEditorToolWindows.h index c16294d08d..daa0c22dca 100644 --- a/src/MuEditor/UI/Common/MuEditorToolWindows.h +++ b/src/MuEditor/UI/Common/MuEditorToolWindows.h @@ -19,6 +19,7 @@ struct ToolWindows bool itemFormulas = false; bool itemChecks = false; bool itemTranslations = false; + bool serverList = false; bool skillEditor = false; bool devEditor = false; bool mapEditor = false; diff --git a/src/MuEditor/UI/Common/MuEditorUI.cpp b/src/MuEditor/UI/Common/MuEditorUI.cpp index 67d3a77c29..71926921e2 100644 --- a/src/MuEditor/UI/Common/MuEditorUI.cpp +++ b/src/MuEditor/UI/Common/MuEditorUI.cpp @@ -228,6 +228,7 @@ void CMuEditorUI::RenderWindowsMenu(MuEditor::ToolWindows& windows, SDL_Window* ImGui::MenuItem(I18N::Editor::ItemTranslations, nullptr, &windows.itemTranslations); ImGui::EndMenu(); } + ImGui::MenuItem(I18N::Editor::ServerList, nullptr, &windows.serverList); ImGui::MenuItem("Skill Editor", nullptr, &windows.skillEditor); ImGui::MenuItem("Dev Editor", nullptr, &windows.devEditor); ImGui::MenuItem("Map Editor", nullptr, &windows.mapEditor); diff --git a/src/MuEditor/UI/Common/MuEditorWindowIds.h b/src/MuEditor/UI/Common/MuEditorWindowIds.h index 801a22c6ec..51b04a82d6 100644 --- a/src/MuEditor/UI/Common/MuEditorWindowIds.h +++ b/src/MuEditor/UI/Common/MuEditorWindowIds.h @@ -25,6 +25,7 @@ inline constexpr char SkillEditor[] = "###SkillEditor"; inline constexpr char DevEditor[] = "###DevEditor"; inline constexpr char EffectBrowser[] = "###EffectBrowser"; inline constexpr char ItemTranslations[] = "###ItemTranslations"; +inline constexpr char ServerList[] = "###ServerList"; // Titles that are not translated: the whole title is the name. inline constexpr char MapEditor[] = "Map Editor"; inline constexpr char Console[] = "Console"; diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp new file mode 100644 index 0000000000..04f99ff647 --- /dev/null +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -0,0 +1,535 @@ +#include "stdafx.h" + +#ifdef _EDITOR + +#include "MuServerListUI.h" + +#include "../MuEditor/Core/MuEditorCore.h" +#include "../MuEditor/UI/Common/KeyboardSelection.h" +#include "../MuEditor/UI/Common/MuEditorLayout.h" +#include "../MuEditor/UI/Common/MuEditorWindowIds.h" +#include "../MuEditor/UI/ItemTools/ItemTextInput.h" +#include "Data/GameData/ServerListData/ServerListJson.h" +#include "I18N/All.h" +#include "Network/Server/ServerListManager.h" +#include "Scenes/SceneCore.h" +#include "UI/Windows/ServerSelWin.h" +#include "imgui.h" + +#include +#include +#include +#include + +using Data::ServerList::ChannelType; +using Data::ServerList::ServerGroup; + +namespace +{ +// Pixel widths before the editor UI scale. +constexpr float GroupListWidth = 220.0f; +constexpr float IdFieldWidth = 120.0f; +constexpr float TextFieldWidth = 320.0f; +constexpr float TypeComboWidth = 140.0f; +constexpr int ChannelTableColumns = 4; +constexpr ImVec4 ErrorColor{1.0f, 0.4f, 0.4f, 1.0f}; + +const char* GetChannelTypeLabel(ChannelType type) +{ + switch (type) + { + case ChannelType::NonPvp: + return I18N::Editor::NonPvP; + case ChannelType::GoldPvp: + return I18N::Editor::GoldPvP; + case ChannelType::Gold: + return I18N::Editor::Gold; + case ChannelType::Normal: + default: + return I18N::Editor::Normal; + } +} + +// What the login screen writes after the group name and channel number +// (CServerListManager::InsertServer). +const char* GetLoginScreenSuffix(ChannelType type) +{ + switch (type) + { + case ChannelType::NonPvp: + return "(Non-PVP)"; + case ChannelType::GoldPvp: + return "(Gold PVP)"; + case ChannelType::Gold: + return "(Gold)"; + case ChannelType::Normal: + default: + return ""; + } +} + +// A grey line that wraps at the window's edge. +void RenderNote(const char* text) +{ + ImGui::PushTextWrapPos(0.0f); + ImGui::TextDisabled("%s", text); + ImGui::PopTextWrapPos(); +} + +void RenderError(const char* text) +{ + ImGui::PushStyleColor(ImGuiCol_Text, ErrorColor); + ImGui::PushTextWrapPos(0.0f); + ImGui::TextUnformatted(text); + ImGui::PopTextWrapPos(); + ImGui::PopStyleColor(); +} + +CMuServerListUI::SentChannels FindSentChannels() +{ + CMuServerListUI::SentChannels sent; + for (const CServerListManager::ReceivedServer& server : g_ServerListManager->GetReceivedServers()) + { + sent[server.connectIndex / Data::ServerList::MaxChannelsPerGroup] + [server.connectIndex % Data::ServerList::MaxChannelsPerGroup] = server.percent; + } + return sent; +} + +std::string ListServerIds(const std::map& channels, int groupId) +{ + std::string text; + for (const auto& [channel, load] : channels) + { + text += (text.empty() ? "" : ", ") + std::to_string(groupId * Data::ServerList::MaxChannelsPerGroup + channel); + } + return text; +} +} // namespace + +CMuServerListUI& CMuServerListUI::GetInstance() +{ + static CMuServerListUI instance; + return instance; +} + +void CMuServerListUI::Render(bool* open) +{ + if (!m_loaded) + { + LoadFile(); + } + char title[MuEditor::WindowIds::TitleCapacity]; + MuEditor::Layout::PrepareToolWindow(); + if (!ImGui::Begin( + MuEditor::WindowIds::Title(title, I18N::Editor::ServerList, MuEditor::WindowIds::ServerList), open, + ImGuiWindowFlags_NoCollapse | MuEditor::Layout::ToolWindowBarFlags(MuEditor::WindowIds::ServerList))) + { + ImGui::End(); + return; + } + MuEditor::Layout::RenderToolWindowBar(); + if (ImGui::IsWindowHovered(ImGuiHoveredFlags_ChildWindows | ImGuiHoveredFlags_AllowWhenBlockedByActiveItem)) + { + g_MuEditorCore.SetHoveringUI(true); + } + + m_sent = FindSentChannels(); + bool changed = RenderToolbar(); + RenderStatus(); + ImGui::Separator(); + + const ImVec2 listSize(GroupListWidth * g_MuEditorCore.GetUIScale(), 0.0f); + if (ImGui::BeginChild("##Groups", listSize, ImGuiChildFlags_Borders | ImGuiChildFlags_ResizeX)) + { + changed |= RenderGroupList(); + changed |= RenderListButtons(); + changed |= RenderGroupsNotInList(); + } + ImGui::EndChild(); + ImGui::SameLine(); + if (ImGui::BeginChild("##Group")) + { + changed |= RenderSelectedGroup(); + } + ImGui::EndChild(); + ImGui::End(); + + if (changed) + { + m_uncommitted = true; + ShowInGame(); + } + // A text field changes the groups with each key; its whole edit is one step. + if (m_uncommitted && !ImGui::IsAnyItemActive()) + { + m_edits.Commit(); + m_uncommitted = false; + } +} + +void CMuServerListUI::LoadFile() +{ + Data::ServerList::ServerListLoadResult loaded = + Data::ServerList::LoadServerListFile(Data::ServerList::ServerListFile); + m_edits.Reset(std::move(loaded.groups)); + m_issues = std::move(loaded.issues); + m_status.clear(); + m_statusIsError = false; + m_loaded = true; + m_uncommitted = false; + m_takenId = -1; + Select(m_edits.GetGroups().empty() ? -1 + : std::clamp(m_selected, 0, static_cast(m_edits.GetGroups().size()) - 1)); + ShowInGame(); +} + +void CMuServerListUI::Save() +{ + m_edits.Commit(); + m_issues.clear(); + if (!Data::ServerList::SaveServerListFile(Data::ServerList::ServerListFile, m_edits.GetGroups(), m_issues)) + { + m_status = I18N::Editor::NotSavedSeeTheProblemsBelow; + m_statusIsError = true; + return; + } + m_edits.MarkSaved(); + m_status = I18N::Format(I18N::Editor::SavedTo0, {Data::ServerList::ServerListFile}); + m_statusIsError = false; +} + +bool CMuServerListUI::RenderToolbar() +{ + if (ImGui::Button(I18N::Editor::Save)) + { + Save(); + } + ImGui::SameLine(); + bool changed = false; + if (ImGui::Button(I18N::Editor::Revert)) + { + LoadFile(); + } + if (ImGui::IsItemHovered()) + { + ImGui::SetTooltip( + "%s", + I18N::Format(I18N::Editor::Reads0AgainUnsavedChangesAreLost, {Data::ServerList::ServerListFile}).c_str()); + } + ImGui::SameLine(); + // Ctrl+Z and Ctrl+Y while this window has the focus; a text field being + // edited takes them first. + const bool canUndo = m_edits.CanUndo() || m_uncommitted; + ImGui::BeginDisabled(!canUndo); + const bool undo = ImGui::Button(I18N::Editor::Undo); + ImGui::EndDisabled(); + if (canUndo && (undo || ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Z))) + { + m_edits.Undo(); + changed = true; + } + ImGui::SameLine(); + const bool canRedo = m_edits.CanRedo(); + ImGui::BeginDisabled(!canRedo); + const bool redo = ImGui::Button(I18N::Editor::Redo); + ImGui::EndDisabled(); + if (canRedo && (redo || ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Y))) + { + m_edits.Redo(); + changed = true; + } + if (m_edits.HasUnsavedChanges()) + { + ImGui::SameLine(); + ImGui::TextUnformatted(I18N::Editor::UnsavedChanges); + } + if (changed) + { + m_takenId = -1; + Select(std::min(m_selected, static_cast(m_edits.GetGroups().size()) - 1)); + } + return changed; +} + +void CMuServerListUI::RenderStatus() +{ + RenderNote(I18N::Format(I18N::Editor::ServerListOrderNote, {Data::ServerList::ServerListFile}).c_str()); + if (!m_status.empty()) + { + if (m_statusIsError) + { + RenderError(m_status.c_str()); + } + else + { + ImGui::TextUnformatted(m_status.c_str()); + } + } + if (m_issues.empty()) + { + return; + } + ImGui::TextUnformatted(I18N::Format(I18N::Editor::ProblemsIn0, {Data::ServerList::ServerListFile}).c_str()); + for (const Data::Items::ItemDataIssue& issue : m_issues) + { + const std::string line = issue.field + ": " + issue.message; + if (issue.severity == Data::Items::ItemDataIssueSeverity::Error) + { + RenderError(line.c_str()); + } + else + { + RenderNote(line.c_str()); + } + } +} + +bool CMuServerListUI::RenderGroupList() +{ + const std::vector& groups = m_edits.GetGroups(); + for (int index = 0; index < static_cast(groups.size()); ++index) + { + const ServerGroup& group = groups[index]; + ImGui::PushID(index); + const std::string label = (group.name.empty() ? std::string("?") : group.name) + " (" + + std::to_string(group.id) + ")" + (group.testServer ? " *" : ""); + if (ImGui::Selectable(label.c_str(), index == m_selected, MuEditor::KeyboardSelection::RowFlags)) + { + Select(index); + } + ImGui::PopID(); + } + return false; +} + +bool CMuServerListUI::RenderListButtons() +{ + bool changed = false; + if (ImGui::Button(I18N::Editor::Add)) + { + if (const std::optional added = m_edits.AddGroup(I18N::Editor::NewGroup)) + { + Select(static_cast(*added)); + changed = true; + } + } + ImGui::SameLine(); + const bool hasSelection = m_selected >= 0; + ImGui::BeginDisabled(!hasSelection); + if (ImGui::Button(I18N::Editor::Remove)) + { + m_edits.RemoveGroup(static_cast(m_selected)); + Select(std::min(m_selected, static_cast(m_edits.GetGroups().size()) - 1)); + changed = true; + } + ImGui::EndDisabled(); + ImGui::BeginDisabled(!hasSelection || m_selected == 0); + if (ImGui::Button(I18N::Editor::MoveUp) && m_edits.MoveGroup(static_cast(m_selected), -1)) + { + Select(m_selected - 1); + changed = true; + } + ImGui::EndDisabled(); + ImGui::SameLine(); + ImGui::BeginDisabled(!hasSelection || m_selected + 1 >= static_cast(m_edits.GetGroups().size())); + if (ImGui::Button(I18N::Editor::MoveDown) && m_edits.MoveGroup(static_cast(m_selected), 1)) + { + Select(m_selected + 1); + changed = true; + } + ImGui::EndDisabled(); + return changed; +} + +bool CMuServerListUI::RenderGroupsNotInList() +{ + bool headerShown = false; + bool changed = false; + for (const auto& [groupId, channels] : m_sent) + { + if (m_edits.IsIdUsed(groupId)) + { + continue; + } + if (!headerShown) + { + ImGui::Separator(); + RenderNote(I18N::Editor::GroupsTheListDoesNotHave); + headerShown = true; + } + ImGui::PushID(groupId); + ImGui::TextUnformatted( + I18N::Format(I18N::Editor::Group0Servers1, {std::to_string(groupId), ListServerIds(channels, groupId)}) + .c_str()); + ImGui::SameLine(); + if (ImGui::SmallButton(I18N::Editor::Add)) + { + if (const std::optional added = m_edits.AddGroup(I18N::Editor::NewGroup, groupId)) + { + Select(static_cast(*added)); + changed = true; + } + } + ImGui::PopID(); + } + return changed; +} + +bool CMuServerListUI::RenderSelectedGroup() +{ + std::vector& groups = m_edits.EditGroups(); + if (m_selected < 0 || m_selected >= static_cast(groups.size())) + { + ImGui::TextDisabled("%s", I18N::Editor::PickAGroupInTheList); + return false; + } + ServerGroup& group = groups[static_cast(m_selected)]; + bool changed = RenderGroupId(group); + changed |= RenderGroupFields(group); + ImGui::Separator(); + changed |= RenderChannels(group); + return changed; +} + +bool CMuServerListUI::RenderGroupId(ServerGroup& group) +{ + int id = group.id; + ImGui::SetNextItemWidth(IdFieldWidth * g_MuEditorCore.GetUIScale()); + bool changed = false; + // Typed only: a step button would stop at an ID another group has. + if (ImGui::InputInt(I18N::Editor::GroupID, &id, 0, 0)) + { + id = std::clamp(id, 0, Data::ServerList::MaxServerGroupId); + // An ID another group has stays with it; the field says so. + const bool taken = id != group.id && m_edits.IsIdUsed(id); + m_takenId = taken ? id : -1; + changed = m_takenId < 0 && id != group.id; + if (changed) + { + group.id = id; + } + } + const int firstServer = group.id * Data::ServerList::MaxChannelsPerGroup; + ImGui::SameLine(); + ImGui::TextDisabled("%s", I18N::Format(I18N::Editor::Servers0To1, + {std::to_string(firstServer), + std::to_string(firstServer + Data::ServerList::MaxChannelsPerGroup - 1)}) + .c_str()); + if (m_takenId >= 0) + { + RenderError(I18N::Format(I18N::Editor::AnotherGroupHasTheID0, {std::to_string(m_takenId)}).c_str()); + } + return changed; +} + +bool CMuServerListUI::RenderGroupFields(ServerGroup& group) +{ + const float scale = g_MuEditorCore.GetUIScale(); + ImGui::SetNextItemWidth(TextFieldWidth * scale); + bool changed = false; + if (MuEditor::Items::InputString(I18N::Editor::Name, group.name)) + { + Data::ServerList::CutToCharacters(group.name, Data::ServerList::MaxServerGroupNameLength); + changed = true; + } + if (group.name.empty()) + { + RenderError(I18N::Editor::AGroupNeedsAName); + } + ImGui::SetNextItemWidth(TextFieldWidth * scale); + if (MuEditor::Items::InputString(I18N::Editor::Description, group.description)) + { + Data::ServerList::CutToCharacters(group.description, Data::ServerList::MaxServerGroupDescriptionLength); + changed = true; + } + RenderNote(I18N::Editor::DescriptionNote); + changed |= ImGui::Checkbox(I18N::Editor::TestServer, &group.testServer); + RenderNote(I18N::Editor::TestServerNote); + return changed; +} + +bool CMuServerListUI::RenderChannels(ServerGroup& group) +{ + constexpr ImGuiTableFlags TableFlags = + ImGuiTableFlags_RowBg | ImGuiTableFlags_BordersInnerV | ImGuiTableFlags_SizingFixedFit; + if (!ImGui::BeginTable("##Channels", ChannelTableColumns, TableFlags)) + { + return false; + } + ImGui::TableSetupColumn(I18N::Editor::Channel); + ImGui::TableSetupColumn(I18N::Editor::Type); + ImGui::TableSetupColumn(I18N::Editor::OnTheLoginScreen); + ImGui::TableSetupColumn(I18N::Editor::ConnectServer); + ImGui::TableHeadersRow(); + + const auto sentGroup = m_sent.find(group.id); + bool changed = false; + for (int channel = 0; channel < Data::ServerList::MaxChannelsPerGroup; ++channel) + { + ImGui::PushID(channel); + ImGui::TableNextRow(); + ImGui::TableNextColumn(); + ImGui::Text("%d", channel + 1); + + ImGui::TableNextColumn(); + ChannelType& type = group.channels[static_cast(channel)]; + ImGui::SetNextItemWidth(TypeComboWidth * g_MuEditorCore.GetUIScale()); + if (ImGui::BeginCombo("##Type", GetChannelTypeLabel(type))) + { + for (int value = 0; value < Data::ServerList::ChannelTypeCount; ++value) + { + const ChannelType option = static_cast(value); + if (ImGui::Selectable(GetChannelTypeLabel(option), option == type) && option != type) + { + type = option; + changed = true; + } + } + ImGui::EndCombo(); + } + + ImGui::TableNextColumn(); + ImGui::TextDisabled("%s-%d%s", group.name.c_str(), channel + 1, GetLoginScreenSuffix(type)); + + ImGui::TableNextColumn(); + if (sentGroup != m_sent.end()) + { + const auto load = sentGroup->second.find(channel); + if (load != sentGroup->second.end()) + { + ImGui::TextUnformatted(I18N::Format(I18N::Editor::Sent0Load, {std::to_string(load->second)}).c_str()); + } + } + ImGui::PopID(); + } + ImGui::EndTable(); + return changed; +} + +void CMuServerListUI::ShowInGame() +{ + g_ServerListManager->SetServerGroups(m_edits.GetGroups()); + // Only the login screen shows the groups; elsewhere the next list the + // connect server sends gets the new names. + if (SceneFlag != LOG_IN_SCENE) + { + return; + } + g_ServerListManager->RebuildServerGroups(); + if (g_ServerSelWin.IsVisible()) + { + g_ServerSelWin.UpdateDisplay(); + } +} + +void CMuServerListUI::Select(int index) +{ + if (index != m_selected) + { + m_takenId = -1; + } + m_selected = index; +} + +#endif // _EDITOR diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.h b/src/MuEditor/UI/ServerList/MuServerListUI.h new file mode 100644 index 0000000000..5e5b673c2a --- /dev/null +++ b/src/MuEditor/UI/ServerList/MuServerListUI.h @@ -0,0 +1,61 @@ +#pragma once + +#ifdef _EDITOR + +#include "ServerListEdits.h" + +#include "Data/GameData/ItemData/ItemDataIssue.h" + +#include +#include +#include + +// The Server List window: the server groups of the login screen +// (Data/Local/ServerList.json), their names, order and channel types. The login +// screen shows each change right away; Save writes the file. +class CMuServerListUI +{ +public: + // The load of each server the connect server last sent, by group ID and + // channel number - 1. + using SentChannels = std::map>; + + static CMuServerListUI& GetInstance(); + + void Render(bool* open); + +private: + void LoadFile(); + void Save(); + // Each Render* that changes the groups returns true. + bool RenderToolbar(); + void RenderStatus(); + bool RenderGroupList(); + bool RenderListButtons(); + bool RenderGroupsNotInList(); + bool RenderSelectedGroup(); + bool RenderGroupId(Data::ServerList::ServerGroup& group); + bool RenderGroupFields(Data::ServerList::ServerGroup& group); + bool RenderChannels(Data::ServerList::ServerGroup& group); + // The game names the servers of the connect server with the groups now. + void ShowInGame(); + void Select(int index); + + MuEditor::ServerList::ServerListEdits m_edits; + bool m_loaded = false; + // The fields changed the groups since the last undo step. + bool m_uncommitted = false; + int m_selected = -1; + // An ID typed for the selected group that another group has, or -1. + int m_takenId = -1; + // What the last read or save found, and what it said. + std::vector m_issues; + std::string m_status; + bool m_statusIsError = false; + // Found again each frame the window is drawn. + SentChannels m_sent; +}; + +#define g_MuServerListUI CMuServerListUI::GetInstance() + +#endif // _EDITOR diff --git a/src/MuEditor/UI/ServerList/ServerListEdits.cpp b/src/MuEditor/UI/ServerList/ServerListEdits.cpp new file mode 100644 index 0000000000..076d3c038c --- /dev/null +++ b/src/MuEditor/UI/ServerList/ServerListEdits.cpp @@ -0,0 +1,116 @@ +#include "stdafx.h" + +#ifdef _EDITOR + +#include "ServerListEdits.h" + +#include +#include + +namespace MuEditor::ServerList +{ +void ServerListEdits::Reset(std::vector groups) +{ + m_groups = std::move(groups); + m_committed = m_groups; + m_saved = m_groups; + m_undo.clear(); + m_redo.clear(); +} + +void ServerListEdits::Commit() +{ + if (m_groups == m_committed) + { + return; + } + m_undo.push_back(std::move(m_committed)); + if (m_undo.size() > MaxUndoSteps) + { + m_undo.erase(m_undo.begin()); + } + m_redo.clear(); + m_committed = m_groups; +} + +void ServerListEdits::Undo() +{ + Commit(); + if (m_undo.empty()) + { + return; + } + m_redo.push_back(std::move(m_groups)); + m_groups = std::move(m_undo.back()); + m_undo.pop_back(); + m_committed = m_groups; +} + +void ServerListEdits::Redo() +{ + Commit(); + if (m_redo.empty()) + { + return; + } + m_undo.push_back(std::move(m_groups)); + m_groups = std::move(m_redo.back()); + m_redo.pop_back(); + m_committed = m_groups; +} + +std::optional ServerListEdits::AddGroup(const std::string& name, std::optional id) +{ + if (!id || IsIdUsed(*id)) + { + id = FindFreeId(); + } + if (!id) + { + return std::nullopt; + } + ServerGroup group; + group.id = *id; + group.name = name; + m_groups.push_back(std::move(group)); + return m_groups.size() - 1; +} + +void ServerListEdits::RemoveGroup(size_t index) +{ + if (index < m_groups.size()) + { + m_groups.erase(m_groups.begin() + static_cast(index)); + } +} + +bool ServerListEdits::MoveGroup(size_t index, int direction) +{ + const long long target = static_cast(index) + direction; + if (index >= m_groups.size() || target < 0 || target >= static_cast(m_groups.size())) + { + return false; + } + std::swap(m_groups[index], m_groups[static_cast(target)]); + return true; +} + +bool ServerListEdits::IsIdUsed(int id) const +{ + return std::any_of(m_groups.begin(), m_groups.end(), [id](const ServerGroup& group) { return group.id == id; }); +} + +std::optional ServerListEdits::FindFreeId() const +{ + for (int id = 0; id <= Data::ServerList::MaxServerGroupId; ++id) + { + if (!IsIdUsed(id)) + { + return id; + } + } + return std::nullopt; +} +} // namespace MuEditor::ServerList + +#endif // _EDITOR diff --git a/src/MuEditor/UI/ServerList/ServerListEdits.h b/src/MuEditor/UI/ServerList/ServerListEdits.h new file mode 100644 index 0000000000..54ade85cf0 --- /dev/null +++ b/src/MuEditor/UI/ServerList/ServerListEdits.h @@ -0,0 +1,82 @@ +#pragma once + +#ifdef _EDITOR + +#include "Data/GameData/ServerListData/ServerGroup.h" + +#include +#include +#include +#include + +namespace MuEditor::ServerList +{ +using Data::ServerList::ServerGroup; + +// The server groups the Server List window changes: what the editor shows, +// what the file has, and an undo list for this session. +class ServerListEdits +{ +public: + static constexpr size_t MaxUndoSteps = 256; + + // The groups of the file, just read or saved: nothing to undo, nothing changed. + void Reset(std::vector groups); + + const std::vector& GetGroups() const + { + return m_groups; + } + // For the window's fields, which change the groups in place; Commit makes + // what they changed one undo step. + std::vector& EditGroups() + { + return m_groups; + } + // The groups changed since the last step become one undo step. + void Commit(); + + bool CanUndo() const + { + return !m_undo.empty(); + } + bool CanRedo() const + { + return !m_redo.empty(); + } + void Undo(); + void Redo(); + + // Whether the groups differ from the file. + bool HasUnsavedChanges() const + { + return m_groups != m_saved; + } + // The groups are what the file has now. + void MarkSaved() + { + m_saved = m_groups; + } + + // Adds a group named `name` with the smallest free ID, or `id` when given + // (and free), at the end of the list. Returns its index; nothing when no + // ID is free. + std::optional AddGroup(const std::string& name, std::optional id = std::nullopt); + void RemoveGroup(size_t index); + // Moves the group one place up (-1) or down (+1); false at the end of the list. + bool MoveGroup(size_t index, int direction); + + bool IsIdUsed(int id) const; + std::optional FindFreeId() const; + +private: + std::vector m_groups; + // The groups as the last undo step left them. + std::vector m_committed; + std::vector m_saved; + std::vector> m_undo; + std::vector> m_redo; +}; +} // namespace MuEditor::ServerList + +#endif // _EDITOR diff --git a/src/source/Network/Server/ServerListManager.cpp b/src/source/Network/Server/ServerListManager.cpp index a2813d6e65..79e5c8a8cd 100644 --- a/src/source/Network/Server/ServerListManager.cpp +++ b/src/source/Network/Server/ServerListManager.cpp @@ -49,6 +49,9 @@ CServerListManager* CServerListManager::GetInstance() void CServerListManager::Release() { +#ifdef _EDITOR + m_receivedServers.clear(); +#endif auto iterServerGroup = m_mapServerGroup.begin(); for (; iterServerGroup != m_mapServerGroup.end(); iterServerGroup++) { @@ -83,6 +86,20 @@ void CServerListManager::SetServerGroups(std::span servers = m_receivedServers; + const int totalServer = m_iTotalServer; + Release(); + SetTotalServer(totalServer); + for (const ReceivedServer& server : servers) + { + InsertServerGroup(server.connectIndex, server.percent); + } +} +#endif + const SServerGroupInfo* CServerListManager::GetServerGroupInfoInScript(int iServerGroupIndex) { ServerListScriptMap::const_iterator iter = m_mapServerListScript.find(iServerGroupIndex); @@ -94,6 +111,9 @@ const SServerGroupInfo* CServerListManager::GetServerGroupInfoInScript(int iServ void CServerListManager::InsertServerGroup(int iConnectIndex, int iServerPercent) { +#ifdef _EDITOR + m_receivedServers.push_back({iConnectIndex, iServerPercent}); +#endif CServerGroup* pServerGroup = NULL; auto iterServerGroup = m_mapServerGroup.begin(); diff --git a/src/source/Network/Server/ServerListManager.h b/src/source/Network/Server/ServerListManager.h index 80e97ae7ed..e15c4c5bef 100644 --- a/src/source/Network/Server/ServerListManager.h +++ b/src/source/Network/Server/ServerListManager.h @@ -8,6 +8,7 @@ #include #include +#include // A group of Data/Local/ServerList.json, with its place in the list. struct SServerGroupInfo @@ -51,6 +52,23 @@ class CServerListManager void SetTotalServer(int iTotalServer); int GetTotalServer(); +#ifdef _EDITOR + // A server of the last list the connect server sent. + struct ReceivedServer + { + int connectIndex = 0; + int percent = 0; + }; + + const std::vector& GetReceivedServers() const + { + return m_receivedServers; + } + // Builds the groups of the last list the connect server sent again, with + // the groups SetServerGroups set since. + void RebuildServerGroups(); +#endif + protected: const SServerGroupInfo* GetServerGroupInfoInScript(int iServerGroupIndex); bool MakeServerGroup(IN int iServerGroupIndex, OUT CServerGroup* pServerGroup); @@ -67,6 +85,9 @@ class CServerListManager protected: ServerListScriptMap m_mapServerListScript; +#ifdef _EDITOR + std::vector m_receivedServers; +#endif }; #define g_ServerListManager CServerListManager::GetInstance() diff --git a/tests/data/CMakeLists.txt b/tests/data/CMakeLists.txt index a1f2494cc1..3be93c0cda 100644 --- a/tests/data/CMakeLists.txt +++ b/tests/data/CMakeLists.txt @@ -38,6 +38,7 @@ target_compile_definitions(light_mood_data_tests PRIVATE mu_add_test( NAME server_list_data_tests SOURCES + test_server_list_edits.cpp test_server_list_json.cpp LINK_LIBS MuClient diff --git a/tests/data/test_server_list_edits.cpp b/tests/data/test_server_list_edits.cpp new file mode 100644 index 0000000000..a61a26a9e9 --- /dev/null +++ b/tests/data/test_server_list_edits.cpp @@ -0,0 +1,191 @@ +#include "stdafx.h" + +#include "doctest.h" + +#ifdef _EDITOR + +#include "UI/ServerList/ServerListEdits.h" + +#include +#include + +using Data::ServerList::ChannelType; +using MuEditor::ServerList::ServerGroup; +using MuEditor::ServerList::ServerListEdits; + +namespace +{ +ServerGroup Group(int id, const std::string& name) +{ + ServerGroup group; + group.id = id; + group.name = name; + return group; +} + +ServerListEdits ThreeGroups() +{ + ServerListEdits edits; + edits.Reset({Group(0, "Valhalla"), Group(1, "Helheim"), Group(2, "Midgard")}); + return edits; +} + +std::vector Names(const ServerListEdits& edits) +{ + std::vector names; + for (const ServerGroup& group : edits.GetGroups()) + { + names.push_back(group.name); + } + return names; +} +} // namespace + +TEST_CASE("ServerListEdits: a reset has nothing to undo and nothing unsaved") +{ + const ServerListEdits edits = ThreeGroups(); + + CHECK_FALSE(edits.CanUndo()); + CHECK_FALSE(edits.CanRedo()); + CHECK_FALSE(edits.HasUnsavedChanges()); +} + +TEST_CASE("ServerListEdits: what changed since the last step is one undo step") +{ + ServerListEdits edits = ThreeGroups(); + edits.EditGroups()[0].name = "V"; + edits.EditGroups()[0].name = "Val"; + edits.EditGroups()[0].channels[0] = ChannelType::Gold; + edits.Commit(); + + CHECK(edits.CanUndo()); + CHECK(edits.HasUnsavedChanges()); + + edits.Undo(); + CHECK(edits.GetGroups()[0].name == "Valhalla"); + CHECK(edits.GetGroups()[0].channels[0] == ChannelType::Normal); + CHECK_FALSE(edits.CanUndo()); + CHECK_FALSE(edits.HasUnsavedChanges()); + + edits.Redo(); + CHECK(edits.GetGroups()[0].name == "Val"); + CHECK(edits.GetGroups()[0].channels[0] == ChannelType::Gold); +} + +TEST_CASE("ServerListEdits: undo also takes back a change not yet committed") +{ + ServerListEdits edits = ThreeGroups(); + edits.EditGroups()[1].name = "Hel"; + + edits.Undo(); + + CHECK(edits.GetGroups()[1].name == "Helheim"); + CHECK(edits.CanRedo()); +} + +TEST_CASE("ServerListEdits: a new change drops what could be redone") +{ + ServerListEdits edits = ThreeGroups(); + edits.RemoveGroup(2); + edits.Commit(); + edits.Undo(); + REQUIRE(edits.CanRedo()); + + edits.MoveGroup(0, 1); + edits.Commit(); + + CHECK_FALSE(edits.CanRedo()); +} + +TEST_CASE("ServerListEdits: committing without a change adds no step") +{ + ServerListEdits edits = ThreeGroups(); + edits.Commit(); + CHECK_FALSE(edits.CanUndo()); + + edits.EditGroups()[0].name = "X"; + edits.EditGroups()[0].name = "Valhalla"; + edits.Commit(); + CHECK_FALSE(edits.CanUndo()); +} + +TEST_CASE("ServerListEdits: the undo list keeps the last steps") +{ + ServerListEdits edits = ThreeGroups(); + for (size_t step = 0; step < ServerListEdits::MaxUndoSteps + 10; ++step) + { + edits.EditGroups()[0].name = "Step " + std::to_string(step); + edits.Commit(); + } + size_t undone = 0; + while (edits.CanUndo()) + { + edits.Undo(); + ++undone; + } + CHECK(undone == ServerListEdits::MaxUndoSteps); + CHECK(edits.GetGroups()[0].name == "Step 9"); +} + +TEST_CASE("ServerListEdits: saved groups are not unsaved changes") +{ + ServerListEdits edits = ThreeGroups(); + edits.EditGroups()[2].testServer = true; + edits.Commit(); + edits.MarkSaved(); + + CHECK_FALSE(edits.HasUnsavedChanges()); + // Undo stays across a save; the file then differs again. + edits.Undo(); + CHECK(edits.HasUnsavedChanges()); +} + +TEST_CASE("ServerListEdits: a new group gets the smallest free ID at the end") +{ + ServerListEdits edits; + edits.Reset({Group(0, "A"), Group(2, "B")}); + + const std::optional added = edits.AddGroup("New"); + + REQUIRE(added == size_t{2}); + CHECK(edits.GetGroups()[2].id == 1); + CHECK(edits.GetGroups()[2].name == "New"); +} + +TEST_CASE("ServerListEdits: a new group gets the wanted ID when it is free") +{ + ServerListEdits edits = ThreeGroups(); + + REQUIRE(edits.AddGroup("Sent", 7).has_value()); + CHECK(edits.GetGroups().back().id == 7); + + REQUIRE(edits.AddGroup("Taken", 1).has_value()); + CHECK(edits.GetGroups().back().id == 3); +} + +TEST_CASE("ServerListEdits: remove and move") +{ + ServerListEdits edits = ThreeGroups(); + + CHECK(edits.MoveGroup(2, -1)); + CHECK(Names(edits) == std::vector{"Valhalla", "Midgard", "Helheim"}); + CHECK_FALSE(edits.MoveGroup(0, -1)); + CHECK_FALSE(edits.MoveGroup(2, 1)); + CHECK_FALSE(edits.MoveGroup(5, 1)); + + edits.RemoveGroup(0); + CHECK(Names(edits) == std::vector{"Midgard", "Helheim"}); + edits.RemoveGroup(9); + CHECK(edits.GetGroups().size() == 2); +} + +TEST_CASE("ServerListEdits: IDs in use") +{ + const ServerListEdits edits = ThreeGroups(); + + CHECK(edits.IsIdUsed(1)); + CHECK_FALSE(edits.IsIdUsed(3)); + CHECK(edits.FindFreeId() == 3); +} + +#endif // _EDITOR From 7a07304f16e3ea4069fedc912c42dd68ba63cb3d Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Thu, 22 Oct 2026 23:42:48 +0200 Subject: [PATCH 05/26] docs: the server list file and its editor window How the login screen turns the connect server's server IDs into groups and channels, the fields of ServerList.json, the Server List window, and what changed from the original ServerList.bmd. --- docs/server-list.md | 101 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 docs/server-list.md diff --git a/docs/server-list.md b/docs/server-list.md new file mode 100644 index 0000000000..3f348807f7 --- /dev/null +++ b/docs/server-list.md @@ -0,0 +1,101 @@ +# Server List + +The server groups on the login screen (Valhalla, Helheim, ...) and the names +and colors of their channels come from `Data/Local/ServerList.json`. It +replaced the encrypted `Data/Local/ServerList.bmd` of the original client. The +game reads it at start; with errors in it the game shows them and stops, as it +does for broken item data. All problems are also written to the log. + +## How the login screen uses it + +The connect server sends the IDs of its game servers and their load. The +client splits each ID into a group and a channel: + +- group = server ID / 20 +- channel = server ID % 20 + 1 + +So servers 0 to 19 are the channels 1 to 20 of group 0, servers 20 to 39 those +of group 1, and so on. On OpenMU the ID is the game server's *Server ID* in the +admin panel; to put a server into a second group, give it an ID from 20 to 39. + +- **Groups:** one button per group, in the order of the list. A server whose + group is not in the list is not shown at all. +- **Test server:** a group marked as test server is shown at the top. The + login screen shows only the first one. +- **Channels:** each channel is shown as `-`, followed by + its type: + + | Type | Shown as | Button | + |-----------|------------------------|---------| + | `normal` | `Valhalla-1` | gray | + | `nonPvp` | `Valhalla-1(Non-PVP)` | default | + | `goldPvp` | `Valhalla-1(Gold PVP)` | orange | + | `gold` | `Valhalla-1(Gold)` | orange | + + The type only names and colors the channel; whether PvP is on is decided by + the server. +- **PvP notice:** the warning about crowded PvP servers is shown for a group + that has no `nonPvp` and no `gold` channel. +- **Description:** shown below the channels while the group is selected. + +## The file + +```json +{ + "formatVersion": 1, + "groups": [ + { + "id": 0, + "name": "Valhalla", + "channels": {"1": "gold", "5": "goldPvp", "6": "nonPvp"} + }, + { + "id": 1, + "name": "Helheim", + "description": "PvP everywhere" + }, + { + "id": 9, + "name": "Test", + "testServer": true + } + ] +} +``` + +| Field | Meaning | +|---------------|--------------------------------------------------------------------------| +| `id` | The group: server ID / 20, from 0 to 3276. Each group once. | +| `name` | The button text and the start of each channel name; up to 32 characters. | +| `description` | Optional; up to 254 characters. | +| `testServer` | Optional; `true` puts the group at the top. | +| `channels` | Optional; the type of a channel by its number (1 to 20). Channels not listed are `normal`. | + +A name or description that is too long is cut (a warning in the log). + +## Editing in MU Editor + +**Windows → Server List** shows the groups in their order on the left and the +selected group on the right: its ID (with the server IDs it covers), name, +description, the test server mark, and a type for each of the 20 channels with +the name the login screen gives it. + +- The login screen shows each change right away, while the window is open. +- **Add**, **Remove**, **Move up** and **Move down** change the list. +- When the connect server sent servers of groups the list does not have, the + window lists them with their server IDs; **Add** there adds a group with that + ID. The channel table shows which channels the connect server sent, with + their load. +- **Save** writes `Data/Local/ServerList.json` in the folder the game runs + from. It refuses a list with errors (a group without a name, an ID used + twice) and lists them. +- **Revert** reads the file again; **Undo** and **Redo** (Ctrl+Z, Ctrl+Y) + work through the changes of the session. + +## Compared to the original client + +`ServerList.bmd` held the same groups, encrypted, with channel types for the +first 15 channels only and a left or right position for each button. The login +screen now lists all groups in one column, so only the test server position +is kept. The original loader read each group's description but never kept it, +so the login screen showed none; the JSON description is shown. From 959096bbb6bfea848434d0a21716fa7fb56e0075 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:15:51 +0200 Subject: [PATCH 06/26] fix(editor): the Server List window's undo keys only while the keyboard is the editor's ImGui keeps the Server List window focused after the keyboard has gone back to the game. Ctrl+Z typed into the login screen's ID or password field then also undid the last server list change and built the login screen's groups again. The keys now need the editor's keyboard (IsKeyboardWithEditor), as the item tools and the map editor check, and Ctrl+Shift+Z redoes too, as there. Review of Mosch0512/MuMain#128 (required). --- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index 04f99ff647..f67c2f99e6 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -218,13 +218,16 @@ bool CMuServerListUI::RenderToolbar() I18N::Format(I18N::Editor::Reads0AgainUnsavedChangesAreLost, {Data::ServerList::ServerListFile}).c_str()); } ImGui::SameLine(); - // Ctrl+Z and Ctrl+Y while this window has the focus; a text field being - // edited takes them first. + // Ctrl+Z and Ctrl+Y (or Ctrl+Shift+Z) while this window has the focus and + // the keyboard is the editor's: ImGui keeps the focus here when the game + // takes the keyboard back, e.g. to type into the login screen. A text field + // being edited takes them first. + const bool editorKeys = g_MuEditorCore.IsKeyboardWithEditor(); const bool canUndo = m_edits.CanUndo() || m_uncommitted; ImGui::BeginDisabled(!canUndo); const bool undo = ImGui::Button(I18N::Editor::Undo); ImGui::EndDisabled(); - if (canUndo && (undo || ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Z))) + if (canUndo && (undo || (editorKeys && ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Z)))) { m_edits.Undo(); changed = true; @@ -234,7 +237,9 @@ bool CMuServerListUI::RenderToolbar() ImGui::BeginDisabled(!canRedo); const bool redo = ImGui::Button(I18N::Editor::Redo); ImGui::EndDisabled(); - if (canRedo && (redo || ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Y))) + const bool redoKeys = editorKeys && (ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Y) || + ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiMod_Shift | ImGuiKey_Z)); + if (canRedo && (redo || redoKeys)) { m_edits.Redo(); changed = true; From be743f25b805877029097f20c696b2ae539c052d Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:16:17 +0200 Subject: [PATCH 07/26] fix(editor): Save stays off after the server list was read with errors The reader leaves out every group with an error, and a file it rejects as a whole (a JSON syntax error, formatVersion 2) gives no groups at all. Save then wrote that shorter list, which reads back without errors, so a group mistyped by hand while the game ran, or the whole list, was gone from ServerList.json without a warning. After a read with errors the groups are not complete (ServerListEdits:: IsComplete): Save is off and the window says why, until the file is fixed and Revert reads it again. Review of Mosch0512/MuMain#128 (required). --- src/Localization/Editor.de.resx | 3 +++ src/Localization/Editor.en.resx | 3 +++ src/MuEditor/UI/ServerList/MuServerListUI.cpp | 17 ++++++++++++++++- src/MuEditor/UI/ServerList/ServerListEdits.cpp | 3 ++- src/MuEditor/UI/ServerList/ServerListEdits.h | 12 ++++++++++-- tests/data/test_server_list_edits.cpp | 15 +++++++++++++++ 6 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/Localization/Editor.de.resx b/src/Localization/Editor.de.resx index 2ac85aa705..02492f9d20 100644 --- a/src/Localization/Editor.de.resx +++ b/src/Localization/Editor.de.resx @@ -1939,4 +1939,7 @@ Die Reihenfolge der Liste ist die der Schaltflächen auf dem Anmeldebildschirm, der jede Änderung sofort zeigt. Speichern schreibt {0}. + + Die Datei hatte beim Lesen Fehler, daher enthält die Liste nur die Gruppen, die gelesen werden konnten. Speichern bleibt aus, bis die Datei korrigiert ist und Zurücksetzen sie neu einliest; es würde die anderen Gruppen aus der Datei entfernen. + diff --git a/src/Localization/Editor.en.resx b/src/Localization/Editor.en.resx index 59a4a4dff9..2d30b64303 100644 --- a/src/Localization/Editor.en.resx +++ b/src/Localization/Editor.en.resx @@ -1960,4 +1960,7 @@ The order of the list is the order of the buttons on the login screen, which shows each change right away. Save writes {0}. + + The file had errors when it was read, so the list has only the groups that could be read. Save is off until the file is fixed and Revert reads it again; it would remove the other groups from the file. + diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index f67c2f99e6..3f86d1a60c 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -172,7 +172,7 @@ void CMuServerListUI::LoadFile() { Data::ServerList::ServerListLoadResult loaded = Data::ServerList::LoadServerListFile(Data::ServerList::ServerListFile); - m_edits.Reset(std::move(loaded.groups)); + m_edits.Reset(std::move(loaded.groups), !Data::Items::HasErrors(loaded.issues)); m_issues = std::move(loaded.issues); m_status.clear(); m_statusIsError = false; @@ -186,6 +186,10 @@ void CMuServerListUI::LoadFile() void CMuServerListUI::Save() { + if (!m_edits.IsComplete()) + { + return; + } m_edits.Commit(); m_issues.clear(); if (!Data::ServerList::SaveServerListFile(Data::ServerList::ServerListFile, m_edits.GetGroups(), m_issues)) @@ -201,10 +205,17 @@ void CMuServerListUI::Save() bool CMuServerListUI::RenderToolbar() { + // A save after a read with errors would remove the groups the read left out. + ImGui::BeginDisabled(!m_edits.IsComplete()); if (ImGui::Button(I18N::Editor::Save)) { Save(); } + ImGui::EndDisabled(); + if (!m_edits.IsComplete() && ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled)) + { + ImGui::SetTooltip("%s", I18N::Editor::SaveOffAfterReadErrors); + } ImGui::SameLine(); bool changed = false; if (ImGui::Button(I18N::Editor::Revert)) @@ -271,6 +282,10 @@ void CMuServerListUI::RenderStatus() ImGui::TextUnformatted(m_status.c_str()); } } + if (!m_edits.IsComplete()) + { + RenderError(I18N::Editor::SaveOffAfterReadErrors); + } if (m_issues.empty()) { return; diff --git a/src/MuEditor/UI/ServerList/ServerListEdits.cpp b/src/MuEditor/UI/ServerList/ServerListEdits.cpp index 076d3c038c..59902ed3c4 100644 --- a/src/MuEditor/UI/ServerList/ServerListEdits.cpp +++ b/src/MuEditor/UI/ServerList/ServerListEdits.cpp @@ -9,8 +9,9 @@ namespace MuEditor::ServerList { -void ServerListEdits::Reset(std::vector groups) +void ServerListEdits::Reset(std::vector groups, bool complete) { + m_complete = complete; m_groups = std::move(groups); m_committed = m_groups; m_saved = m_groups; diff --git a/src/MuEditor/UI/ServerList/ServerListEdits.h b/src/MuEditor/UI/ServerList/ServerListEdits.h index 54ade85cf0..a18c060a96 100644 --- a/src/MuEditor/UI/ServerList/ServerListEdits.h +++ b/src/MuEditor/UI/ServerList/ServerListEdits.h @@ -20,8 +20,9 @@ class ServerListEdits public: static constexpr size_t MaxUndoSteps = 256; - // The groups of the file, just read or saved: nothing to undo, nothing changed. - void Reset(std::vector groups); + // The groups of the file, just read or saved: nothing to undo, nothing + // changed. `complete` is false when the read left groups out (errors). + void Reset(std::vector groups, bool complete = true); const std::vector& GetGroups() const { @@ -57,6 +58,12 @@ class ServerListEdits { m_saved = m_groups; } + // Whether the groups are all the file has. After a read with errors they + // are not, and a save would remove the groups left out from the file. + bool IsComplete() const + { + return m_complete; + } // Adds a group named `name` with the smallest free ID, or `id` when given // (and free), at the end of the list. Returns its index; nothing when no @@ -76,6 +83,7 @@ class ServerListEdits std::vector m_saved; std::vector> m_undo; std::vector> m_redo; + bool m_complete = true; }; } // namespace MuEditor::ServerList diff --git a/tests/data/test_server_list_edits.cpp b/tests/data/test_server_list_edits.cpp index a61a26a9e9..1f346350b6 100644 --- a/tests/data/test_server_list_edits.cpp +++ b/tests/data/test_server_list_edits.cpp @@ -140,6 +140,21 @@ TEST_CASE("ServerListEdits: saved groups are not unsaved changes") CHECK(edits.HasUnsavedChanges()); } +TEST_CASE("ServerListEdits: groups from a read with errors are not complete until a clean read") +{ + ServerListEdits edits; + edits.Reset({Group(0, "Valhalla")}, false); + CHECK_FALSE(edits.IsComplete()); + + edits.EditGroups()[0].name = "Asgard"; + edits.Commit(); + edits.MarkSaved(); + CHECK_FALSE(edits.IsComplete()); + + edits.Reset({Group(0, "Valhalla"), Group(1, "Helheim")}); + CHECK(edits.IsComplete()); +} + TEST_CASE("ServerListEdits: a new group gets the smallest free ID at the end") { ServerListEdits edits; From 13f2a43763a77408ed02ca3a7a9f1a4a4e87bd2b Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:16:26 +0200 Subject: [PATCH 08/26] fix(editor): the server select window lets go of a group the editor rebuilt The Server List window builds the login screen's groups again after each change, which deletes the old CServerGroup objects. The server select window was refreshed only while it was visible; after a server was picked it is hidden and kept m_pSelectServerGroup on a deleted group, and the new groups had no button position. Nothing used the pointer before the next UpdateDisplay, but a path that shows the window without one (MsgWin's RECEIVE_JOIN_SERVER_WAITING) would have read freed memory on a click. UpdateDisplay now runs after every rebuild; it does nothing visible while the window is hidden. Review of Mosch0512/MuMain#128. --- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index 3f86d1a60c..dc90e49029 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -537,10 +537,9 @@ void CMuServerListUI::ShowInGame() return; } g_ServerListManager->RebuildServerGroups(); - if (g_ServerSelWin.IsVisible()) - { - g_ServerSelWin.UpdateDisplay(); - } + // Also while the server select window is hidden (after a server was + // picked): it keeps a pointer to the open group, which the rebuild deleted. + g_ServerSelWin.UpdateDisplay(); } void CMuServerListUI::Select(int index) From f86a5352eff091d57febf300edc80bdb4855c591 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:16:51 +0200 Subject: [PATCH 09/26] fix(editor): the login screen keeps its open group when the Server List window reorders The server select window remembers the open group by its button position, and the rebuild after an edit numbers the buttons again in the new list order. With Valhalla open, moving Midgard above it in the Server List window opened Midgard on the login screen, with its channels; Remove and the Test server box did the same. The window now finds the open group again by its ID after a rebuild (CServerSelWin::GetOpenGroupId and ShowRebuiltGroups, editor builds only). A group that is gone leaves none open. Review of Mosch0512/MuMain#128. --- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 5 ++- src/source/UI/Windows/ServerSelWin.cpp | 33 +++++++++++++++++++ src/source/UI/Windows/ServerSelWin.h | 9 +++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index dc90e49029..9ea00a2ed8 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -536,10 +536,13 @@ void CMuServerListUI::ShowInGame() { return; } + // The login screen knows its open group by its button, which a group + // moved or removed in the list changes; it is found again by its ID. + const int openGroupId = g_ServerSelWin.GetOpenGroupId(); g_ServerListManager->RebuildServerGroups(); // Also while the server select window is hidden (after a server was // picked): it keeps a pointer to the open group, which the rebuild deleted. - g_ServerSelWin.UpdateDisplay(); + g_ServerSelWin.ShowRebuiltGroups(openGroupId); } void CMuServerListUI::Select(int index) diff --git a/src/source/UI/Windows/ServerSelWin.cpp b/src/source/UI/Windows/ServerSelWin.cpp index 026c9dd0d2..81c5399031 100644 --- a/src/source/UI/Windows/ServerSelWin.cpp +++ b/src/source/UI/Windows/ServerSelWin.cpp @@ -339,6 +339,39 @@ bool CServerSelWin::Render() // Called only at the point of an actual state change, not polled from Render() every frame -- // this window's state only ever changes from its own click handlers. +#ifdef _EDITOR +int CServerSelWin::GetOpenGroupId() const +{ + if (m_iSelectServerBtnIndex == -1) + return -1; + + // By its button, not m_pSelectServerGroup: that can be left on a deleted group. + const CServerGroup* pServerGroup = g_ServerListManager->GetServerGroupByBtnPos(m_iSelectServerBtnIndex); + return pServerGroup != nullptr ? pServerGroup->m_iServerIndex : -1; +} + +void CServerSelWin::ShowRebuiltGroups(int openGroupId) +{ + // The first pass gives the rebuilt groups their buttons. + m_iSelectServerBtnIndex = -1; + UpdateDisplay(); + if (openGroupId < 0) + return; + + CServerGroup* pServerGroup = nullptr; + g_ServerListManager->SetFirst(); + while (g_ServerListManager->GetNext(pServerGroup)) + { + if (pServerGroup->m_iServerIndex == openGroupId && pServerGroup->m_iBtnPos >= 0) + { + m_iSelectServerBtnIndex = pServerGroup->m_iBtnPos; + UpdateDisplay(); + return; + } + } +} +#endif + void CServerSelWin::SyncRmlModel() { if (!m_RmlView.Document()) return; diff --git a/src/source/UI/Windows/ServerSelWin.h b/src/source/UI/Windows/ServerSelWin.h index c5977da19d..a97b761d23 100644 --- a/src/source/UI/Windows/ServerSelWin.h +++ b/src/source/UI/Windows/ServerSelWin.h @@ -40,6 +40,15 @@ class CServerSelWin : public mu::ui::window::CObject // such group or server, or when the server is full. bool SelectServer(const wchar_t* groupName, int serverIndex); +#ifdef _EDITOR + // The ID (CServerGroup::m_iServerIndex) of the open group, or -1; for the + // editor, before it builds the groups again. + int GetOpenGroupId() const; + // After the editor built the groups again: shows them, with the group of + // that ID open again wherever it is in the list now, or none when it is gone. + void ShowRebuiltGroups(int openGroupId); +#endif + // mu::ui::window::IObject bool Render() override; bool Update() override; From ff0d39b7430f26d7137b129494801288e3f5d078 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:18:07 +0200 Subject: [PATCH 10/26] fix(server-list): name and description limits count UTF-16 units The limits counted characters. A character outside the Basic Multilingual Plane (most emoji) takes two wchar_t on Windows, so a 32-character name could be 64 wchar_t long, and the screenshot message (SceneManager.cpp) formats the group name and the character name into wchar_t[64] without a size: 24 emoji and a 10-letter name wrote past it. The description's 254 characters could also be more than the 254 wchar_t the group keeps, and that cut could split a surrogate pair. CutToUtf16Length counts an emoji as two and never cuts inside a character; the reader and the Server List window use it for both limits. Review of Mosch0512/MuMain#128. --- docs/server-list.md | 3 +- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 4 +-- .../GameData/ServerListData/ServerGroup.h | 5 ++- .../ServerListData/ServerListJson.cpp | 34 ++++++++++++++----- .../GameData/ServerListData/ServerListJson.h | 7 ++-- tests/data/test_server_list_json.cpp | 27 ++++++++++++++- 6 files changed, 65 insertions(+), 15 deletions(-) diff --git a/docs/server-list.md b/docs/server-list.md index 3f348807f7..a619cd10e5 100644 --- a/docs/server-list.md +++ b/docs/server-list.md @@ -71,7 +71,8 @@ admin panel; to put a server into a second group, give it an ID from 20 to 39. | `testServer` | Optional; `true` puts the group at the top. | | `channels` | Optional; the type of a channel by its number (1 to 20). Channels not listed are `normal`. | -A name or description that is too long is cut (a warning in the log). +A name or description that is too long is cut (a warning in the log). Most +emoji count as two characters, as the client stores them on Windows. ## Editing in MU Editor diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index 9ea00a2ed8..0355347ceb 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -450,7 +450,7 @@ bool CMuServerListUI::RenderGroupFields(ServerGroup& group) bool changed = false; if (MuEditor::Items::InputString(I18N::Editor::Name, group.name)) { - Data::ServerList::CutToCharacters(group.name, Data::ServerList::MaxServerGroupNameLength); + Data::ServerList::CutToUtf16Length(group.name, Data::ServerList::MaxServerGroupNameLength); changed = true; } if (group.name.empty()) @@ -460,7 +460,7 @@ bool CMuServerListUI::RenderGroupFields(ServerGroup& group) ImGui::SetNextItemWidth(TextFieldWidth * scale); if (MuEditor::Items::InputString(I18N::Editor::Description, group.description)) { - Data::ServerList::CutToCharacters(group.description, Data::ServerList::MaxServerGroupDescriptionLength); + Data::ServerList::CutToUtf16Length(group.description, Data::ServerList::MaxServerGroupDescriptionLength); changed = true; } RenderNote(I18N::Editor::DescriptionNote); diff --git a/src/source/Data/GameData/ServerListData/ServerGroup.h b/src/source/Data/GameData/ServerListData/ServerGroup.h index 97611a2b12..15fd979478 100644 --- a/src/source/Data/GameData/ServerListData/ServerGroup.h +++ b/src/source/Data/GameData/ServerListData/ServerGroup.h @@ -25,7 +25,10 @@ inline constexpr int ChannelTypeCount = 4; inline constexpr int MaxChannelsPerGroup = MAX_SERVER_PER_GROUP; // Server IDs are 16 bit. inline constexpr int MaxServerGroupId = 0xFFFF / MAX_SERVER_PER_GROUP; -// The width of the group buttons on the login screen. +// In UTF-16 units, as the client's wide strings hold them on Windows (an emoji +// takes two): the name goes into fixed wide buffers, e.g. the screenshot +// message's 64 with the character name. 32 is also the width of the group +// buttons on the login screen. inline constexpr int MaxServerGroupNameLength = 32; inline constexpr int MaxServerGroupDescriptionLength = MAX_TEXT_LENGTH - 1; diff --git a/src/source/Data/GameData/ServerListData/ServerListJson.cpp b/src/source/Data/GameData/ServerListData/ServerListJson.cpp index 8d28bf3b4b..28a58e7a95 100644 --- a/src/source/Data/GameData/ServerListData/ServerListJson.cpp +++ b/src/source/Data/GameData/ServerListData/ServerListJson.cpp @@ -30,9 +30,14 @@ constexpr const char* TemporaryExtension = ".tmp"; // In the order of the ChannelType values. constexpr std::array ChannelTypeNames = {"normal", "nonPvp", "goldPvp", "gold"}; -// The bits of a UTF-8 byte that tell whether it continues a character. +// The bits of a UTF-8 byte that tell whether it continues a character, and +// whether it starts one of four bytes. constexpr unsigned char Utf8ContinuationMask = 0xC0; constexpr unsigned char Utf8ContinuationBits = 0x80; +constexpr unsigned char Utf8FourByteMask = 0xF8; +constexpr unsigned char Utf8FourByteBits = 0xF0; +// A character outside the Basic Multilingual Plane takes two UTF-16 units. +constexpr size_t SurrogatePairUnits = 2; class IssueList { @@ -70,6 +75,13 @@ bool IsContinuationByte(char byte) return (static_cast(byte) & Utf8ContinuationMask) == Utf8ContinuationBits; } +// The UTF-16 units of the character a UTF-8 lead byte starts: a four-byte +// character (most emoji) is outside the Basic Multilingual Plane. +size_t Utf16UnitsOf(char leadByte) +{ + return (static_cast(leadByte) & Utf8FourByteMask) == Utf8FourByteBits ? SurrogatePairUnits : 1; +} + bool ReadId(const OrderedJson& json, const std::string& field, int& id, IssueList& issues) { const auto value = json.find(IdKey); @@ -85,8 +97,8 @@ bool ReadId(const OrderedJson& json, const std::string& field, int& id, IssueLis return true; } -// A text of the group; one longer than maxCharacters is cut. -bool ReadText(const OrderedJson& json, const std::string& field, const char* key, size_t maxCharacters, +// A text of the group; one longer than maxUnits UTF-16 units is cut. +bool ReadText(const OrderedJson& json, const std::string& field, const char* key, size_t maxUnits, std::string& text, IssueList& issues) { const auto value = json.find(key); @@ -100,9 +112,10 @@ bool ReadText(const OrderedJson& json, const std::string& field, const char* key return false; } text = value->get(); - if (CutToCharacters(text, maxCharacters)) + if (CutToUtf16Length(text, maxUnits)) { - issues.Warning(Field(field, key), "longer than " + std::to_string(maxCharacters) + " characters, cut"); + issues.Warning(Field(field, key), + "longer than " + std::to_string(maxUnits) + " characters (an emoji counts as two), cut"); } return true; } @@ -264,12 +277,17 @@ OrderedJson WriteGroup(const ServerGroup& group) } } // namespace -bool CutToCharacters(std::string& text, size_t maxCharacters) +bool CutToUtf16Length(std::string& text, size_t maxUnits) { - size_t characters = 0; + size_t units = 0; for (size_t i = 0; i < text.size(); ++i) { - if (!IsContinuationByte(text[i]) && characters++ == maxCharacters) + if (IsContinuationByte(text[i])) + { + continue; + } + units += Utf16UnitsOf(text[i]); + if (units > maxUnits) { text.resize(i); return true; diff --git a/src/source/Data/GameData/ServerListData/ServerListJson.h b/src/source/Data/GameData/ServerListData/ServerListJson.h index 22cb53708d..069c9568ab 100644 --- a/src/source/Data/GameData/ServerListData/ServerListJson.h +++ b/src/source/Data/GameData/ServerListData/ServerListJson.h @@ -36,14 +36,17 @@ std::optional FindChannelType(std::string_view name); // Reads the groups of the file into `groups`, in their order. A group with an // error is left out; a name or description that is too long is cut (a warning). +// Lengths are counted in UTF-16 units, as the client's wide strings hold them on +// Windows: an emoji counts as two. void ReadServerListJson(std::string_view text, const std::string& source, std::vector& groups, std::vector& issues); // The text of the file with these groups, in their order. std::string WriteServerListJson(std::span groups); -// Cuts UTF-8 text after maxCharacters characters; false when it is not longer. -bool CutToCharacters(std::string& text, size_t maxCharacters); +// Cuts UTF-8 text to at most maxUnits UTF-16 units, never inside a +// character; false when it is not longer. +bool CutToUtf16Length(std::string& text, size_t maxUnits); struct ServerListLoadResult { diff --git a/tests/data/test_server_list_json.cpp b/tests/data/test_server_list_json.cpp index a1cd9b993a..c9a94bddba 100644 --- a/tests/data/test_server_list_json.cpp +++ b/tests/data/test_server_list_json.cpp @@ -195,7 +195,7 @@ TEST_CASE("ServerListJson: a name or description that is too long is cut") CHECK(read.groups[0].description.size() == 254); } -TEST_CASE("ServerListJson: names are counted in characters, not bytes") +TEST_CASE("ServerListJson: names are counted in UTF-16 units, not bytes") { // 32 two-byte characters fit; the cut never splits one. std::string name; @@ -209,6 +209,31 @@ TEST_CASE("ServerListJson: names are counted in characters, not bytes") CHECK(read.groups[0].name == name.substr(0, 64)); } +TEST_CASE("ServerListJson: an emoji counts as two UTF-16 units") +{ + const std::string emoji = "\xF0\x9F\x98\x80"; // U+1F600, a surrogate pair in UTF-16 + std::string emojis; + for (int i = 0; i < 17; ++i) + { + emojis += emoji; + } + const ReadResult onlyEmoji = Read(GroupFile(R"({ "id": 0, "name": ")" + emojis + R"(" })")); + REQUIRE(onlyEmoji.groups.size() == 1); + CHECK(onlyEmoji.groups[0].name == emojis.substr(0, 16 * emoji.size())); + + // 31 letters and an emoji are 33 units: the emoji goes, whole. + const std::string mixed = std::string(31, 'x') + emoji; + const ReadResult cut = Read(GroupFile(R"({ "id": 0, "name": ")" + mixed + R"(" })")); + REQUIRE(cut.groups.size() == 1); + CHECK(cut.groups[0].name == std::string(31, 'x')); + CHECK(HasIssueAbout(cut.issues, "groups[0].name", "an emoji counts as two")); + + // 30 letters and an emoji are exactly 32. + std::string fits = std::string(30, 'x') + emoji; + CHECK_FALSE(Data::ServerList::CutToUtf16Length(fits, Data::ServerList::MaxServerGroupNameLength)); + CHECK(fits == std::string(30, 'x') + emoji); +} + TEST_CASE("ServerListJson: unknown fields and a second test server are warnings") { const ReadResult read = Read(R"({ "formatVersion": 1, "color": "red", "groups": [ From 0a3a37fb3097c003567f920f819c3cd8a47d1cba Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:18:28 +0200 Subject: [PATCH 11/26] docs(server-list): a test server group stays at its place in the list The docs, a comment and the editor's note said a test server group is shown first. CServerSelWin::UpdateDisplay adds the groups in list order and server_select.rml keeps that order: testServer only hides every test server group after the first, it does not move one. The four texts say so now. Review of Mosch0512/MuMain#128. --- docs/server-list.md | 13 +++++++------ src/Localization/Editor.de.resx | 2 +- src/Localization/Editor.en.resx | 2 +- .../Data/GameData/ServerListData/ServerGroup.h | 3 ++- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/server-list.md b/docs/server-list.md index a619cd10e5..61cc2690e9 100644 --- a/docs/server-list.md +++ b/docs/server-list.md @@ -20,8 +20,8 @@ admin panel; to put a server into a second group, give it an ID from 20 to 39. - **Groups:** one button per group, in the order of the list. A server whose group is not in the list is not shown at all. -- **Test server:** a group marked as test server is shown at the top. The - login screen shows only the first one. +- **Test server:** of the groups marked as test server, the login screen + shows only the first, at its place in the list; the others are hidden. - **Channels:** each channel is shown as `-`, followed by its type: @@ -68,7 +68,7 @@ admin panel; to put a server into a second group, give it an ID from 20 to 39. | `id` | The group: server ID / 20, from 0 to 3276. Each group once. | | `name` | The button text and the start of each channel name; up to 32 characters. | | `description` | Optional; up to 254 characters. | -| `testServer` | Optional; `true` puts the group at the top. | +| `testServer` | Optional; `true` marks a test server group: only the first is shown. | | `channels` | Optional; the type of a channel by its number (1 to 20). Channels not listed are `normal`. | A name or description that is too long is cut (a warning in the log). Most @@ -96,7 +96,8 @@ the name the login screen gives it. ## Compared to the original client `ServerList.bmd` held the same groups, encrypted, with channel types for the -first 15 channels only and a left or right position for each button. The login -screen now lists all groups in one column, so only the test server position -is kept. The original loader read each group's description but never kept it, +first 15 channels only and a left, right or center position for each button. +The original screen drew the center one, a test server, as a button of its +own; the login screen now lists all groups in one column in the list's order, +so only "test server" is kept, for the rule that only the first is shown. The original loader read each group's description but never kept it, so the login screen showed none; the JSON description is shown. diff --git a/src/Localization/Editor.de.resx b/src/Localization/Editor.de.resx index 02492f9d20..4980429737 100644 --- a/src/Localization/Editor.de.resx +++ b/src/Localization/Editor.de.resx @@ -1898,7 +1898,7 @@ Testserver - Steht oben; der Anmeldebildschirm zeigt nur die erste Testserver-Gruppe. + Der Anmeldebildschirm zeigt nur die erste Testserver-Gruppe, an ihrem Platz in der Liste. Kanal diff --git a/src/Localization/Editor.en.resx b/src/Localization/Editor.en.resx index 2d30b64303..234cd84180 100644 --- a/src/Localization/Editor.en.resx +++ b/src/Localization/Editor.en.resx @@ -1919,7 +1919,7 @@ Test server - Shown first; the login screen shows only the first test server group. + The login screen shows only the first test server group, at its place in the list. Channel diff --git a/src/source/Data/GameData/ServerListData/ServerGroup.h b/src/source/Data/GameData/ServerListData/ServerGroup.h index 15fd979478..67429f03c5 100644 --- a/src/source/Data/GameData/ServerListData/ServerGroup.h +++ b/src/source/Data/GameData/ServerListData/ServerGroup.h @@ -39,7 +39,8 @@ struct ServerGroup int id = 0; std::string name; // UTF-8 std::string description; // UTF-8, shown below the channels of the selected group - // A test server group: shown first, and only one of them. + // A test server group: the login screen shows only the first one, at its + // place in the list. bool testServer = false; // Index: channel number - 1. std::array channels{}; From e2be00567a8958f76c03fbe060eb6f558dc5b808 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:18:38 +0200 Subject: [PATCH 12/26] fix(editor): a "##" in a group name shows in the Server List window The group list used the name as the Selectable's label, so ImGui read a "##" in it as the start of the ID: a group named "PvP##EU" showed as "PvP", without its ID and test server mark. The row's label is now only an ID (the rows are told apart by PushID already) and the name is drawn as plain text over it. Review of Mosch0512/MuMain#128. --- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index 0355347ceb..0c9dcfe173 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -314,10 +314,14 @@ bool CMuServerListUI::RenderGroupList() ImGui::PushID(index); const std::string label = (group.name.empty() ? std::string("?") : group.name) + " (" + std::to_string(group.id) + ")" + (group.testServer ? " *" : ""); - if (ImGui::Selectable(label.c_str(), index == m_selected, MuEditor::KeyboardSelection::RowFlags)) + // The name is drawn as plain text: as the row's label, a "##" in it + // would start the ImGui ID and hide the rest. + const ImVec2 textPosition = ImGui::GetCursorScreenPos(); + if (ImGui::Selectable("##Group", index == m_selected, MuEditor::KeyboardSelection::RowFlags)) { Select(index); } + ImGui::GetWindowDrawList()->AddText(textPosition, ImGui::GetColorU32(ImGuiCol_Text), label.c_str()); ImGui::PopID(); } return false; From 7f9d274ef1330cf19b863e346ae3717da7b39363 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:22:18 +0200 Subject: [PATCH 13/26] test(server-list): the login screen's groups as CServerListManager builds them The tests covered the JSON reader and writer and the editor's edits, but not the game side, where the list becomes the login screen's groups: - the list order becomes m_iSequence, - a server of a group the list does not have is not shown, - all 20 channel types reach m_abyNonPvpServer, m_bPvPServer and the channel names ("Valhalla-1(Gold) ..."), - description and test server (SBP_CENTER) reach the group, - new groups replace the old ones, and a rebuild (editor builds) names the servers last sent with the new groups. Review of Mosch0512/MuMain#128. --- tests/data/CMakeLists.txt | 1 + tests/data/test_server_list_manager.cpp | 209 ++++++++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 tests/data/test_server_list_manager.cpp diff --git a/tests/data/CMakeLists.txt b/tests/data/CMakeLists.txt index 3be93c0cda..ca97d4c797 100644 --- a/tests/data/CMakeLists.txt +++ b/tests/data/CMakeLists.txt @@ -40,6 +40,7 @@ mu_add_test( SOURCES test_server_list_edits.cpp test_server_list_json.cpp + test_server_list_manager.cpp LINK_LIBS MuClient ) diff --git a/tests/data/test_server_list_manager.cpp b/tests/data/test_server_list_manager.cpp new file mode 100644 index 0000000000..e87237f2d8 --- /dev/null +++ b/tests/data/test_server_list_manager.cpp @@ -0,0 +1,209 @@ +#include "stdafx.h" + +#include "doctest.h" + +#include "I18N/All.h" +#include "Network/Server/ServerListManager.h" + +#include +#include + +using Data::ServerList::ChannelType; +using Data::ServerList::ServerGroup; + +namespace +{ +// The shared instance's constructor is protected; each test gets a manager of its own. +class TestServerListManager : public CServerListManager +{ +public: + TestServerListManager() = default; +}; + +// The load the connect server sends; the login screen's text for it (InsertServer). +constexpr int LowLoad = 10; +constexpr int LowLoadText = 562; + +ServerGroup Group(int id, const std::string& name) +{ + ServerGroup group; + group.id = id; + group.name = name; + return group; +} + +int ServerId(int groupId, int channel) +{ + return groupId * MAX_SERVER_PER_GROUP + channel - 1; +} + +std::vector GroupsInOrder(CServerListManager& manager) +{ + std::vector groups; + CServerGroup* group = nullptr; + manager.SetFirst(); + while (manager.GetNext(group)) + { + groups.push_back(group); + } + return groups; +} + +std::wstring ChannelName(const wchar_t* groupAndChannel) +{ + return std::wstring(groupAndChannel) + L" " + I18N::Game::Lookup(LowLoadText); +} +} // namespace + +TEST_CASE("ServerListManager: the list order is the order of the groups") +{ + TestServerListManager manager; + manager.SetServerGroups(std::vector{Group(2, "Midgard"), Group(0, "Valhalla"), Group(1, "Helheim")}); + for (const int groupId : {0, 1, 2}) + { + manager.InsertServerGroup(ServerId(groupId, 1), LowLoad); + } + + const std::vector groups = GroupsInOrder(manager); + REQUIRE(groups.size() == 3); + CHECK(std::wstring(groups[0]->m_szName) == L"Midgard"); + CHECK(groups[0]->m_iSequence == 1); + CHECK(groups[0]->m_iServerIndex == 2); + CHECK(std::wstring(groups[1]->m_szName) == L"Valhalla"); + CHECK(groups[1]->m_iSequence == 2); + CHECK(std::wstring(groups[2]->m_szName) == L"Helheim"); + CHECK(groups[2]->m_iSequence == 3); +} + +TEST_CASE("ServerListManager: a server of a group the list does not have is not shown") +{ + TestServerListManager manager; + manager.SetServerGroups(std::vector{Group(0, "Valhalla")}); + + manager.InsertServerGroup(ServerId(3, 1), LowLoad); + CHECK(manager.GetServerGroupSize() == 0); + + manager.InsertServerGroup(ServerId(0, 2), LowLoad); + CHECK(manager.GetServerGroupSize() == 1); +} + +TEST_CASE("ServerListManager: the servers of a group are its channels") +{ + TestServerListManager manager; + manager.SetServerGroups(std::vector{Group(1, "Helheim")}); + manager.InsertServerGroup(ServerId(1, 1), LowLoad); + manager.InsertServerGroup(ServerId(1, 20), LowLoad); + + const std::vector groups = GroupsInOrder(manager); + REQUIRE(groups.size() == 1); + REQUIRE(groups[0]->GetServerSize() == 2); + CHECK(groups[0]->GetServerInfo(0)->m_iIndex == 1); + CHECK(groups[0]->GetServerInfo(0)->m_iConnectIndex == ServerId(1, 1)); + CHECK(groups[0]->GetServerInfo(1)->m_iIndex == 20); + CHECK(groups[0]->GetServerInfo(1)->m_iConnectIndex == ServerId(1, 20)); +} + +TEST_CASE("ServerListManager: the channel types reach the channels and their names") +{ + ServerGroup valhalla = Group(0, "Valhalla"); + valhalla.channels[0] = ChannelType::Gold; + valhalla.channels[1] = ChannelType::GoldPvp; + valhalla.channels[2] = ChannelType::NonPvp; + valhalla.channels[19] = ChannelType::Gold; + TestServerListManager manager; + manager.SetServerGroups(std::vector{valhalla}); + for (const int channel : {1, 2, 3, 4, 20}) + { + manager.InsertServerGroup(ServerId(0, channel), LowLoad); + } + + const std::vector groups = GroupsInOrder(manager); + REQUIRE(groups.size() == 1); + CServerGroup* group = groups[0]; + for (int channel = 0; channel < MAX_SERVER_PER_GROUP; ++channel) + { + CAPTURE(channel); + CHECK(group->m_abyNonPvpServer[channel] == static_cast(valhalla.channels[channel])); + } + // A group with a non-PvP channel shows no PvP notice. + CHECK_FALSE(group->m_bPvPServer); + + REQUIRE(group->GetServerSize() == 5); + CHECK(group->GetServerInfo(0)->m_byNonPvP == static_cast(ChannelType::Gold)); + CHECK(std::wstring(group->GetServerInfo(0)->m_bName) == ChannelName(L"Valhalla-1(Gold)")); + CHECK(std::wstring(group->GetServerInfo(1)->m_bName) == ChannelName(L"Valhalla-2(Gold PVP)")); + CHECK(std::wstring(group->GetServerInfo(2)->m_bName) == ChannelName(L"Valhalla-3(Non-PVP)")); + CHECK(std::wstring(group->GetServerInfo(3)->m_bName) == ChannelName(L"Valhalla-4")); + CHECK(std::wstring(group->GetServerInfo(4)->m_bName) == ChannelName(L"Valhalla-20(Gold)")); +} + +TEST_CASE("ServerListManager: a group without a non-PvP channel shows the PvP notice") +{ + ServerGroup helheim = Group(1, "Helheim"); + helheim.channels[0] = ChannelType::GoldPvp; + TestServerListManager manager; + manager.SetServerGroups(std::vector{helheim}); + manager.InsertServerGroup(ServerId(1, 1), LowLoad); + + const std::vector groups = GroupsInOrder(manager); + REQUIRE(groups.size() == 1); + CHECK(groups[0]->m_bPvPServer); +} + +TEST_CASE("ServerListManager: description and test server reach the group") +{ + ServerGroup test = Group(9, "Test"); + test.description = "Only for the team"; + test.testServer = true; + TestServerListManager manager; + manager.SetServerGroups(std::vector{Group(0, "Valhalla"), test}); + manager.InsertServerGroup(ServerId(0, 1), LowLoad); + manager.InsertServerGroup(ServerId(9, 1), LowLoad); + + const std::vector groups = GroupsInOrder(manager); + REQUIRE(groups.size() == 2); + CHECK(groups[0]->m_iWidthPos == CServerGroup::SBP_LEFT); + CHECK(std::wstring(groups[0]->m_szDescription).empty()); + CHECK(groups[1]->m_iWidthPos == CServerGroup::SBP_CENTER); + CHECK(std::wstring(groups[1]->m_szDescription) == L"Only for the team"); +} + +TEST_CASE("ServerListManager: new groups replace the old ones") +{ + TestServerListManager manager; + manager.SetServerGroups(std::vector{Group(0, "Valhalla"), Group(1, "Helheim")}); + manager.SetServerGroups(std::vector{Group(1, "Asgard")}); + + manager.InsertServerGroup(ServerId(0, 1), LowLoad); + manager.InsertServerGroup(ServerId(1, 1), LowLoad); + + const std::vector groups = GroupsInOrder(manager); + REQUIRE(groups.size() == 1); + CHECK(std::wstring(groups[0]->m_szName) == L"Asgard"); +} + +#ifdef _EDITOR +TEST_CASE("ServerListManager: a rebuild names the servers last sent with the new groups") +{ + TestServerListManager manager; + manager.SetServerGroups(std::vector{Group(0, "Valhalla")}); + manager.SetTotalServer(3); + manager.InsertServerGroup(ServerId(0, 1), LowLoad); + manager.InsertServerGroup(ServerId(0, 2), LowLoad); + manager.InsertServerGroup(ServerId(1, 1), LowLoad); + REQUIRE(manager.GetReceivedServers().size() == 3); + + manager.SetServerGroups(std::vector{Group(1, "Helheim"), Group(0, "Asgard")}); + manager.RebuildServerGroups(); + + CHECK(manager.GetReceivedServers().size() == 3); + CHECK(manager.GetTotalServer() == 3); + const std::vector groups = GroupsInOrder(manager); + REQUIRE(groups.size() == 2); + CHECK(std::wstring(groups[0]->m_szName) == L"Helheim"); + CHECK(groups[0]->GetServerSize() == 1); + CHECK(std::wstring(groups[1]->m_szName) == L"Asgard"); + REQUIRE(groups[1]->GetServerSize() == 2); + CHECK(std::wstring(groups[1]->GetServerInfo(1)->m_bName) == ChannelName(L"Asgard-2")); +} +#endif From 5dbfd65b533f445c3d8a0425bdd9e509bb9bea43 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:24:29 +0200 Subject: [PATCH 14/26] refactor(server-list): one function for the channel suffixes of the login screen The Server List window had a copy of the four suffixes InsertServer writes after "-" ((Non-PVP), (Gold PVP), (Gold)), so a change on one side would have left the window's "On the login screen" column showing something else. GetChannelSuffix, next to the channel types, gives them to both. InsertServer formats the name with it in one call instead of a switch over four formats; the channel names stay the same, as the manager tests of the last commit check. Review of Mosch0512/MuMain#128. --- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 20 +------------ .../GameData/ServerListData/ServerGroup.h | 17 +++++++++++ .../Network/Server/ServerListManager.cpp | 29 +++++-------------- 3 files changed, 25 insertions(+), 41 deletions(-) diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index 0c9dcfe173..1263f1df6c 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -50,24 +50,6 @@ const char* GetChannelTypeLabel(ChannelType type) } } -// What the login screen writes after the group name and channel number -// (CServerListManager::InsertServer). -const char* GetLoginScreenSuffix(ChannelType type) -{ - switch (type) - { - case ChannelType::NonPvp: - return "(Non-PVP)"; - case ChannelType::GoldPvp: - return "(Gold PVP)"; - case ChannelType::Gold: - return "(Gold)"; - case ChannelType::Normal: - default: - return ""; - } -} - // A grey line that wraps at the window's edge. void RenderNote(const char* text) { @@ -514,7 +496,7 @@ bool CMuServerListUI::RenderChannels(ServerGroup& group) } ImGui::TableNextColumn(); - ImGui::TextDisabled("%s-%d%s", group.name.c_str(), channel + 1, GetLoginScreenSuffix(type)); + ImGui::TextDisabled("%s-%d%s", group.name.c_str(), channel + 1, Data::ServerList::GetChannelSuffix(type)); ImGui::TableNextColumn(); if (sentGroup != m_sent.end()) diff --git a/src/source/Data/GameData/ServerListData/ServerGroup.h b/src/source/Data/GameData/ServerListData/ServerGroup.h index 67429f03c5..3bcc5c2daa 100644 --- a/src/source/Data/GameData/ServerListData/ServerGroup.h +++ b/src/source/Data/GameData/ServerListData/ServerGroup.h @@ -20,6 +20,23 @@ enum class ChannelType : std::uint8_t inline constexpr int ChannelTypeCount = 4; +// What the login screen writes after "-" for the type. +constexpr const char* GetChannelSuffix(ChannelType type) +{ + switch (type) + { + case ChannelType::NonPvp: + return "(Non-PVP)"; + case ChannelType::GoldPvp: + return "(Gold PVP)"; + case ChannelType::Gold: + return "(Gold)"; + case ChannelType::Normal: + default: + return ""; + } +} + // A server sends its ID; the group is ID / MAX_SERVER_PER_GROUP and the // channel ID % MAX_SERVER_PER_GROUP + 1. inline constexpr int MaxChannelsPerGroup = MAX_SERVER_PER_GROUP; diff --git a/src/source/Network/Server/ServerListManager.cpp b/src/source/Network/Server/ServerListManager.cpp index 79e5c8a8cd..e9b4ba0668 100644 --- a/src/source/Network/Server/ServerListManager.cpp +++ b/src/source/Network/Server/ServerListManager.cpp @@ -14,6 +14,7 @@ #include #include #include +#include namespace { @@ -196,28 +197,12 @@ void CServerListManager::InsertServer(CServerGroup* pServerGroup, int iConnectIn iTextIndex = 562; } - switch (pServerInfo->m_byNonPvP) - { - case 0: - mu_swprintf(pServerInfo->m_bName, L"%ls-%d %ls", pServerGroup->m_szName, - pServerInfo->m_iIndex, I18N::Game::Lookup(iTextIndex)); - break; - - case 1: - mu_swprintf(pServerInfo->m_bName, L"%ls-%d(Non-PVP) %ls", pServerGroup->m_szName, - pServerInfo->m_iIndex, I18N::Game::Lookup(iTextIndex)); - break; - - case 2: - mu_swprintf(pServerInfo->m_bName, L"%ls-%d(Gold PVP) %ls", pServerGroup->m_szName, - pServerInfo->m_iIndex, I18N::Game::Lookup(iTextIndex)); - break; - - case 3: - mu_swprintf(pServerInfo->m_bName, L"%ls-%d(Gold) %ls", pServerGroup->m_szName, - pServerInfo->m_iIndex, I18N::Game::Lookup(iTextIndex)); - break; - } + // The suffix is ASCII, so each char is its wchar_t. + const std::string_view suffix = + Data::ServerList::GetChannelSuffix(static_cast(pServerInfo->m_byNonPvP)); + const std::wstring wideSuffix(suffix.begin(), suffix.end()); + mu_swprintf(pServerInfo->m_bName, L"%ls-%d%ls %ls", pServerGroup->m_szName, pServerInfo->m_iIndex, + wideSuffix.c_str(), I18N::Game::Lookup(iTextIndex)); pServerGroup->InsertServerInfo(pServerInfo); } From e8638afe8993923506b568caf6c88893d38bf288 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:24:48 +0200 Subject: [PATCH 15/26] refactor(server-list): the group name is copied with wcsncpy_s CopyText repeated what wcsncpy_s with _TRUNCATE does on every platform (the CRT on Windows, Core/Platform/SecureCrt.h elsewhere), as TextLineWrap.cpp already uses it. Review of Mosch0512/MuMain#128. --- src/source/Network/Server/ServerListManager.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/source/Network/Server/ServerListManager.cpp b/src/source/Network/Server/ServerListManager.cpp index e9b4ba0668..12458dd5d6 100644 --- a/src/source/Network/Server/ServerListManager.cpp +++ b/src/source/Network/Server/ServerListManager.cpp @@ -10,8 +10,6 @@ #include "Data/DataHandler/DataIssueReport.h" #include "Data/GameData/ServerListData/ServerListJson.h" -#include -#include #include #include #include @@ -21,13 +19,6 @@ namespace constexpr const char* ServerListName = "server list"; // The bit of a channel type that turns PvP off. constexpr BYTE NonPvpBit = 0x01; - -template void CopyText(wchar_t (&destination)[Capacity], const std::wstring& source) -{ - const size_t length = std::min(source.size(), Capacity - 1); - std::wmemcpy(destination, source.c_str(), length); - destination[length] = L'\0'; -} } // namespace CServerListManager::CServerListManager() @@ -158,8 +149,10 @@ bool CServerListManager::MakeServerGroup(IN int iServerGroupIndex, OUT CServerGr return false; const Data::ServerList::ServerGroup& group = pServerGroupInfo->group; - CopyText(pServerGroup->m_szName, Core::Text::FromUtf8(group.name)); - CopyText(pServerGroup->m_szDescription, Core::Text::FromUtf8(group.description)); + const std::wstring name = Core::Text::FromUtf8(group.name); + const std::wstring description = Core::Text::FromUtf8(group.description); + wcsncpy_s(pServerGroup->m_szName, name.c_str(), _TRUNCATE); + wcsncpy_s(pServerGroup->m_szDescription, description.c_str(), _TRUNCATE); pServerGroup->m_iSequence = pServerGroupInfo->sequence; pServerGroup->m_iWidthPos = group.testServer ? CServerGroup::SBP_CENTER : CServerGroup::SBP_LEFT; pServerGroup->m_iServerIndex = iServerGroupIndex; From 50d4b480073a7f7cdfb5dbc23cb6b25c369cecad Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:25:05 +0200 Subject: [PATCH 16/26] perf(editor): the Server List window maps the connect server's channels only when they change FindSentChannels built nested maps from every server the connect server sent on each frame the window was drawn, while that list only changes when a new one arrives. CServerListManager counts the changes of the received servers (editor builds only), and the window builds its maps again only when the count moved. Review of Mosch0512/MuMain#128. --- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 6 +++++- src/MuEditor/UI/ServerList/MuServerListUI.h | 4 +++- src/source/Network/Server/ServerListManager.cpp | 2 ++ src/source/Network/Server/ServerListManager.h | 7 +++++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index 1263f1df6c..bd86f37c4c 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -116,7 +116,11 @@ void CMuServerListUI::Render(bool* open) g_MuEditorCore.SetHoveringUI(true); } - m_sent = FindSentChannels(); + if (m_sentVersion != g_ServerListManager->GetReceivedServersVersion()) + { + m_sent = FindSentChannels(); + m_sentVersion = g_ServerListManager->GetReceivedServersVersion(); + } bool changed = RenderToolbar(); RenderStatus(); ImGui::Separator(); diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.h b/src/MuEditor/UI/ServerList/MuServerListUI.h index 5e5b673c2a..2193cad9aa 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.h +++ b/src/MuEditor/UI/ServerList/MuServerListUI.h @@ -52,8 +52,10 @@ class CMuServerListUI std::vector m_issues; std::string m_status; bool m_statusIsError = false; - // Found again each frame the window is drawn. + // Found again when the connect server sent a new list + // (CServerListManager::GetReceivedServersVersion). SentChannels m_sent; + int m_sentVersion = -1; }; #define g_MuServerListUI CMuServerListUI::GetInstance() diff --git a/src/source/Network/Server/ServerListManager.cpp b/src/source/Network/Server/ServerListManager.cpp index 12458dd5d6..d2e4fb866d 100644 --- a/src/source/Network/Server/ServerListManager.cpp +++ b/src/source/Network/Server/ServerListManager.cpp @@ -43,6 +43,7 @@ void CServerListManager::Release() { #ifdef _EDITOR m_receivedServers.clear(); + ++m_receivedServersVersion; #endif auto iterServerGroup = m_mapServerGroup.begin(); for (; iterServerGroup != m_mapServerGroup.end(); iterServerGroup++) @@ -105,6 +106,7 @@ void CServerListManager::InsertServerGroup(int iConnectIndex, int iServerPercent { #ifdef _EDITOR m_receivedServers.push_back({iConnectIndex, iServerPercent}); + ++m_receivedServersVersion; #endif CServerGroup* pServerGroup = NULL; diff --git a/src/source/Network/Server/ServerListManager.h b/src/source/Network/Server/ServerListManager.h index e15c4c5bef..3eb43c367f 100644 --- a/src/source/Network/Server/ServerListManager.h +++ b/src/source/Network/Server/ServerListManager.h @@ -64,6 +64,12 @@ class CServerListManager { return m_receivedServers; } + // Changes each time the received servers change, so a view of them is + // built again only then. + int GetReceivedServersVersion() const + { + return m_receivedServersVersion; + } // Builds the groups of the last list the connect server sent again, with // the groups SetServerGroups set since. void RebuildServerGroups(); @@ -87,6 +93,7 @@ class CServerListManager ServerListScriptMap m_mapServerListScript; #ifdef _EDITOR std::vector m_receivedServers; + int m_receivedServersVersion = 0; #endif }; From f8fe9cd047f5ca0337233df0e94909fd756dd5ec Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:28:43 +0200 Subject: [PATCH 17/26] feat(editor): the editor bar's Undo and Redo also undo the Server List window The Server List window had Undo and Redo buttons of its own beside the editor bar's, which only undid item edits. The bar's buttons now act on the tool worked in last: a tool window that has the focus makes its history the bar's (MuEditor::UndoTargets), the item tools until another tool is used. The item edit history and the Server List window are the two UndoTargets; the window's own buttons are gone, its Ctrl+Z and Ctrl+Y stay. --- docs/server-list.md | 7 +- src/MuEditor/README.md | 5 +- src/MuEditor/UI/Common/MuEditorUI.cpp | 3 +- src/MuEditor/UI/Common/UndoTarget.cpp | 54 ++++++++++ src/MuEditor/UI/Common/UndoTarget.h | 39 ++++++++ src/MuEditor/UI/ItemTools/ItemToolWindow.cpp | 74 ++++++++------ src/MuEditor/UI/ItemTools/ItemToolWindow.h | 11 ++- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 99 ++++++++++++------- src/MuEditor/UI/ServerList/MuServerListUI.h | 23 ++++- 9 files changed, 239 insertions(+), 76 deletions(-) create mode 100644 src/MuEditor/UI/Common/UndoTarget.cpp create mode 100644 src/MuEditor/UI/Common/UndoTarget.h diff --git a/docs/server-list.md b/docs/server-list.md index 61cc2690e9..cf58d46b6b 100644 --- a/docs/server-list.md +++ b/docs/server-list.md @@ -90,8 +90,11 @@ the name the login screen gives it. - **Save** writes `Data/Local/ServerList.json` in the folder the game runs from. It refuses a list with errors (a group without a name, an ID used twice) and lists them. -- **Revert** reads the file again; **Undo** and **Redo** (Ctrl+Z, Ctrl+Y) - work through the changes of the session. +- **Revert** reads the file again. +- **Undo** and **Redo** in the editor bar (or Ctrl+Z and Ctrl+Y in the window) + work through the changes of the session. The bar's buttons act on the tool + used last: after a click into the Server List window they undo server list + changes, after a click into an item tool item edits. ## Compared to the original client diff --git a/src/MuEditor/README.md b/src/MuEditor/README.md index bd6a59bccd..1e7b5ee24e 100644 --- a/src/MuEditor/README.md +++ b/src/MuEditor/README.md @@ -22,6 +22,7 @@ The MU Editor is a C++ ImGui-based in-game editor that provides real-time game e - **MuEditor.h/cpp** - Main editor class, initialization, lifecycle management - **MuEditorUI.h/cpp** - The editor window and its menu bar, or the Open Editor button while closed - **MuEditorLayout.h/cpp** - Where the editor window first opens, and the first layout of its tool windows +- **UndoTarget.h/cpp** - The editor bar's Undo and Redo, for the tool worked in last (the item tools or the Server List window) - **MuEditorWindowIds.h** - The names the tool windows are docked by - **MuEditorPlatformWindows.h/cpp** - The editor's own OS windows on Windows, Linux (X11) and macOS - **MuEditorCursor.h/cpp** - The OS cursor's shape over the editor off Windows @@ -44,7 +45,7 @@ stores it when a field is left. - **MuItemGlowColorsUI** - The glow color list: new colors, changed values and unused colors removed (GlowColorDialog, which the model tool's color fields open too) - **MuItemFormulasUI** - The item stat file: drop level bonus, default, profiles, level bonus tables and profile rules, changed on a copy and applied after the list of the items it reaches (ItemStatTermFields, ItemStatFileChange) - **MuItemChecksUI** - The checks of a save, each leading to its item and field (ItemCheckTargets) -- **ItemToolWindow** - What every item tool window shares: its title, the selected item, and Undo and Redo (shown in the editor's bar) +- **ItemToolWindow** - What every item tool window shares: its title, the selected item, and the item edit history as the editor bar's undo target ### Item Actions (UI/ItemEditor) - **ItemEditorActions.h/cpp** - Save, import from bmd and the exports @@ -55,7 +56,7 @@ stores it when a field is left. ### Server List (UI/ServerList) - **MuServerListUI.h/cpp** - The server groups of the login screen (Data/Local/ServerList.json): order, names, test server and channel types, shown on the login screen right away (docs/server-list.md) -- **ServerListEdits.h/cpp** - The groups being changed, what the file has, and the window's undo +- **ServerListEdits.h/cpp** - The groups being changed, what the file has, and their undo history (the editor bar's Undo and Redo while the window was used last) ### Input Management - **MuInputBlocker.h/cpp** - Blocks game input when hovering over ImGui UI diff --git a/src/MuEditor/UI/Common/MuEditorUI.cpp b/src/MuEditor/UI/Common/MuEditorUI.cpp index 71926921e2..0d65b8ae8d 100644 --- a/src/MuEditor/UI/Common/MuEditorUI.cpp +++ b/src/MuEditor/UI/Common/MuEditorUI.cpp @@ -17,6 +17,7 @@ #include #include "../MuEditor/UI/Console/MuEditorConsoleUI.h" #include "../MuEditor/UI/ItemTools/ItemToolWindow.h" +#include "UndoTarget.h" // Layout of the Open Editor strip over the game while the editor is closed. The // pixel sizes are multiplied by the editor UI scale so the strip grows with it @@ -202,7 +203,7 @@ void CMuEditorUI::RenderToolWindowBar() void CMuEditorUI::RenderBar(MuEditor::ToolWindows& windows) { RenderWindowsMenu(windows, MuEditor::PlatformWindows::WindowOf(ImGui::GetWindowViewport())); - MuEditor::Items::ToolWindow::RenderUndoRedo(); + MuEditor::UndoTargets::RenderBarButtons(MuEditor::Items::ToolWindow::GetUndoTarget()); RenderUIScale(); RenderLanguageSelector(); } diff --git a/src/MuEditor/UI/Common/UndoTarget.cpp b/src/MuEditor/UI/Common/UndoTarget.cpp new file mode 100644 index 0000000000..f92efdcdc9 --- /dev/null +++ b/src/MuEditor/UI/Common/UndoTarget.cpp @@ -0,0 +1,54 @@ +#include "stdafx.h" + +#ifdef _EDITOR + +#include "UndoTarget.h" + +#include "I18N/All.h" +#include "imgui.h" +#include "imgui_internal.h" + +namespace MuEditor::UndoTargets +{ +namespace +{ +UndoTarget* g_activeTarget = nullptr; + +void RenderButton(const char* label, const char* keys, bool enabled, const std::string& step, void (UndoTarget::*act)(), + UndoTarget& target) +{ + ImGui::BeginDisabled(!enabled); + if (ImGui::Button(label)) + { + (target.*act)(); + } + if (enabled && ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled)) + { + ImGui::SetTooltip("%s (%s)", step.c_str(), keys); + } + ImGui::EndDisabled(); +} +} // namespace + +void Activate(UndoTarget& target) +{ + g_activeTarget = ⌖ +} + +void RenderBarButtons(UndoTarget& fallback) +{ + UndoTarget& target = g_activeTarget != nullptr ? *g_activeTarget : fallback; + // A click leaves the focus in the tool, where Ctrl+Z and Ctrl+Y work. + ImGui::PushItemFlag(ImGuiItemFlags_NoFocus, true); + const bool canUndo = target.CanUndo(); + RenderButton(I18N::Editor::Undo, "Ctrl+Z", canUndo, canUndo ? target.GetUndoLabel() : std::string(), + &UndoTarget::Undo, target); + ImGui::SameLine(); + const bool canRedo = target.CanRedo(); + RenderButton(I18N::Editor::Redo, "Ctrl+Y", canRedo, canRedo ? target.GetRedoLabel() : std::string(), + &UndoTarget::Redo, target); + ImGui::PopItemFlag(); +} +} // namespace MuEditor::UndoTargets + +#endif // _EDITOR diff --git a/src/MuEditor/UI/Common/UndoTarget.h b/src/MuEditor/UI/Common/UndoTarget.h new file mode 100644 index 0000000000..fd4e5acd7d --- /dev/null +++ b/src/MuEditor/UI/Common/UndoTarget.h @@ -0,0 +1,39 @@ +#pragma once + +#ifdef _EDITOR + +#include + +namespace MuEditor +{ +// A tool with an undo history of its own (the item tools, the Server List +// window). The editor bar's Undo and Redo act on the tool the user worked in +// last. +class UndoTarget +{ +public: + virtual bool CanUndo() const = 0; + virtual bool CanRedo() const = 0; + // What the next Undo or Redo would do, for the buttons' tooltips. + virtual std::string GetUndoLabel() const = 0; + virtual std::string GetRedoLabel() const = 0; + virtual void Undo() = 0; + virtual void Redo() = 0; + +protected: + ~UndoTarget() = default; +}; + +namespace UndoTargets +{ +// A tool window calls this while it has the focus: the bar's Undo and Redo then +// act on its tool, also after the focus went elsewhere. +void Activate(UndoTarget& target); + +// The editor bar's Undo and Redo, for the tool worked in last, or for +// `fallback` before any was. A click does not take the focus from the tool. +void RenderBarButtons(UndoTarget& fallback); +} // namespace UndoTargets +} // namespace MuEditor + +#endif // _EDITOR diff --git a/src/MuEditor/UI/ItemTools/ItemToolWindow.cpp b/src/MuEditor/UI/ItemTools/ItemToolWindow.cpp index 288a1e99dc..a8d04c3ff6 100644 --- a/src/MuEditor/UI/ItemTools/ItemToolWindow.cpp +++ b/src/MuEditor/UI/ItemTools/ItemToolWindow.cpp @@ -11,6 +11,7 @@ #include "../MuEditor/Core/MuEditorCore.h" #include "../MuEditor/UI/Common/MuEditorLayout.h" #include "../MuEditor/UI/Common/MuEditorWindowIds.h" +#include "../MuEditor/UI/Common/UndoTarget.h" #include "../MuEditor/UI/Console/MuEditorConsoleUI.h" #include "Core/Text/Utf8.h" #include "Data/GameData/ItemData/ItemDatabase.h" @@ -45,7 +46,7 @@ void LogRefusal() } } -void Undo() +void UndoItemEdit() { if (g_ItemEdits.GetHistory().CanUndo() && !g_ItemEdits.Undo()) { @@ -53,7 +54,7 @@ void Undo() } } -void Redo() +void RedoItemEdit() { if (g_ItemEdits.GetHistory().CanRedo() && !g_ItemEdits.Redo()) { @@ -61,6 +62,36 @@ void Redo() } } +// The item edit history, for the editor bar's Undo and Redo. +class ItemUndoTarget final : public UndoTarget +{ +public: + bool CanUndo() const override + { + return g_ItemEdits.GetHistory().CanUndo(); + } + bool CanRedo() const override + { + return g_ItemEdits.GetHistory().CanRedo(); + } + std::string GetUndoLabel() const override + { + return g_ItemEdits.GetHistory().GetUndoLabel(); + } + std::string GetRedoLabel() const override + { + return g_ItemEdits.GetHistory().GetRedoLabel(); + } + void Undo() override + { + UndoItemEdit(); + } + void Redo() override + { + RedoItemEdit(); + } +}; + // Only while the keyboard is the editor's and this window has the focus: ImGui // keeps the focus in the window when the game window takes the keyboard back // (Alt+Tab with the editor inside the game window). A text field keeps its own @@ -73,11 +104,11 @@ void HandleUndoKeys() } if (ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Z)) { - Undo(); + UndoItemEdit(); } if (ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Y) || ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiMod_Shift | ImGuiKey_Z)) { - Redo(); + RedoItemEdit(); } } } // namespace @@ -92,6 +123,11 @@ bool Begin(const char* title, const char* windowId, bool* open) { return false; } + // Before the bar: in this window's own bar it shows the item undo then. + if (ImGui::IsWindowFocused(ImGuiFocusedFlags_RootAndChildWindows)) + { + UndoTargets::Activate(GetUndoTarget()); + } MuEditor::Layout::RenderToolWindowBar(); if (ImGui::IsWindowHovered(ImGuiHoveredFlags_RootAndChildWindows | ImGuiHoveredFlags_AllowWhenBlockedByPopup | ImGuiHoveredFlags_AllowWhenBlockedByActiveItem) || @@ -108,34 +144,10 @@ void End() ImGui::End(); } -void RenderUndoRedo() +UndoTarget& GetUndoTarget() { - // In the editor's bar: a click leaves the focus in the item tool, where - // Ctrl+Z and Ctrl+Y undo and redo. - ImGui::PushItemFlag(ImGuiItemFlags_NoFocus, true); - const ItemEditHistory& history = g_ItemEdits.GetHistory(); - ImGui::BeginDisabled(!history.CanUndo()); - if (ImGui::Button(I18N::Editor::Undo)) - { - Undo(); - } - if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && history.CanUndo()) - { - ImGui::SetTooltip("%s (Ctrl+Z)", history.GetUndoLabel().c_str()); - } - ImGui::EndDisabled(); - ImGui::SameLine(); - ImGui::BeginDisabled(!history.CanRedo()); - if (ImGui::Button(I18N::Editor::Redo)) - { - Redo(); - } - if (ImGui::IsItemHovered(ImGuiHoveredFlags_AllowWhenDisabled) && history.CanRedo()) - { - ImGui::SetTooltip("%s (Ctrl+Y)", history.GetRedoLabel().c_str()); - } - ImGui::EndDisabled(); - ImGui::PopItemFlag(); + static ItemUndoTarget target; + return target; } bool RenderSelectedItem(int itemType) diff --git a/src/MuEditor/UI/ItemTools/ItemToolWindow.h b/src/MuEditor/UI/ItemTools/ItemToolWindow.h index 25ec31d60a..64185d3d80 100644 --- a/src/MuEditor/UI/ItemTools/ItemToolWindow.h +++ b/src/MuEditor/UI/ItemTools/ItemToolWindow.h @@ -2,6 +2,11 @@ #ifdef _EDITOR +namespace MuEditor +{ +class UndoTarget; +} + // What every item tool window does: where it docks, that the editor counts // the mouse over it, the undo and redo keys, and the line that names the // selected item. @@ -12,9 +17,9 @@ namespace MuEditor::Items::ToolWindow bool Begin(const char* title, const char* windowId, bool* open); void End(); -// The undo and redo buttons with what they would undo or redo, for the -// editor's bar; a click does not take the focus from the item tool. -void RenderUndoRedo(); +// The item edit history, which the editor bar's Undo and Redo act on while +// an item tool is the tool worked in last (UndoTargets). +UndoTarget& GetUndoTarget(); // "Kris (0,0)" in the UI language, with what tells it apart from items of the // same name (ItemNamesakes), or "Select an item"; false when there is no item diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index bd86f37c4c..bceb813047 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -110,19 +110,26 @@ void CMuServerListUI::Render(bool* open) ImGui::End(); return; } + // Before the bar: in this window's own bar it shows the Server List's undo then. + if (ImGui::IsWindowFocused(ImGuiFocusedFlags_RootAndChildWindows)) + { + MuEditor::UndoTargets::Activate(*this); + } MuEditor::Layout::RenderToolWindowBar(); if (ImGui::IsWindowHovered(ImGuiHoveredFlags_ChildWindows | ImGuiHoveredFlags_AllowWhenBlockedByActiveItem)) { g_MuEditorCore.SetHoveringUI(true); } + HandleUndoKeys(); if (m_sentVersion != g_ServerListManager->GetReceivedServersVersion()) { m_sent = FindSentChannels(); m_sentVersion = g_ServerListManager->GetReceivedServersVersion(); } - bool changed = RenderToolbar(); + RenderToolbar(); RenderStatus(); + bool changed = false; ImGui::Separator(); const ImVec2 listSize(GroupListWidth * g_MuEditorCore.GetUIScale(), 0.0f); @@ -189,7 +196,7 @@ void CMuServerListUI::Save() m_statusIsError = false; } -bool CMuServerListUI::RenderToolbar() +void CMuServerListUI::RenderToolbar() { // A save after a read with errors would remove the groups the read left out. ImGui::BeginDisabled(!m_edits.IsComplete()); @@ -203,7 +210,6 @@ bool CMuServerListUI::RenderToolbar() ImGui::SetTooltip("%s", I18N::Editor::SaveOffAfterReadErrors); } ImGui::SameLine(); - bool changed = false; if (ImGui::Button(I18N::Editor::Revert)) { LoadFile(); @@ -214,44 +220,71 @@ bool CMuServerListUI::RenderToolbar() "%s", I18N::Format(I18N::Editor::Reads0AgainUnsavedChangesAreLost, {Data::ServerList::ServerListFile}).c_str()); } - ImGui::SameLine(); - // Ctrl+Z and Ctrl+Y (or Ctrl+Shift+Z) while this window has the focus and - // the keyboard is the editor's: ImGui keeps the focus here when the game - // takes the keyboard back, e.g. to type into the login screen. A text field - // being edited takes them first. - const bool editorKeys = g_MuEditorCore.IsKeyboardWithEditor(); - const bool canUndo = m_edits.CanUndo() || m_uncommitted; - ImGui::BeginDisabled(!canUndo); - const bool undo = ImGui::Button(I18N::Editor::Undo); - ImGui::EndDisabled(); - if (canUndo && (undo || (editorKeys && ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Z)))) + if (m_edits.HasUnsavedChanges()) { - m_edits.Undo(); - changed = true; + ImGui::SameLine(); + ImGui::TextUnformatted(I18N::Editor::UnsavedChanges); } - ImGui::SameLine(); - const bool canRedo = m_edits.CanRedo(); - ImGui::BeginDisabled(!canRedo); - const bool redo = ImGui::Button(I18N::Editor::Redo); - ImGui::EndDisabled(); - const bool redoKeys = editorKeys && (ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Y) || - ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiMod_Shift | ImGuiKey_Z)); - if (canRedo && (redo || redoKeys)) +} + +bool CMuServerListUI::CanUndo() const +{ + return m_edits.CanUndo() || m_uncommitted; +} + +bool CMuServerListUI::CanRedo() const +{ + return m_edits.CanRedo(); +} + +std::string CMuServerListUI::GetUndoLabel() const +{ + return I18N::Editor::ServerList; +} + +std::string CMuServerListUI::GetRedoLabel() const +{ + return I18N::Editor::ServerList; +} + +void CMuServerListUI::Undo() +{ + m_edits.Undo(); + AfterHistoryStep(); +} + +void CMuServerListUI::Redo() +{ + m_edits.Redo(); + AfterHistoryStep(); +} + +void CMuServerListUI::AfterHistoryStep() +{ + m_uncommitted = false; + m_takenId = -1; + Select(std::min(m_selected, static_cast(m_edits.GetGroups().size()) - 1)); + ShowInGame(); +} + +void CMuServerListUI::HandleUndoKeys() +{ + // Only while the keyboard is the editor's: ImGui keeps the focus here when + // the game takes the keyboard back, e.g. to type into the login screen. A + // text field being edited takes the keys first. + if (!g_MuEditorCore.IsKeyboardWithEditor()) { - m_edits.Redo(); - changed = true; + return; } - if (m_edits.HasUnsavedChanges()) + if (CanUndo() && ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Z)) { - ImGui::SameLine(); - ImGui::TextUnformatted(I18N::Editor::UnsavedChanges); + Undo(); } - if (changed) + if (CanRedo() && + (ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_Y) || ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiMod_Shift | ImGuiKey_Z))) { - m_takenId = -1; - Select(std::min(m_selected, static_cast(m_edits.GetGroups().size()) - 1)); + Redo(); } - return changed; } void CMuServerListUI::RenderStatus() diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.h b/src/MuEditor/UI/ServerList/MuServerListUI.h index 2193cad9aa..fbccbf48e9 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.h +++ b/src/MuEditor/UI/ServerList/MuServerListUI.h @@ -4,6 +4,7 @@ #include "ServerListEdits.h" +#include "../MuEditor/UI/Common/UndoTarget.h" #include "Data/GameData/ItemData/ItemDataIssue.h" #include @@ -12,8 +13,9 @@ // The Server List window: the server groups of the login screen // (Data/Local/ServerList.json), their names, order and channel types. The login -// screen shows each change right away; Save writes the file. -class CMuServerListUI +// screen shows each change right away; Save writes the file. The editor bar's +// Undo and Redo act on its changes while it is the tool worked in last. +class CMuServerListUI final : public MuEditor::UndoTarget { public: // The load of each server the connect server last sent, by group ID and @@ -24,12 +26,25 @@ class CMuServerListUI void Render(bool* open); + // MuEditor::UndoTarget + bool CanUndo() const override; + bool CanRedo() const override; + std::string GetUndoLabel() const override; + std::string GetRedoLabel() const override; + void Undo() override; + void Redo() override; + private: void LoadFile(); void Save(); - // Each Render* that changes the groups returns true. - bool RenderToolbar(); + // Ctrl+Z and Ctrl+Y (or Ctrl+Shift+Z) while this window has the focus. + void HandleUndoKeys(); + // After an undo or redo: the selection stays in the list, and the login + // screen shows the groups. + void AfterHistoryStep(); + void RenderToolbar(); void RenderStatus(); + // Each of these that changes the groups returns true. bool RenderGroupList(); bool RenderListButtons(); bool RenderGroupsNotInList(); From c4286d7e3037db7aeed5e5b09b0b26419f1effc6 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:28:50 +0200 Subject: [PATCH 18/26] docs(server-list): Save stays off after the file was read with errors The Server List window turns Save off while the groups come from a read with errors, so it cannot remove the groups the read left out; the doc says so now. --- docs/server-list.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/server-list.md b/docs/server-list.md index cf58d46b6b..5459d02dba 100644 --- a/docs/server-list.md +++ b/docs/server-list.md @@ -89,7 +89,10 @@ the name the login screen gives it. their load. - **Save** writes `Data/Local/ServerList.json` in the folder the game runs from. It refuses a list with errors (a group without a name, an ID used - twice) and lists them. + twice) and lists them. When the file itself had errors as it was read + (changed by hand while the game ran), the window has only the groups that + could be read: Save stays off then, so it cannot remove the others, until + the file is fixed and **Revert** reads it again. - **Revert** reads the file again. - **Undo** and **Redo** in the editor bar (or Ctrl+Z and Ctrl+Y in the window) work through the changes of the session. The bar's buttons act on the tool From aaaeb87c7eed8f9814194703cfd8e8a909c9f63b Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:30:03 +0200 Subject: [PATCH 19/26] refactor(editor): the Server List window's long functions in smaller parts Render (65 lines) and RenderChannels (57) were over the ~40-line aim of the coding rules. Render now begins the window with the editor's bar (BeginWindow), draws the two panes (RenderPanes) and handles what changed (AfterEdits); one channel row (number, type combo, login screen name, load) is a function of its own. No change in what the window does. Review of Mosch0512/MuMain#128. --- src/MuEditor/UI/ServerList/MuServerListUI.cpp | 152 +++++++++++------- src/MuEditor/UI/ServerList/MuServerListUI.h | 7 + 2 files changed, 104 insertions(+), 55 deletions(-) diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index bceb813047..e054c34159 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -67,6 +67,72 @@ void RenderError(const char* text) ImGui::PopStyleColor(); } +bool BeginChannelTable() +{ + constexpr ImGuiTableFlags TableFlags = + ImGuiTableFlags_RowBg | ImGuiTableFlags_BordersInnerV | ImGuiTableFlags_SizingFixedFit; + if (!ImGui::BeginTable("##Channels", ChannelTableColumns, TableFlags)) + { + return false; + } + ImGui::TableSetupColumn(I18N::Editor::Channel); + ImGui::TableSetupColumn(I18N::Editor::Type); + ImGui::TableSetupColumn(I18N::Editor::OnTheLoginScreen); + ImGui::TableSetupColumn(I18N::Editor::ConnectServer); + ImGui::TableHeadersRow(); + return true; +} + +bool RenderChannelTypeCombo(ChannelType& type) +{ + ImGui::SetNextItemWidth(TypeComboWidth * g_MuEditorCore.GetUIScale()); + if (!ImGui::BeginCombo("##Type", GetChannelTypeLabel(type))) + { + return false; + } + bool changed = false; + for (int value = 0; value < Data::ServerList::ChannelTypeCount; ++value) + { + const ChannelType option = static_cast(value); + if (ImGui::Selectable(GetChannelTypeLabel(option), option == type) && option != type) + { + type = option; + changed = true; + } + } + ImGui::EndCombo(); + return changed; +} + +// One channel: its number, type, name on the login screen and the load the +// connect server sent for it (sentChannels: the group's, or nullptr). +bool RenderChannelRow(ServerGroup& group, int channel, const std::map* sentChannels) +{ + ImGui::PushID(channel); + ImGui::TableNextRow(); + ImGui::TableNextColumn(); + ImGui::Text("%d", channel + 1); + + ImGui::TableNextColumn(); + ChannelType& type = group.channels[static_cast(channel)]; + const bool changed = RenderChannelTypeCombo(type); + + ImGui::TableNextColumn(); + ImGui::TextDisabled("%s-%d%s", group.name.c_str(), channel + 1, Data::ServerList::GetChannelSuffix(type)); + + ImGui::TableNextColumn(); + if (sentChannels != nullptr) + { + const auto load = sentChannels->find(channel); + if (load != sentChannels->end()) + { + ImGui::TextUnformatted(I18N::Format(I18N::Editor::Sent0Load, {std::to_string(load->second)}).c_str()); + } + } + ImGui::PopID(); + return changed; +} + CMuServerListUI::SentChannels FindSentChannels() { CMuServerListUI::SentChannels sent; @@ -101,6 +167,25 @@ void CMuServerListUI::Render(bool* open) { LoadFile(); } + if (!BeginWindow(open)) + { + return; + } + if (m_sentVersion != g_ServerListManager->GetReceivedServersVersion()) + { + m_sent = FindSentChannels(); + m_sentVersion = g_ServerListManager->GetReceivedServersVersion(); + } + RenderToolbar(); + RenderStatus(); + ImGui::Separator(); + const bool changed = RenderPanes(); + ImGui::End(); + AfterEdits(changed); +} + +bool CMuServerListUI::BeginWindow(bool* open) +{ char title[MuEditor::WindowIds::TitleCapacity]; MuEditor::Layout::PrepareToolWindow(); if (!ImGui::Begin( @@ -108,7 +193,7 @@ void CMuServerListUI::Render(bool* open) ImGuiWindowFlags_NoCollapse | MuEditor::Layout::ToolWindowBarFlags(MuEditor::WindowIds::ServerList))) { ImGui::End(); - return; + return false; } // Before the bar: in this window's own bar it shows the Server List's undo then. if (ImGui::IsWindowFocused(ImGuiFocusedFlags_RootAndChildWindows)) @@ -121,17 +206,12 @@ void CMuServerListUI::Render(bool* open) g_MuEditorCore.SetHoveringUI(true); } HandleUndoKeys(); + return true; +} - if (m_sentVersion != g_ServerListManager->GetReceivedServersVersion()) - { - m_sent = FindSentChannels(); - m_sentVersion = g_ServerListManager->GetReceivedServersVersion(); - } - RenderToolbar(); - RenderStatus(); +bool CMuServerListUI::RenderPanes() +{ bool changed = false; - ImGui::Separator(); - const ImVec2 listSize(GroupListWidth * g_MuEditorCore.GetUIScale(), 0.0f); if (ImGui::BeginChild("##Groups", listSize, ImGuiChildFlags_Borders | ImGuiChildFlags_ResizeX)) { @@ -146,8 +226,11 @@ void CMuServerListUI::Render(bool* open) changed |= RenderSelectedGroup(); } ImGui::EndChild(); - ImGui::End(); + return changed; +} +void CMuServerListUI::AfterEdits(bool changed) +{ if (changed) { m_uncommitted = true; @@ -494,57 +577,16 @@ bool CMuServerListUI::RenderGroupFields(ServerGroup& group) bool CMuServerListUI::RenderChannels(ServerGroup& group) { - constexpr ImGuiTableFlags TableFlags = - ImGuiTableFlags_RowBg | ImGuiTableFlags_BordersInnerV | ImGuiTableFlags_SizingFixedFit; - if (!ImGui::BeginTable("##Channels", ChannelTableColumns, TableFlags)) + if (!BeginChannelTable()) { return false; } - ImGui::TableSetupColumn(I18N::Editor::Channel); - ImGui::TableSetupColumn(I18N::Editor::Type); - ImGui::TableSetupColumn(I18N::Editor::OnTheLoginScreen); - ImGui::TableSetupColumn(I18N::Editor::ConnectServer); - ImGui::TableHeadersRow(); - const auto sentGroup = m_sent.find(group.id); + const std::map* sentChannels = sentGroup != m_sent.end() ? &sentGroup->second : nullptr; bool changed = false; for (int channel = 0; channel < Data::ServerList::MaxChannelsPerGroup; ++channel) { - ImGui::PushID(channel); - ImGui::TableNextRow(); - ImGui::TableNextColumn(); - ImGui::Text("%d", channel + 1); - - ImGui::TableNextColumn(); - ChannelType& type = group.channels[static_cast(channel)]; - ImGui::SetNextItemWidth(TypeComboWidth * g_MuEditorCore.GetUIScale()); - if (ImGui::BeginCombo("##Type", GetChannelTypeLabel(type))) - { - for (int value = 0; value < Data::ServerList::ChannelTypeCount; ++value) - { - const ChannelType option = static_cast(value); - if (ImGui::Selectable(GetChannelTypeLabel(option), option == type) && option != type) - { - type = option; - changed = true; - } - } - ImGui::EndCombo(); - } - - ImGui::TableNextColumn(); - ImGui::TextDisabled("%s-%d%s", group.name.c_str(), channel + 1, Data::ServerList::GetChannelSuffix(type)); - - ImGui::TableNextColumn(); - if (sentGroup != m_sent.end()) - { - const auto load = sentGroup->second.find(channel); - if (load != sentGroup->second.end()) - { - ImGui::TextUnformatted(I18N::Format(I18N::Editor::Sent0Load, {std::to_string(load->second)}).c_str()); - } - } - ImGui::PopID(); + changed |= RenderChannelRow(group, channel, sentChannels); } ImGui::EndTable(); return changed; diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.h b/src/MuEditor/UI/ServerList/MuServerListUI.h index fbccbf48e9..98b59a913e 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.h +++ b/src/MuEditor/UI/ServerList/MuServerListUI.h @@ -35,6 +35,13 @@ class CMuServerListUI final : public MuEditor::UndoTarget void Redo() override; private: + // Begins the window with the editor's bar; false (and ended) while hidden. + bool BeginWindow(bool* open); + // The group list and the selected group; true when the groups changed. + bool RenderPanes(); + // After the widgets: the login screen shows a change, and a finished edit + // becomes one undo step. + void AfterEdits(bool changed); void LoadFile(); void Save(); // Ctrl+Z and Ctrl+Y (or Ctrl+Shift+Z) while this window has the focus. From d8f4da023854777645cd1cebdba2b88e147d8df4 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:30:46 +0200 Subject: [PATCH 20/26] refactor(server-list): the group header is ServerListGroup.h Data/GameData/ServerListData/ServerGroup.h had the name of Network/Server/ServerGroup.h (CServerGroup), and ServerListManager.h includes both. The data header is ServerListGroup.h now, so the two are not mixed up when reading or including; the type stays Data::ServerList::ServerGroup. Review of Mosch0512/MuMain#128. --- src/MuEditor/UI/ServerList/ServerListEdits.h | 2 +- .../ServerListData/{ServerGroup.h => ServerListGroup.h} | 0 src/source/Data/GameData/ServerListData/ServerListJson.h | 2 +- src/source/Network/Server/ServerListManager.h | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename src/source/Data/GameData/ServerListData/{ServerGroup.h => ServerListGroup.h} (100%) diff --git a/src/MuEditor/UI/ServerList/ServerListEdits.h b/src/MuEditor/UI/ServerList/ServerListEdits.h index a18c060a96..53a60e77d9 100644 --- a/src/MuEditor/UI/ServerList/ServerListEdits.h +++ b/src/MuEditor/UI/ServerList/ServerListEdits.h @@ -2,7 +2,7 @@ #ifdef _EDITOR -#include "Data/GameData/ServerListData/ServerGroup.h" +#include "Data/GameData/ServerListData/ServerListGroup.h" #include #include diff --git a/src/source/Data/GameData/ServerListData/ServerGroup.h b/src/source/Data/GameData/ServerListData/ServerListGroup.h similarity index 100% rename from src/source/Data/GameData/ServerListData/ServerGroup.h rename to src/source/Data/GameData/ServerListData/ServerListGroup.h diff --git a/src/source/Data/GameData/ServerListData/ServerListJson.h b/src/source/Data/GameData/ServerListData/ServerListJson.h index 069c9568ab..fb50db9b70 100644 --- a/src/source/Data/GameData/ServerListData/ServerListJson.h +++ b/src/source/Data/GameData/ServerListData/ServerListJson.h @@ -1,7 +1,7 @@ #pragma once #include "Data/GameData/ItemData/ItemDataIssue.h" -#include "Data/GameData/ServerListData/ServerGroup.h" +#include "Data/GameData/ServerListData/ServerListGroup.h" #include #include diff --git a/src/source/Network/Server/ServerListManager.h b/src/source/Network/Server/ServerListManager.h index 3eb43c367f..01cc137327 100644 --- a/src/source/Network/Server/ServerListManager.h +++ b/src/source/Network/Server/ServerListManager.h @@ -4,7 +4,7 @@ #pragma once #include "ServerGroup.h" -#include "Data/GameData/ServerListData/ServerGroup.h" +#include "Data/GameData/ServerListData/ServerListGroup.h" #include #include From d60a5cfa1cfb961987f8bac358ff93c035185da9 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:31:17 +0200 Subject: [PATCH 21/26] style(server-list): clang-format ReadText's parameters The rename of maxCharacters to maxUnits made the parameter list fit differently; clang-format 21.1.8 (the Quality Gates version) wraps it after std::string& text. Formatting only. --- src/source/Data/GameData/ServerListData/ServerListJson.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/source/Data/GameData/ServerListData/ServerListJson.cpp b/src/source/Data/GameData/ServerListData/ServerListJson.cpp index 28a58e7a95..87c01c2ced 100644 --- a/src/source/Data/GameData/ServerListData/ServerListJson.cpp +++ b/src/source/Data/GameData/ServerListData/ServerListJson.cpp @@ -98,8 +98,8 @@ bool ReadId(const OrderedJson& json, const std::string& field, int& id, IssueLis } // A text of the group; one longer than maxUnits UTF-16 units is cut. -bool ReadText(const OrderedJson& json, const std::string& field, const char* key, size_t maxUnits, - std::string& text, IssueList& issues) +bool ReadText(const OrderedJson& json, const std::string& field, const char* key, size_t maxUnits, std::string& text, + IssueList& issues) { const auto value = json.find(key); if (value == json.end()) From f8b374e327f6487b9d180194a362b82e3c57bea5 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:45:54 +0200 Subject: [PATCH 22/26] fix(editor): a closed tool leaves the bar's Undo and Redo to the item tools The bar's Undo and Redo stayed with the tool whose window had the focus last, also after that window was closed. With the Server List window closed, the bar's Undo then took back a server list change, from a window no longer shown, instead of the last item edit, and its tooltip offered Ctrl+Z, which does nothing there. UndoTarget::IsOpen tells whether the tool's window is open; while the last tool is closed the bar falls back to the item tools, whose edits stay undoable with their windows closed, as before. Follow-up review of Mosch0512/MuMain#128. --- src/MuEditor/UI/Common/UndoTarget.cpp | 3 ++- src/MuEditor/UI/Common/UndoTarget.h | 6 +++++- src/MuEditor/UI/ItemTools/ItemToolWindow.cpp | 6 ++++++ src/MuEditor/UI/ServerList/MuServerListUI.cpp | 5 +++++ src/MuEditor/UI/ServerList/MuServerListUI.h | 1 + 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/MuEditor/UI/Common/UndoTarget.cpp b/src/MuEditor/UI/Common/UndoTarget.cpp index f92efdcdc9..da0669c7c1 100644 --- a/src/MuEditor/UI/Common/UndoTarget.cpp +++ b/src/MuEditor/UI/Common/UndoTarget.cpp @@ -37,7 +37,8 @@ void Activate(UndoTarget& target) void RenderBarButtons(UndoTarget& fallback) { - UndoTarget& target = g_activeTarget != nullptr ? *g_activeTarget : fallback; + const bool activeIsOpen = g_activeTarget != nullptr && g_activeTarget->IsOpen(); + UndoTarget& target = activeIsOpen ? *g_activeTarget : fallback; // A click leaves the focus in the tool, where Ctrl+Z and Ctrl+Y work. ImGui::PushItemFlag(ImGuiItemFlags_NoFocus, true); const bool canUndo = target.CanUndo(); diff --git a/src/MuEditor/UI/Common/UndoTarget.h b/src/MuEditor/UI/Common/UndoTarget.h index fd4e5acd7d..0770065458 100644 --- a/src/MuEditor/UI/Common/UndoTarget.h +++ b/src/MuEditor/UI/Common/UndoTarget.h @@ -12,6 +12,9 @@ namespace MuEditor class UndoTarget { public: + // Whether the tool's window is open: a closed tool leaves the bar to the + // fallback (the item tools). + virtual bool IsOpen() const = 0; virtual bool CanUndo() const = 0; virtual bool CanRedo() const = 0; // What the next Undo or Redo would do, for the buttons' tooltips. @@ -31,7 +34,8 @@ namespace UndoTargets void Activate(UndoTarget& target); // The editor bar's Undo and Redo, for the tool worked in last, or for -// `fallback` before any was. A click does not take the focus from the tool. +// `fallback` before any was or while that tool is closed. A click does not take +// the focus from the tool. void RenderBarButtons(UndoTarget& fallback); } // namespace UndoTargets } // namespace MuEditor diff --git a/src/MuEditor/UI/ItemTools/ItemToolWindow.cpp b/src/MuEditor/UI/ItemTools/ItemToolWindow.cpp index a8d04c3ff6..c5f41ae3e0 100644 --- a/src/MuEditor/UI/ItemTools/ItemToolWindow.cpp +++ b/src/MuEditor/UI/ItemTools/ItemToolWindow.cpp @@ -66,6 +66,12 @@ void RedoItemEdit() class ItemUndoTarget final : public UndoTarget { public: + // The fallback of the bar: item edits stay undoable with the item tools + // closed, as they always were. + bool IsOpen() const override + { + return true; + } bool CanUndo() const override { return g_ItemEdits.GetHistory().CanUndo(); diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index e054c34159..21df95cff0 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -310,6 +310,11 @@ void CMuServerListUI::RenderToolbar() } } +bool CMuServerListUI::IsOpen() const +{ + return g_MuEditorCore.GetToolWindows().serverList; +} + bool CMuServerListUI::CanUndo() const { return m_edits.CanUndo() || m_uncommitted; diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.h b/src/MuEditor/UI/ServerList/MuServerListUI.h index 98b59a913e..1d63a78f44 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.h +++ b/src/MuEditor/UI/ServerList/MuServerListUI.h @@ -27,6 +27,7 @@ class CMuServerListUI final : public MuEditor::UndoTarget void Render(bool* open); // MuEditor::UndoTarget + bool IsOpen() const override; bool CanUndo() const override; bool CanRedo() const override; std::string GetUndoLabel() const override; From 42de1109a551f2c72627129fef4f517a04691c24 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:49:51 +0200 Subject: [PATCH 23/26] feat(editor): the bar's Undo and Redo tooltips name the Server List step For the Server List window the editor bar's tooltip only said "Server List (Ctrl+Z)", while item edits name their step; after a few changes it did not tell whether Undo takes back a rename, a removed group or a channel type. Each step of ServerListEdits now has a label: Add, Remove and Move name the group ("Remove Helheim"), a change of fields names the group as it was and the field ("Valhalla: name", "Valhalla: channel 3"). A step keeps its label when it is undone and redone. English and German. Follow-up review of Mosch0512/MuMain#128. --- src/Localization/Editor.de.resx | 24 ++++++ src/Localization/Editor.en.resx | 24 ++++++ src/MuEditor/README.md | 1 + src/MuEditor/UI/ServerList/MuServerListUI.cpp | 4 +- .../UI/ServerList/ServerListEdits.cpp | 40 ++++++++-- src/MuEditor/UI/ServerList/ServerListEdits.h | 21 ++++- .../UI/ServerList/ServerListStepLabel.cpp | 78 +++++++++++++++++++ .../UI/ServerList/ServerListStepLabel.h | 23 ++++++ tests/data/test_server_list_edits.cpp | 53 +++++++++++++ 9 files changed, 259 insertions(+), 9 deletions(-) create mode 100644 src/MuEditor/UI/ServerList/ServerListStepLabel.cpp create mode 100644 src/MuEditor/UI/ServerList/ServerListStepLabel.h diff --git a/src/Localization/Editor.de.resx b/src/Localization/Editor.de.resx index 4980429737..21ce8b4c5b 100644 --- a/src/Localization/Editor.de.resx +++ b/src/Localization/Editor.de.resx @@ -1942,4 +1942,28 @@ Die Datei hatte beim Lesen Fehler, daher enthält die Liste nur die Gruppen, die gelesen werden konnten. Speichern bleibt aus, bis die Datei korrigiert ist und Zurücksetzen sie neu einliest; es würde die anderen Gruppen aus der Datei entfernen. + + {0} hinzufügen + + + {0} entfernen + + + {0} verschieben + + + {0}: Name + + + {0}: ID + + + {0}: Beschreibung + + + {0}: Testserver + + + {0}: Kanal {1} + diff --git a/src/Localization/Editor.en.resx b/src/Localization/Editor.en.resx index 234cd84180..3556972544 100644 --- a/src/Localization/Editor.en.resx +++ b/src/Localization/Editor.en.resx @@ -1963,4 +1963,28 @@ The file had errors when it was read, so the list has only the groups that could be read. Save is off until the file is fixed and Revert reads it again; it would remove the other groups from the file. + + Add {0} + + + Remove {0} + + + Move {0} + + + {0}: name + + + {0}: ID + + + {0}: description + + + {0}: test server + + + {0}: channel {1} + diff --git a/src/MuEditor/README.md b/src/MuEditor/README.md index 1e7b5ee24e..1c6df64d23 100644 --- a/src/MuEditor/README.md +++ b/src/MuEditor/README.md @@ -57,6 +57,7 @@ stores it when a field is left. ### Server List (UI/ServerList) - **MuServerListUI.h/cpp** - The server groups of the login screen (Data/Local/ServerList.json): order, names, test server and channel types, shown on the login screen right away (docs/server-list.md) - **ServerListEdits.h/cpp** - The groups being changed, what the file has, and their undo history (the editor bar's Undo and Redo while the window was used last) +- **ServerListStepLabel.h/cpp** - What each undo step did ("Remove Helheim", "Valhalla: channel 3"), for the tooltips of the editor bar's Undo and Redo ### Input Management - **MuInputBlocker.h/cpp** - Blocks game input when hovering over ImGui UI diff --git a/src/MuEditor/UI/ServerList/MuServerListUI.cpp b/src/MuEditor/UI/ServerList/MuServerListUI.cpp index 21df95cff0..b90dc70797 100644 --- a/src/MuEditor/UI/ServerList/MuServerListUI.cpp +++ b/src/MuEditor/UI/ServerList/MuServerListUI.cpp @@ -327,12 +327,12 @@ bool CMuServerListUI::CanRedo() const std::string CMuServerListUI::GetUndoLabel() const { - return I18N::Editor::ServerList; + return m_edits.GetUndoLabel(); } std::string CMuServerListUI::GetRedoLabel() const { - return I18N::Editor::ServerList; + return m_edits.GetRedoLabel(); } void CMuServerListUI::Undo() diff --git a/src/MuEditor/UI/ServerList/ServerListEdits.cpp b/src/MuEditor/UI/ServerList/ServerListEdits.cpp index 59902ed3c4..f7005e2279 100644 --- a/src/MuEditor/UI/ServerList/ServerListEdits.cpp +++ b/src/MuEditor/UI/ServerList/ServerListEdits.cpp @@ -4,6 +4,8 @@ #include "ServerListEdits.h" +#include "ServerListStepLabel.h" + #include #include @@ -17,21 +19,25 @@ void ServerListEdits::Reset(std::vector groups, bool complete) m_saved = m_groups; m_undo.clear(); m_redo.clear(); + m_pendingLabel.clear(); } void ServerListEdits::Commit() { if (m_groups == m_committed) { + m_pendingLabel.clear(); return; } - m_undo.push_back(std::move(m_committed)); + std::string label = GetPendingLabel(); + m_undo.push_back({std::move(m_committed), std::move(label)}); if (m_undo.size() > MaxUndoSteps) { m_undo.erase(m_undo.begin()); } m_redo.clear(); m_committed = m_groups; + m_pendingLabel.clear(); } void ServerListEdits::Undo() @@ -41,9 +47,10 @@ void ServerListEdits::Undo() { return; } - m_redo.push_back(std::move(m_groups)); - m_groups = std::move(m_undo.back()); + Step step = std::move(m_undo.back()); m_undo.pop_back(); + m_redo.push_back({std::move(m_groups), std::move(step.label)}); + m_groups = std::move(step.groups); m_committed = m_groups; } @@ -54,12 +61,32 @@ void ServerListEdits::Redo() { return; } - m_undo.push_back(std::move(m_groups)); - m_groups = std::move(m_redo.back()); + Step step = std::move(m_redo.back()); m_redo.pop_back(); + m_undo.push_back({std::move(m_groups), std::move(step.label)}); + m_groups = std::move(step.groups); m_committed = m_groups; } +std::string ServerListEdits::GetUndoLabel() const +{ + if (m_groups != m_committed) + { + return GetPendingLabel(); + } + return m_undo.empty() ? std::string() : m_undo.back().label; +} + +std::string ServerListEdits::GetRedoLabel() const +{ + return m_redo.empty() ? std::string() : m_redo.back().label; +} + +std::string ServerListEdits::GetPendingLabel() const +{ + return m_pendingLabel.empty() ? StepLabel::DescribeFieldChange(m_committed, m_groups) : m_pendingLabel; +} + std::optional ServerListEdits::AddGroup(const std::string& name, std::optional id) { if (!id || IsIdUsed(*id)) @@ -74,6 +101,7 @@ std::optional ServerListEdits::AddGroup(const std::string& name, std::op group.id = *id; group.name = name; m_groups.push_back(std::move(group)); + m_pendingLabel = StepLabel::Add(name); return m_groups.size() - 1; } @@ -81,6 +109,7 @@ void ServerListEdits::RemoveGroup(size_t index) { if (index < m_groups.size()) { + m_pendingLabel = StepLabel::Remove(m_groups[index].name); m_groups.erase(m_groups.begin() + static_cast(index)); } } @@ -92,6 +121,7 @@ bool ServerListEdits::MoveGroup(size_t index, int direction) { return false; } + m_pendingLabel = StepLabel::Move(m_groups[index].name); std::swap(m_groups[index], m_groups[static_cast(target)]); return true; } diff --git a/src/MuEditor/UI/ServerList/ServerListEdits.h b/src/MuEditor/UI/ServerList/ServerListEdits.h index 53a60e77d9..da572b737e 100644 --- a/src/MuEditor/UI/ServerList/ServerListEdits.h +++ b/src/MuEditor/UI/ServerList/ServerListEdits.h @@ -47,6 +47,10 @@ class ServerListEdits } void Undo(); void Redo(); + // What Undo or Redo would take back or do again ("Remove Helheim", + // "Valhalla: channel 3"); empty when there is nothing. + std::string GetUndoLabel() const; + std::string GetRedoLabel() const; // Whether the groups differ from the file. bool HasUnsavedChanges() const @@ -77,12 +81,25 @@ class ServerListEdits std::optional FindFreeId() const; private: + // The groups on the other side of a step, and what the step did. + struct Step + { + std::vector groups; + std::string label; + }; + + // What the groups changed since the last step would be called. + std::string GetPendingLabel() const; + std::vector m_groups; // The groups as the last undo step left them. std::vector m_committed; std::vector m_saved; - std::vector> m_undo; - std::vector> m_redo; + std::vector m_undo; + std::vector m_redo; + // Set by AddGroup, RemoveGroup and MoveGroup until the next Commit; a + // change of fields is described by comparing the groups instead. + std::string m_pendingLabel; bool m_complete = true; }; } // namespace MuEditor::ServerList diff --git a/src/MuEditor/UI/ServerList/ServerListStepLabel.cpp b/src/MuEditor/UI/ServerList/ServerListStepLabel.cpp new file mode 100644 index 0000000000..7b457f2997 --- /dev/null +++ b/src/MuEditor/UI/ServerList/ServerListStepLabel.cpp @@ -0,0 +1,78 @@ +#include "stdafx.h" + +#ifdef _EDITOR + +#include "ServerListStepLabel.h" + +#include "I18N/All.h" + +#include + +namespace MuEditor::ServerList::StepLabel +{ +namespace +{ +using Data::ServerList::ServerGroup; + +// The first channel whose type differs, as its number; 0 when none does. +int FindChangedChannel(const ServerGroup& before, const ServerGroup& after) +{ + const auto changed = std::mismatch(before.channels.begin(), before.channels.end(), after.channels.begin()); + return changed.first == before.channels.end() + ? 0 + : static_cast(std::distance(before.channels.begin(), changed.first)) + 1; +} + +std::string DescribeGroupChange(const ServerGroup& before, const ServerGroup& after) +{ + const std::string& name = before.name.empty() ? after.name : before.name; + if (before.id != after.id) + { + return I18N::Format(I18N::Editor::StepID, {name}); + } + if (before.name != after.name) + { + return I18N::Format(I18N::Editor::StepName, {name}); + } + if (before.description != after.description) + { + return I18N::Format(I18N::Editor::StepDescription, {name}); + } + if (before.testServer != after.testServer) + { + return I18N::Format(I18N::Editor::StepTestServer, {name}); + } + return I18N::Format(I18N::Editor::StepChannel, {name, std::to_string(FindChangedChannel(before, after))}); +} +} // namespace + +std::string Add(const std::string& groupName) +{ + return I18N::Format(I18N::Editor::StepAdd, {groupName}); +} + +std::string Remove(const std::string& groupName) +{ + return I18N::Format(I18N::Editor::StepRemove, {groupName}); +} + +std::string Move(const std::string& groupName) +{ + return I18N::Format(I18N::Editor::StepMove, {groupName}); +} + +std::string DescribeFieldChange(std::span before, std::span after) +{ + const size_t count = std::min(before.size(), after.size()); + for (size_t index = 0; index < count; ++index) + { + if (before[index] != after[index]) + { + return DescribeGroupChange(before[index], after[index]); + } + } + return I18N::Editor::ServerList; +} +} // namespace MuEditor::ServerList::StepLabel + +#endif // _EDITOR diff --git a/src/MuEditor/UI/ServerList/ServerListStepLabel.h b/src/MuEditor/UI/ServerList/ServerListStepLabel.h new file mode 100644 index 0000000000..2f2da9e0c4 --- /dev/null +++ b/src/MuEditor/UI/ServerList/ServerListStepLabel.h @@ -0,0 +1,23 @@ +#pragma once + +#ifdef _EDITOR + +#include "Data/GameData/ServerListData/ServerListGroup.h" + +#include +#include + +// What an undo step of the Server List window did, for the editor bar's +// tooltips: "Remove Helheim", "Valhalla: channel 3". +namespace MuEditor::ServerList::StepLabel +{ +std::string Add(const std::string& groupName); +std::string Remove(const std::string& groupName); +std::string Move(const std::string& groupName); +// A change of one group's fields, by the first group and field that differ; +// the group is named as it was before. +std::string DescribeFieldChange(std::span before, + std::span after); +} // namespace MuEditor::ServerList::StepLabel + +#endif // _EDITOR diff --git a/tests/data/test_server_list_edits.cpp b/tests/data/test_server_list_edits.cpp index 1f346350b6..f2f1d7af7f 100644 --- a/tests/data/test_server_list_edits.cpp +++ b/tests/data/test_server_list_edits.cpp @@ -4,6 +4,7 @@ #ifdef _EDITOR +#include "I18N/All.h" #include "UI/ServerList/ServerListEdits.h" #include @@ -194,6 +195,58 @@ TEST_CASE("ServerListEdits: remove and move") CHECK(edits.GetGroups().size() == 2); } +TEST_CASE("ServerListEdits: each step is named for the editor bar") +{ + I18N::SetLocale("en"); + ServerListEdits edits = ThreeGroups(); + CHECK(edits.GetUndoLabel().empty()); + + edits.EditGroups()[0].channels[2] = ChannelType::Gold; + // Not committed yet (a field being typed in): named all the same. + CHECK(edits.GetUndoLabel() == "Valhalla: channel 3"); + edits.Commit(); + CHECK(edits.GetUndoLabel() == "Valhalla: channel 3"); + + edits.EditGroups()[1].name = "Asgard"; + edits.Commit(); + CHECK(edits.GetUndoLabel() == "Helheim: name"); + + edits.MoveGroup(2, -1); + edits.Commit(); + CHECK(edits.GetUndoLabel() == "Move Midgard"); + + edits.RemoveGroup(0); + edits.Commit(); + CHECK(edits.GetUndoLabel() == "Remove Valhalla"); + + edits.AddGroup("New Group"); + edits.Commit(); + CHECK(edits.GetUndoLabel() == "Add New Group"); + + // A step keeps its name when it is undone and redone. + edits.Undo(); + CHECK(edits.GetRedoLabel() == "Add New Group"); + CHECK(edits.GetUndoLabel() == "Remove Valhalla"); + edits.Redo(); + CHECK(edits.GetUndoLabel() == "Add New Group"); + CHECK(edits.GetRedoLabel().empty()); +} + +TEST_CASE("ServerListEdits: field changes are named by the group as it was") +{ + I18N::SetLocale("en"); + ServerListEdits edits = ThreeGroups(); + + edits.EditGroups()[2].id = 7; + CHECK(edits.GetUndoLabel() == "Midgard: ID"); + edits.Commit(); + edits.EditGroups()[2].description = "New"; + CHECK(edits.GetUndoLabel() == "Midgard: description"); + edits.Commit(); + edits.EditGroups()[2].testServer = true; + CHECK(edits.GetUndoLabel() == "Midgard: test server"); +} + TEST_CASE("ServerListEdits: IDs in use") { const ServerListEdits edits = ThreeGroups(); From f9f5b94c6a2a9c83c77bc588f69ee16f00c65b49 Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:50:14 +0200 Subject: [PATCH 24/26] docs(editor): the bar's Undo and Redo act on the tool used last dev-editor.md still said the editor bar has the Undo and Redo of the item tools, and item-data.md points there for them. Since the shared Undo and Redo they act on the tool used last, the item tools or the Server List window (item edits while that window is closed). The Windows menu list names the Server List too. Follow-up review of Mosch0512/MuMain#128. --- docs/dev-editor.md | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/docs/dev-editor.md b/docs/dev-editor.md index 787153abab..f122181ece 100644 --- a/docs/dev-editor.md +++ b/docs/dev-editor.md @@ -36,19 +36,23 @@ the game window; **Close Editor** is on the right of the editor window's menu bar. The editor opens in a window of its own, beside the game window where the -screen has room (else over its right side), and stays in front of the game. -Its **Windows** menu opens the tool windows: Item, Skill, Dev and Map Editor, -the Effect Browser and the console. They open as tabs docked in the editor -window, the console along the bottom. Drag a tab out to float a tool window -anywhere, over the game window too, or onto another window to dock it there; -**Windows > Reset Layout** docks them all back. The layout and the windows' -places are kept between sessions (`imgui.ini` in the game folder). - -The editor window's bar has the **Windows** menu, **Undo** and **Redo** of the -item tools, the UI scale and the language. A window of tool windows floating -outside the editor window has the same bar once, on the tab shown at its top -left when several are docked in it, so it can be worked with from another -monitor; its **Windows > Fullscreen** switches that window to full screen. +screen has room (else over its right side), and stays in front of the game. Its +**Windows** menu opens the tool windows: Item, Server List +([server-list.md](server-list.md)), Skill, Dev and Map Editor, the Effect +Browser and the console. They open as tabs docked in the editor window, the +console along the bottom. Drag a tab out to float a tool window anywhere, over +the game window too, or onto another window to dock it there; **Windows > Reset +Layout** docks them all back. The layout and the windows' places are kept +between sessions (`imgui.ini` in the game folder). + +The editor window's bar has the **Windows** menu, **Undo** and **Redo**, the UI +scale and the language. Undo and Redo act on the tool used last: after a click +into the Server List window on its changes, after a click into an item tool on +item edits; while the Server List window is closed, on item edits. A window of +tool windows floating outside the editor window has the same bar once, on the +tab shown at its top left when several are docked in it, so it can be worked +with from another monitor; its **Windows > Fullscreen** switches that window to +full screen. The editor window, and a tool window floating outside the game window, are ordinary windows of the system, with its title bar, buttons and window From f96eca062bf163c6fa7d0ab28213985b0440511f Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:50:25 +0200 Subject: [PATCH 25/26] style(ui): SyncRmlModel's comment sits above SyncRmlModel again The editor-only GetOpenGroupId and ShowRebuiltGroups went in between SyncRmlModel and its comment, and the comment's claim that the window's state only changes from its own click handlers is no longer true. The editor block now comes before the comment, which names ShowRebuiltGroups. Follow-up review of Mosch0512/MuMain#128. --- src/source/UI/Windows/ServerSelWin.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/source/UI/Windows/ServerSelWin.cpp b/src/source/UI/Windows/ServerSelWin.cpp index 81c5399031..187526a96c 100644 --- a/src/source/UI/Windows/ServerSelWin.cpp +++ b/src/source/UI/Windows/ServerSelWin.cpp @@ -337,8 +337,6 @@ bool CServerSelWin::Render() return true; } -// Called only at the point of an actual state change, not polled from Render() every frame -- -// this window's state only ever changes from its own click handlers. #ifdef _EDITOR int CServerSelWin::GetOpenGroupId() const { @@ -372,6 +370,9 @@ void CServerSelWin::ShowRebuiltGroups(int openGroupId) } #endif +// Called only at the point of an actual state change, not polled from Render() every frame -- +// this window's state only changes from its own click handlers and, in editor builds, from +// ShowRebuiltGroups after the Server List window built the groups again. void CServerSelWin::SyncRmlModel() { if (!m_RmlView.Document()) return; From 487c7cd6cd921a09759b0057a7fc7afbc721c8fe Mon Sep 17 00:00:00 2001 From: Mosch0512 Date: Fri, 23 Oct 2026 00:50:31 +0200 Subject: [PATCH 26/26] docs(server-list): wrap the comparison with the original client One line of the paragraph was 142 characters; the file wraps at about 78. Wording unchanged. Follow-up review of Mosch0512/MuMain#128. --- docs/server-list.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/server-list.md b/docs/server-list.md index 5459d02dba..0fe2cf8711 100644 --- a/docs/server-list.md +++ b/docs/server-list.md @@ -103,7 +103,8 @@ the name the login screen gives it. `ServerList.bmd` held the same groups, encrypted, with channel types for the first 15 channels only and a left, right or center position for each button. -The original screen drew the center one, a test server, as a button of its -own; the login screen now lists all groups in one column in the list's order, -so only "test server" is kept, for the rule that only the first is shown. The original loader read each group's description but never kept it, -so the login screen showed none; the JSON description is shown. +The original screen drew the center one, a test server, as a button of its own; +the login screen now lists all groups in one column in the list's order, so +only "test server" is kept, for the rule that only the first is shown. The +original loader read each group's description but never kept it, so the login +screen showed none; the JSON description is shown.