Wildcard tree search enhancement and tests

src/game.cpp

@@ -463,14 +463,17 @@ ReturnValue Game::getPlayerByNameWildcard(const std::string& s, Player*& player)
 	}
 
 	if (s.back() == '~') {
-		const std::string& query = boost::algorithm::to_lower_copy(s.substr(0, strlen - 1));
-		std::string result;
-		ReturnValue ret = wildcardTree.findOne(query, result);
-		if (ret != RETURNVALUE_NOERROR) {
-			return ret;
+		const auto& query = boost::algorithm::to_lower_copy(s.substr(0, strlen - 1));
+		auto search_result = tfs::game::wildcardtree::search(query);
+		switch (search_result.first) {
+			case WildcardTreeSearchResult::NotFound:
+				return RETURNVALUE_PLAYERWITHTHISNAMEISNOTONLINE;
+			case WildcardTreeSearchResult::Ambiguous:
+				return RETURNVALUE_NAMEISTOOAMBIGUOUS;
+			case WildcardTreeSearchResult::Found:
+				player = getPlayerByName(search_result.second);
+				break;
 		}
-
-		player = getPlayerByName(result);
 	} else {
 		player = getPlayerByName(s);
 	}
@@ -5665,7 +5668,7 @@ void Game::addPlayer(Player* player)
 	const std::string& lowercase_name = boost::algorithm::to_lower_copy(player->getName());
 	mappedPlayerNames[lowercase_name] = player;
 	mappedPlayerGuids[player->getGUID()] = player;
-	wildcardTree.insert(lowercase_name);
+	tfs::game::wildcardtree::add(lowercase_name);
 	players[player->getID()] = player;
 }
 
@@ -5674,7 +5677,7 @@ void Game::removePlayer(Player* player)
 	const std::string& lowercase_name = boost::algorithm::to_lower_copy(player->getName());
 	mappedPlayerNames.erase(lowercase_name);
 	mappedPlayerGuids.erase(player->getGUID());
-	wildcardTree.remove(lowercase_name);
+	tfs::game::wildcardtree::remove(lowercase_name);
 	players.erase(player->getID());
 }
 

src/game.h

@@ -519,8 +519,6 @@ class Game
 
 	size_t lastBucket = 0;
 
-	WildcardTreeNode wildcardTree{false};
-
 	std::map<uint32_t, Npc*> npcs;
 	std::map<uint32_t, Monster*> monsters;
 

src/tests/CMakeLists.txt

@@ -4,6 +4,7 @@ set(tests_SRC
     ${CMAKE_CURRENT_LIST_DIR}/test_matrixarea.cpp
     ${CMAKE_CURRENT_LIST_DIR}/test_rsa.cpp
     ${CMAKE_CURRENT_LIST_DIR}/test_sha1.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/test_wildcard_tree.cpp
     ${CMAKE_CURRENT_LIST_DIR}/test_xtea.cpp
     )
 

src/tests/test_wildcard_tree.cpp

@@ -0,0 +1,78 @@
+#define BOOST_TEST_MODULE wildcard_tree
+
+#include "../otpch.h"
+
+#include "../wildcardtree.h"
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_AUTO_TEST_CASE(test_wildcard_tree_single_words)
+{
+	tfs::game::wildcardtree::add("test");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("tes").first, WildcardTreeSearchResult::Found);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("tes").second, "test");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").first, WildcardTreeSearchResult::Found);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").second, "test");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("testing").first, WildcardTreeSearchResult::NotFound);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("testing").second, "");
+}
+
+BOOST_AUTO_TEST_CASE(test_wildcard_tree_ambiguity)
+{
+	tfs::game::wildcardtree::add("test");
+    tfs::game::wildcardtree::add("te");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("te").first, WildcardTreeSearchResult::Ambiguous);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("te").second, "");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").first, WildcardTreeSearchResult::Found);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").second, "test");
+}
+
+BOOST_AUTO_TEST_CASE(test_wildcard_tree_remove)
+{
+    tfs::game::wildcardtree::add("test");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").first, WildcardTreeSearchResult::Found);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").second, "test");
+
+    tfs::game::wildcardtree::remove("test");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").first, WildcardTreeSearchResult::NotFound);
+}
+
+BOOST_AUTO_TEST_CASE(test_wildcard_tree_partial_search)
+{
+    tfs::game::wildcardtree::add("test");
+    tfs::game::wildcardtree::add("te");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("te").first, WildcardTreeSearchResult::Ambiguous);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("te").second, "");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").first, WildcardTreeSearchResult::Found);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").second, "test");
+
+    tfs::game::wildcardtree::remove("test");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("te").first, WildcardTreeSearchResult::Found);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("te").second, "te");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").first, WildcardTreeSearchResult::NotFound);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").second, "");
+}
+
+BOOST_AUTO_TEST_CASE(test_wildcard_tree_search_after_remove)
+{
+    tfs::game::wildcardtree::add("test");
+    tfs::game::wildcardtree::add("te");
+    tfs::game::wildcardtree::remove("test");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").first, WildcardTreeSearchResult::NotFound);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("test").second, "");
+
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("te").first, WildcardTreeSearchResult::Found);
+    BOOST_CHECK_EQUAL(tfs::game::wildcardtree::search("te").second, "te");
+}

src/wildcardtree.cpp

@@ -6,109 +6,127 @@
 #include "wildcardtree.h"
 
 #include <stack>
-#include <tuple>
 
-WildcardTreeNode* WildcardTreeNode::getChild(char ch)
-{
-	auto it = children.find(ch);
-	if (it == children.end()) {
-		return nullptr;
-	}
-	return &it->second;
-}
+namespace {
+
+auto wildcardtree_root = std::make_shared<WildcardTreeNode>(false);
 
-const WildcardTreeNode* WildcardTreeNode::getChild(char ch) const
+std::shared_ptr<WildcardTreeNode> WildcardTreeNode::find_child(char c)
 {
-	auto it = children.find(ch);
+	auto it = children.find(c);
 	if (it == children.end()) {
 		return nullptr;
 	}
-	return &it->second;
+	return it->second;
 }
 
-WildcardTreeNode* WildcardTreeNode::addChild(char ch, bool breakpoint)
+std::shared_ptr<WildcardTreeNode> WildcardTreeNode::add_child(char c, bool breakpoint)
 {
-	WildcardTreeNode* child = getChild(ch);
-	if (child) {
+	if (auto child = find_child(c)) {
+		// If the child already exists, update its breakpoint if necessary
 		if (breakpoint && !child->breakpoint) {
 			child->breakpoint = true;
 		}
-	} else {
-		auto pair =
-		    children.emplace(std::piecewise_construct, std::forward_as_tuple(ch), std::forward_as_tuple(breakpoint));
-		child = &pair.first->second;
+		return child;
 	}
-	return child;
+
+	auto pair = children.emplace(c, std::make_shared<WildcardTreeNode>(breakpoint));
+	return pair.first->second;
 }
 
-void WildcardTreeNode::insert(const std::string& str)
+} // namespace
+
+namespace tfs::game::wildcardtree {
+
+void add(std::string_view s)
 {
-	WildcardTreeNode* cur = this;
+	auto node = wildcardtree_root;
 
-	size_t length = str.length() - 1;
-	for (size_t pos = 0; pos < length; ++pos) {
-		cur = cur->addChild(str[pos], false);
+	auto length = s.length();
+	// Iterate through the string, adding nodes for each character except the last
+	for (size_t pos = 0; pos < length - 1; ++pos) {
+		// Add child nodes without marking breakpoints
+		node = node->add_child(s[pos], false);
 	}
 
-	cur->addChild(str[length], true);
+	// Mark the final character as a breakpoint
+	node->add_child(s.back(), true);
 }
 
-void WildcardTreeNode::remove(const std::string& str)
+void remove(std::string_view s)
 {
-	WildcardTreeNode* cur = this;
-
-	std::stack<WildcardTreeNode*> path;
-	path.push(cur);
-	size_t len = str.length();
-	for (size_t pos = 0; pos < len; ++pos) {
-		cur = cur->getChild(str[pos]);
-		if (!cur) {
+	auto node = wildcardtree_root;
+
+	// Stack to keep track of the path as we traverse the tree
+	std::stack<std::shared_ptr<WildcardTreeNode>> path;
+	path.push(node);
+
+	auto length = s.length();
+	// Traverse the tree based on the input string
+	for (size_t pos = 0; pos < length; ++pos) {
+		node = node->find_child(s[pos]);
+		if (!node) {
 			return;
 		}
-		path.push(cur);
+		path.push(node);
 	}
 
-	cur->breakpoint = false;
+	node->breakpoint = false;
 
-	do {
-		cur = path.top();
+	// Remove orphaned nodes
+	while (true) {
+		node = path.top();
 		path.pop();
+		if (path.empty()) {
+			// Stop if the root is reached
+			break;
+		}
 
-		if (!cur->children.empty() || cur->breakpoint || path.empty()) {
+		if (node->breakpoint || !node->children.empty()) {
+			// Stop if the node has children or is a breakpoint
 			break;
 		}
 
-		cur = path.top();
+		node = path.top(); // Go back to the parent node
 
-		auto it = cur->children.find(str[--len]);
-		if (it != cur->children.end()) {
-			cur->children.erase(it);
+		auto it = node->children.find(s[--length]);
+		if (it != node->children.end()) {
+			// Erase the child node from the parent if it exists
+			node->children.erase(it);
 		}
-	} while (true);
+	}
 }
 
-ReturnValue WildcardTreeNode::findOne(const std::string& query, std::string& result) const
+std::pair<WildcardTreeSearchResult, std::string> search(std::string_view query)
 {
-	const WildcardTreeNode* cur = this;
-	for (char pos : query) {
-		cur = cur->getChild(pos);
-		if (!cur) {
-			return RETURNVALUE_PLAYERWITHTHISNAMEISNOTONLINE;
+	auto node = wildcardtree_root;
+
+	for (auto c : query) {
+		node = node->find_child(c);
+		if (!node) {
+			return std::make_pair(WildcardTreeSearchResult::NotFound, "");
 		}
 	}
 
-	result = query;
+	std::string result(query);
 
-	do {
-		size_t size = cur->children.size();
+	// Continue traversal until ambiguity or the end of the string is found
+	while (true) {
+		const auto size = node->children.size();
 		if (size == 0) {
-			return RETURNVALUE_NOERROR;
-		} else if (size > 1 || cur->breakpoint) {
-			return RETURNVALUE_NAMEISTOOAMBIGUOUS;
+			// Exact match found
+			return std::make_pair(WildcardTreeSearchResult::Found, result);
+		}
+
+		if (size > 1 || node->breakpoint) {
+			// Return ambiguous if multiple child nodes exist or a breakpoint is reached
+			return std::make_pair(WildcardTreeSearchResult::Ambiguous, "");
 		}
 
-		auto it = cur->children.begin();
+		auto it = node->children.begin();
 		result += it->first;
-		cur = &it->second;
-	} while (true);
+		node = it->second;
+	}
 }
+
+} // namespace tfs::game::wildcardtree