Improve party organization with auto-party

BH/CMakeLists.txt

@@ -50,4 +50,5 @@ set(DRAWING_SOURCES "Drawing/Hook.cpp"
 
 set(SOURCE_FILES ${SOURCE_FILES} ${MODULE_SOURCES} ${DRAWING_SOURCES})
 add_library(BH ${SOURCE_FILES})
+target_include_directories(BH PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../ThirdParty/cpp-lru-cache/include)
 target_link_libraries(BH ${STORM_LIBRARY} Shlwapi)

BH/Common.cpp

@@ -123,11 +123,13 @@ int StringToNumber(std::string str) {
 	return ret;
 }
 
+// This function prints at most 151 characters (152 including null)
+// TODO: Fix this so this limitation
 void PrintText(DWORD Color, char *szText, ...) {
 	char szBuffer[152] = {0};
 	va_list Args;
 	va_start(Args, szText);
-	vsprintf_s(szBuffer, 152, szText, Args);
+	vsnprintf_s(szBuffer, 152, _TRUNCATE, szText, Args);
 	va_end(Args); 
 	wchar_t Buffer[0x130];
 	MultiByteToWideChar(CODE_PAGE, 1, szBuffer, 152, Buffer, 304);
@@ -461,4 +463,4 @@ char *commaprint(unsigned long n)
 	} while (n != 0);
 
 	return p;
-}
+}

BH/Constants.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#define BH_VERSION "BH 1.9.7"
+#define BH_VERSION "BH 1.9.8alpha5"
 
 #define CODE_PAGE 1252 // windows-1252	ANSI Latin 1; Western European (Windows)
 

BH/Drawing/UI/UI.cpp

@@ -174,10 +174,10 @@ void UI::OnDraw() {
 	}
 }
 
-void UI::SetDragged(bool state) { 
+void UI::SetDragged(bool state, bool write_file) {
 	Lock(); 
 	dragged = state; 
-	if (!state) {
+	if (!state && write_file) {
 		WritePrivateProfileString(name.c_str(), "X", to_string<unsigned int>(GetX()).c_str(), string(BH::path + "UI.ini").c_str());
 		WritePrivateProfileString(name.c_str(), "Y", to_string<unsigned int>(GetY()).c_str(), string(BH::path + "UI.ini").c_str());
 		WritePrivateProfileString(name.c_str(), "minimizedX", to_string<unsigned int>(GetMinimizedX()).c_str(), string(BH::path + "UI.ini").c_str());
@@ -186,6 +186,10 @@ void UI::SetDragged(bool state) {
 	Unlock(); 
 }
 
+void UI::SetDragged(bool state) {
+    SetDragged(state, false);
+}
+
 void UI::SetMinimized(bool newState) { 
 	if (newState == minimized) 
 		return; 
@@ -228,7 +232,7 @@ bool UI::OnLeftClick(bool up, unsigned int mouseX, unsigned int mouseY) {
 			}
 			else
 			{
-				SetDragged(false);
+				SetDragged(false, true);
 				if(!up) {
 					PrintText(7, "CTRL-click to open settings" );
 					PrintText(7, "Shift-drag to move" );
@@ -248,7 +252,7 @@ bool UI::OnLeftClick(bool up, unsigned int mouseX, unsigned int mouseY) {
 		} 
 		else
 		{
-			SetDragged(false);
+			SetDragged(false, true);
 			if( startX == mouseX && startY == mouseY && GetAsyncKeyState(VK_CONTROL) )
 			{
 				PrintText(135, "Right Click to Close" );
@@ -271,7 +275,7 @@ bool UI::OnLeftClick(bool up, unsigned int mouseX, unsigned int mouseY) {
 		return true;
 	}
 	SetActive(false);
-	SetDragged(false);
+	SetDragged(false, false);
 	return false;
 }
 

BH/Drawing/UI/UI.h

@@ -55,7 +55,8 @@ namespace Drawing {
 			void SetActive(bool newState) { Lock(); active = newState; Unlock(); };
 			void SetMinimized(bool newState);
 			void SetName(std::string newName) { Lock(); name = newName;  Unlock(); };
-			void SetDragged(bool state);
+			void SetDragged(bool state, bool write_file); // only write config to file if write_file is true
+			void SetDragged(bool state); // never writes the config file
 			void SetZOrder(unsigned int newZ) { Lock(); zOrder = newZ; Unlock(); };
 
 			UITab* GetActiveTab() { if (!currentTab) { currentTab = (*Tabs.begin()); } return currentTab; };

BH/Modules/Item/Item.cpp

@@ -50,6 +50,7 @@
 #include "../../D2Stubs.h"
 #include "ItemDisplay.h"
 #include "../../MPQInit.h"
+#include "lrucache.hpp"
 
 ItemsTxtStat* GetAllStatModifier(ItemsTxtStat* pStats, int nStats, int nStat, ItemsTxtStat* pOrigin);
 ItemsTxtStat* GetItemsTxtStatByMod(ItemsTxtStat* pStats, int nStats, int nStat, int nStatParam);
@@ -86,6 +87,13 @@ void Item::OnLoad() {
 	DrawSettings();
 }
 
+void Item::OnGameJoin() {
+	// reset the item name cache upon joining games
+	// (GUIDs not unique across games)
+	item_name_cache.ResetCache();
+	map_action_cache.ResetCache();
+}
+
 void Item::LoadConfig() {
 	BH::config->ReadToggle("Show Ethereal", "None", true, Toggles["Show Ethereal"]);
 	BH::config->ReadToggle("Show Sockets", "None", true, Toggles["Show Sockets"]);

BH/Modules/Item/Item.h

@@ -67,6 +67,8 @@ class Item : public Module {
 		void LoadConfig();
 		void DrawSettings();
 
+		void OnGameJoin();
+
 		void OnLoop();
 		void OnKey(bool up, BYTE key, LPARAM lParam, bool* block);
 		void OnLeftClick(bool up, int x, int y, bool* block);

BH/Modules/Item/ItemDisplay.cpp

@@ -151,15 +151,55 @@ BYTE RuneNumberFromItemCode(char *code){
 	return (BYTE)(((code[1] - '0') * 10) + code[2] - '0');
 }
 
-void GetItemName(UnitItemInfo *uInfo, string &name) {
-	for (vector<Rule*>::iterator it = RuleList.begin(); it != RuleList.end(); it++) {
+// Find the item name. This code is called only when there's a cache miss
+string ItemNameLookupCache::make_cached_T(UnitItemInfo *uInfo, const string &name) {
+	string new_name(name);
+	for (vector<Rule*>::const_iterator it = RuleList.begin(); it != RuleList.end(); it++) {
 		if ((*it)->Evaluate(uInfo, NULL)) {
-			SubstituteNameVariables(uInfo, name, &(*it)->action);
+			SubstituteNameVariables(uInfo, new_name, &(*it)->action);
 			if ((*it)->action.stopProcessing) {
 				break;
 			}
 		}
 	}
+	return new_name;
+}
+
+string ItemNameLookupCache::to_str(const string &name) {
+	size_t start_pos = 0;
+	std::string itemName(name);
+	while ((start_pos = itemName.find('\n', start_pos)) != std::string::npos) {
+		itemName.replace(start_pos, 1, " - ");
+		start_pos += 3;
+	}
+	return itemName;
+}
+
+vector<Action> MapActionLookupCache::make_cached_T(UnitItemInfo *uInfo) {
+	vector<Action> actions;
+	for (vector<Rule*>::const_iterator it = RuleList.begin(); it != RuleList.end(); it++) {
+		if ((*it)->Evaluate(uInfo, NULL)) {
+			actions.push_back((*it)->action);
+		}
+	}
+	return actions;
+}
+
+string MapActionLookupCache::to_str(const vector<Action> &actions) {
+	string name;
+	for (auto &action : actions) {
+		name += action.name + " ";
+	}
+	return name;
+}
+
+// least recently used cache for storing a limited number of item names
+ItemNameLookupCache item_name_cache(RuleList);
+MapActionLookupCache map_action_cache(MapRuleList);
+
+void GetItemName(UnitItemInfo *uInfo, string &name) {
+	string new_name = item_name_cache.Get(uInfo, name);
+	name.assign(new_name);
 }
 
 void SubstituteNameVariables(UnitItemInfo *uInfo, string &name, Action *action) {
@@ -333,6 +373,8 @@ namespace ItemDisplay {
 
 		item_display_initialized = true;
 		rules.clear();
+		item_name_cache.ResetCache();
+		map_action_cache.ResetCache();
 		BH::config->ReadMapList("ItemDisplay", rules);
 		for (unsigned int i = 0; i < rules.size(); i++) {
 			string buf;
@@ -343,13 +385,11 @@ namespace ItemDisplay {
 			}
 
 			LastConditionType = CT_None;
-			Rule *r = new Rule();
 			vector<Condition*> RawConditions;
 			for (vector<string>::iterator tok = tokens.begin(); tok < tokens.end(); tok++) {
 				Condition::BuildConditions(RawConditions, (*tok));
 			}
-			Condition::ProcessConditions(RawConditions, r->conditions);
-			BuildAction(&(rules[i].second), &(r->action));
+			Rule *r = new Rule(RawConditions, &(rules[i].second));
 
 			RuleList.push_back(r);
 			if (r->action.colorOnMap != UNDEFINED_COLOR ||
@@ -366,13 +406,31 @@ namespace ItemDisplay {
 	}
 
 	void UninitializeItemRules() {
+		// RuleList contains every created rule. MapRuleList and IgnoreRuleList have a subset of rules.
+		// Deleting objects in RuleList is sufficient.
+		if (item_display_initialized) {
+			for (Rule *r : RuleList) {
+				for (Condition *condition : r->conditions) {
+					delete condition;
+				}
+				delete r;
+			}
+		}
 		item_display_initialized = false;
+		item_name_cache.ResetCache();
+		map_action_cache.ResetCache();
 		RuleList.clear();
 		MapRuleList.clear();
 		IgnoreRuleList.clear();
 	}
 }
 
+Rule::Rule(vector<Condition*> &inputConditions, string *str) {
+	Condition::ProcessConditions(inputConditions, conditions);
+	BuildAction(str, &action);
+	conditionStack.reserve(conditions.size()); // TODO: too large?
+}
+
 void BuildAction(string *str, Action *act) {
 	act->name = string(str->c_str());
 

BH/Modules/Item/ItemDisplay.h

@@ -6,6 +6,7 @@
 #include "../../BH.h"
 #include <cstdlib>
 #include <regex>
+#include "../../RuleLookupCache.h"
 
 #define EXCEPTION_INVALID_STAT			1
 #define EXCEPTION_INVALID_OPERATION		2
@@ -520,7 +521,11 @@ struct Action {
 struct Rule {
 	vector<Condition*> conditions;
 	Action action;
+	vector<Condition*> conditionStack;
 
+	Rule(vector<Condition*> &inputConditions, string *str);
+
+	// TODO: Should this really be defined in the header? This will force it to be inlined AFAIK. -ybd
 	// Evaluate conditions which are in Reverse Polish Notation
 	bool Evaluate(UnitItemInfo *uInfo, ItemInfo *info) {
 		if (conditions.size() == 0) {
@@ -529,7 +534,12 @@ struct Rule {
 
 		bool retval;
 		try {
-			vector<Condition*> conditionStack;
+			// conditionStack was previously declared on the stack within this function. This caused
+			// excessive allocaiton calls and was limiting the speed of the item display (causing
+			// frame rate drops with complex item displays while ALT was held down). Moving this vector
+			// to the object level, preallocating size, and using the resize(0) method to clear avoids
+			// excessive reallocation.
+			conditionStack.resize(0); 
 			for (unsigned int i = 0; i < conditions.size(); i++) {
 				Condition *input = conditions[i];
 				if (input->conditionType == CT_Operand) {
@@ -567,10 +577,26 @@ struct Rule {
 	}
 };
 
+class ItemNameLookupCache : public RuleLookupCache<string, const string &> {
+	string make_cached_T(UnitItemInfo *uInfo, const string &name) override;
+	string to_str(const string &name) override;
+
+	using RuleLookupCache::RuleLookupCache;
+};
+
+class MapActionLookupCache : public RuleLookupCache<vector<Action>> {
+	vector<Action> make_cached_T(UnitItemInfo *uInfo) override;
+	string to_str(const vector<Action> &actions);
+
+	using RuleLookupCache::RuleLookupCache;
+};
+
 extern vector<Rule*> RuleList;
 extern vector<Rule*> MapRuleList;
 extern vector<Rule*> IgnoreRuleList;
 extern vector<pair<string, string>> rules;
+extern ItemNameLookupCache item_name_cache;
+extern MapActionLookupCache map_action_cache;
 
 namespace ItemDisplay {
 	void InitializeItemRules();

BH/Modules/Maphack/Maphack.cpp

@@ -512,32 +512,31 @@ void Maphack::OnAutomapDraw() {
 					uInfo.itemCode[3] = 0;
 					if (ItemAttributeMap.find(uInfo.itemCode) != ItemAttributeMap.end()) {
 						uInfo.attrs = ItemAttributeMap[uInfo.itemCode];
-						for (vector<Rule*>::iterator it = MapRuleList.begin(); it != MapRuleList.end(); it++) {
-							if ((*it)->Evaluate(&uInfo, NULL)) {
-								auto color = (*it)->action.colorOnMap;
-								auto borderColor = (*it)->action.borderColor;
-								auto dotColor = (*it)->action.dotColor;
-								auto pxColor = (*it)->action.pxColor;
-								auto lineColor = (*it)->action.lineColor;
-								xPos = unit->pItemPath->dwPosX;
-								yPos = unit->pItemPath->dwPosY;
-								automapBuffer.push_top_layer(
-										[color, unit, xPos, yPos, MyPos, borderColor, dotColor, pxColor, lineColor]()->void{
-									POINT automapLoc;
-									Drawing::Hook::ScreenToAutomap(&automapLoc, xPos, yPos);
-									if (borderColor != UNDEFINED_COLOR)
-										Drawing::Boxhook::Draw(automapLoc.x - 4, automapLoc.y - 4, 8, 8, borderColor, Drawing::BTHighlight);
-									if (color != UNDEFINED_COLOR)
-										Drawing::Boxhook::Draw(automapLoc.x - 3, automapLoc.y - 3, 6, 6, color, Drawing::BTHighlight);
-									if (dotColor != UNDEFINED_COLOR)
-										Drawing::Boxhook::Draw(automapLoc.x - 2, automapLoc.y - 2, 4, 4, dotColor, Drawing::BTHighlight);
-									if (pxColor != UNDEFINED_COLOR)
-										Drawing::Boxhook::Draw(automapLoc.x - 1, automapLoc.y - 1, 2, 2, pxColor, Drawing::BTHighlight);
-									if (lineColor != UNDEFINED_COLOR) {
-										Drawing::Linehook::Draw(MyPos.x, MyPos.y, automapLoc.x, automapLoc.y, lineColor);
-									}
-								});
-							}
+						const vector<Action> actions = map_action_cache.Get(&uInfo);
+						for (auto &action : actions) {
+							auto color = action.colorOnMap;
+							auto borderColor = action.borderColor;
+							auto dotColor = action.dotColor;
+							auto pxColor = action.pxColor;
+							auto lineColor = action.lineColor;
+							xPos = unit->pItemPath->dwPosX;
+							yPos = unit->pItemPath->dwPosY;
+							automapBuffer.push_top_layer(
+									[color, unit, xPos, yPos, MyPos, borderColor, dotColor, pxColor, lineColor]()->void{
+								POINT automapLoc;
+								Drawing::Hook::ScreenToAutomap(&automapLoc, xPos, yPos);
+								if (borderColor != UNDEFINED_COLOR)
+									Drawing::Boxhook::Draw(automapLoc.x - 4, automapLoc.y - 4, 8, 8, borderColor, Drawing::BTHighlight);
+								if (color != UNDEFINED_COLOR)
+									Drawing::Boxhook::Draw(automapLoc.x - 3, automapLoc.y - 3, 6, 6, color, Drawing::BTHighlight);
+								if (dotColor != UNDEFINED_COLOR)
+									Drawing::Boxhook::Draw(automapLoc.x - 2, automapLoc.y - 2, 4, 4, dotColor, Drawing::BTHighlight);
+								if (pxColor != UNDEFINED_COLOR)
+									Drawing::Boxhook::Draw(automapLoc.x - 1, automapLoc.y - 1, 2, 2, pxColor, Drawing::BTHighlight);
+								if (lineColor != UNDEFINED_COLOR) {
+									Drawing::Linehook::Draw(MyPos.x, MyPos.y, automapLoc.x, automapLoc.y, lineColor);
+								}
+							});
 						}
 					}
 					else {