Performance and border fixes (#164)

Mctalian · Oct 27, 2024 · 89d0b5c · 89d0b5c
1 parent 8269e5a
commit 89d0b5c
Show file tree

Hide file tree

Showing 8 changed files with 257 additions and 114 deletions.
diff --git a/.scripts/hardcode_string_check.py b/.scripts/hardcode_string_check.py
@@ -67,7 +67,9 @@ def scan_directory(directory, ignore_files=None, ignore_dirs=None):
 
 
 def main():
-    ignore_files = []
+    ignore_files = [
+        "SmokeTest.lua",
+    ]
     ignore_dirs = [
         ".git",
         ".github",

diff --git a/Features/ItemLoot.lua b/Features/ItemLoot.lua
@@ -50,10 +50,7 @@ function ItemLoot.Element:new(...)
 	local t
 	element.key, t, element.icon, element.quantity = ...
 
-	element.isPassingFilter = function()
-		local itemName, _, itemQuality, itemLevel, itemMinLevel, itemType, itemSubType, itemStackCount, itemEquipLoc, itemTexture, sellPrice, classID, subclassID, bindType, expansionID, setID, isCraftingReagent =
-			C_Item.GetItemInfo(t)
-
+	function element:isPassingFilter(itemName, itemQuality)
 		if not G_RLF.db.global.itemQualityFilter[itemQuality] then
 			element:getLogger():Debug(
 				itemName .. " ignored by quality: " .. itemQualityName(itemQuality),
@@ -102,21 +99,46 @@ end
 
 function ItemLoot:OnDisable()
 	self:UnregisterEvent("CHAT_MSG_LOOT")
+	self:UnregisterEvent("GET_ITEM_INFO_RECEIVED")
 end
 
 function ItemLoot:OnEnable()
 	self:RegisterEvent("CHAT_MSG_LOOT")
+	self:RegisterEvent("GET_ITEM_INFO_RECEIVED")
+end
+
+local pendingItemRequests = {}
+local function onItemReadyToShow(itemId, itemLink, itemTexture, amount, itemName, itemQuality)
+	pendingItemRequests[itemId] = nil
+	local e = ItemLoot.Element:new(itemId, itemLink, itemTexture, amount)
+	e:Show(itemName, itemQuality)
+end
+
+function ItemLoot:GET_ITEM_INFO_RECEIVED(eventName, itemID, success)
+	if not pendingItemRequests[itemID] then
+		return
+	end
+
+	local itemLink, amount = unpack(pendingItemRequests[itemID])
+
+	if not success then
+		error("Failed to load item: " .. itemID .. " " .. itemLink .. " x" .. amount)
+	else
+		local itemName, _, itemQuality, _, _, _, _, _, _, itemTexture, _, _, _, _, _, _, _ =
+			C_Item.GetItemInfo(itemLink)
+		onItemReadyToShow(itemID, itemLink, itemTexture, amount, itemName, itemQuality)
+	end
 end
 
 local function showItemLoot(msg, itemLink)
 	local amount = tonumber(msg:match("r ?x(%d+)") or 1)
-	local _, _, itemQuality, itemLevel, itemMinLevel, itemType, itemSubType, itemStackCount, itemEquipLoc, itemTexture, sellPrice, classID, subclassID, bindType, expansionID, setID, isCraftingReagent =
-		C_Item.GetItemInfo(itemLink)
-
 	local itemId = itemLink:match("Hitem:(%d+)")
+	pendingItemRequests[itemId] = { itemLink, amount }
+	local itemName, _, itemQuality, _, _, _, _, _, _, itemTexture, _, _, _, _, _, _, _ = C_Item.GetItemInfo(itemLink)
 
-	local e = ItemLoot.Element:new(itemId, itemLink, itemTexture, amount)
-	e:Show()
+	if itemName ~= nil then
+		onItemReadyToShow(itemId, itemLink, itemTexture, amount, itemName, itemQuality)
+	end
 end
 
 function ItemLoot:CHAT_MSG_LOOT(eventName, ...)

diff --git a/Features/LootDisplayProperties.lua b/Features/LootDisplayProperties.lua
@@ -29,8 +29,8 @@ function G_RLF.InitializeLootDisplayProperties(self)
 		return true
 	end
 
-	self.Show = function()
-		if self.isPassingFilter() then
+	self.Show = function(_, itemName, itemQuality)
+		if self:isPassingFilter(itemName, itemQuality) then
 			G_RLF.LootDisplay:ShowLoot(self)
 		end
 	end

diff --git a/LootDisplay/LootDisplayFrame/LootDisplayFrame.lua b/LootDisplay/LootDisplayFrame/LootDisplayFrame.lua
@@ -177,6 +177,7 @@ function LootDisplayFrameMixin:LeaseRow(key)
 
 	row:SetPosition(self)
 	C_Timer.After(0, function()
+		row:ResetHighlightBorder()
 		row:Show()
 	end)
 

diff --git a/LootDisplay/LootDisplayFrame/LootDisplayRow/LootDisplayRow.lua b/LootDisplay/LootDisplayFrame/LootDisplayRow/LootDisplayRow.lua
@@ -176,6 +176,9 @@ local function rowHighlightBorder(row)
 				fadeOut:SetToAlpha(0)
 				fadeOut:SetDuration(0.2)
 				fadeOut:SetStartDelay(0.3)
+				fadeOut:SetScript("OnFinished", function()
+					row:ResetHighlightBorder()
+				end)
 			end
 		end
 
@@ -413,8 +416,10 @@ function LootDisplayRowMixin:UpdateIcon(key, icon, quality)
 end
 
 function LootDisplayRowMixin:ResetFadeOut()
-	self.FadeOutAnimation:Stop()
-	self.FadeOutAnimation:Play()
+	C_Timer.After(0, function()
+		self.FadeOutAnimation:Stop()
+		self.FadeOutAnimation:Play()
+	end)
 end
 
 function LootDisplayRowMixin:ResetHighlightBorder()

diff --git a/LootDisplayProfiler.lua b/LootDisplayProfiler.lua
@@ -22,3 +22,58 @@ for k, v in pairs(LootDisplayRowMixin) do
 		LootDisplayRowMixin[k] = G_RLF:ProfileFunction(v, "LootDisplayRowMixin:" .. k)
 	end
 end
+
+-- Wrap all Feature modules' methods in the G_RLF:ProfileFunction method
+local ItemLoot = G_RLF.RLF:GetModule("ItemLoot")
+local Currency = G_RLF.RLF:GetModule("Currency")
+local Rep = G_RLF.RLF:GetModule("Reputation")
+local Xp = G_RLF.RLF:GetModule("Experience")
+local Money = G_RLF.RLF:GetModule("Money")
+
+for k, v in pairs(ItemLoot) do
+	if type(v) == "function" then
+		ItemLoot[k] = G_RLF:ProfileFunction(v, "ItemLoot:" .. k)
+	end
+end
+
+for k, v in pairs(Currency) do
+	if type(v) == "function" then
+		Currency[k] = G_RLF:ProfileFunction(v, "Currency:" .. k)
+	end
+end
+
+for k, v in pairs(Rep) do
+	if type(v) == "function" then
+		Rep[k] = G_RLF:ProfileFunction(v, "Rep:" .. k)
+	end
+end
+
+for k, v in pairs(Xp) do
+	if type(v) == "function" then
+		Xp[k] = G_RLF:ProfileFunction(v, "Xp:" .. k)
+	end
+end
+
+for k, v in pairs(Money) do
+	if type(v) == "function" then
+		Money[k] = G_RLF:ProfileFunction(v, "Money:" .. k)
+	end
+end
+
+-- Wrap all TestMode methods in the G_RLF:ProfileFunction method
+local TestMode = G_RLF.RLF:GetModule("TestMode")
+
+for k, v in pairs(TestMode) do
+	if type(v) == "function" then
+		TestMode[k] = G_RLF:ProfileFunction(v, "TestMode:" .. k)
+	end
+end
+
+-- Wrap all Logger methods in the G_RLF:ProfileFunction method
+local Logger = G_RLF.RLF:GetModule("Logger")
+
+for k, v in pairs(Logger) do
+	if type(v) == "function" then
+		Logger[k] = G_RLF:ProfileFunction(v, "Logger:" .. k)
+	end
+end
diff --git a/SmokeTest.lua b/SmokeTest.lua
@@ -3,7 +3,7 @@ local addonName, G_RLF = ...
 --@alpha@
 -- trunk-ignore-begin(no-invalid-prints/invalid-print)
 local TestMode = G_RLF.RLF:GetModule("TestMode")
-local testItems, testCurrencies, testFactions
+local testItems, testCurrencies, testFactions, testItem
 
 local tests = {}
 local prints = ""
@@ -26,6 +26,28 @@ local function assertEqual(actual, expected, testName, err)
 	end
 end
 
+local function testGetItemInfo(id)
+	-- Get the id of the last element in testItems
+	local itemName, itemLink, itemQuality, itemLevel, itemMinLevel, itemType, itemSubType, itemStackCount, itemEquipLoc, itemTexture, sellPrice, classID, subclassID, bindType, expansionID, _, isCraftingReagent =
+		C_Item.GetItemInfo(id)
+	assertEqual(itemName ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemName")
+	assertEqual(itemLink ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemLink")
+	assertEqual(itemQuality ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemQuality")
+	assertEqual(itemLevel ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemLevel")
+	assertEqual(itemMinLevel ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemMinLevel")
+	assertEqual(itemType ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemType")
+	assertEqual(itemSubType ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemSubType")
+	assertEqual(itemStackCount ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemStackCount")
+	assertEqual(itemEquipLoc ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemEquipLoc")
+	assertEqual(itemTexture ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").itemTexture")
+	assertEqual(sellPrice ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").sellPrice")
+	assertEqual(classID ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").classID")
+	assertEqual(subclassID ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").subclassID")
+	assertEqual(bindType ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").bindType")
+	assertEqual(expansionID ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").expansionID")
+	assertEqual(isCraftingReagent ~= nil, true, "Global: C_Item.GetItemInfo(" .. id .. ").isCraftingReagent")
+end
+
 local function testWoWGlobals()
 	assertEqual(type(EventRegistry), "table", "Global: EventRegistry")
 	assertEqual(type(C_CVar.SetCVar), "function", "Global C_CVar.SetCVar")
@@ -62,25 +84,13 @@ local function testWoWGlobals()
 	assertEqual(type(UnitLevel), "function", "Global: UnitLevel")
 	assertEqual(type(GetPlayerGuid), "function", "Global: GetPlayerGuid")
 	assertEqual(type(C_Item.GetItemInfo), "function", "Global: C_Item.GetItemInfo")
-	local itemName, itemLink, itemQuality, itemLevel, itemMinLevel, itemType, itemSubType, itemStackCount, itemEquipLoc, itemTexture, sellPrice, classID, subclassID, bindType, expansionID, setID, isCraftingReagent =
-		C_Item.GetItemInfo(34494)
-	assertEqual(itemName ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemName")
-	assertEqual(itemLink ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemLink")
-	assertEqual(itemQuality ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemQuality")
-	assertEqual(itemLevel ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemLevel")
-	assertEqual(itemMinLevel ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemMinLevel")
-	assertEqual(itemType ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemType")
-	assertEqual(itemSubType ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemSubType")
-	assertEqual(itemStackCount ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemStackCount")
-	assertEqual(itemEquipLoc ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemEquipLoc")
-	assertEqual(itemTexture ~= nil, true, "Global: C_Item.GetItemInfo(34494).itemTexture")
-	assertEqual(sellPrice ~= nil, true, "Global: C_Item.GetItemInfo(34494).sellPrice")
-	assertEqual(classID ~= nil, true, "Global: C_Item.GetItemInfo(34494).classID")
-	assertEqual(subclassID ~= nil, true, "Global: C_Item.GetItemInfo(34494).subclassID")
-	assertEqual(bindType ~= nil, true, "Global: C_Item.GetItemInfo(34494).bindType")
-	assertEqual(expansionID ~= nil, true, "Global: C_Item.GetItemInfo(34494).expansionID")
-	assertEqual(setID == nil, true, "Global: C_Item.GetItemInfo(34494).setID")
-	assertEqual(isCraftingReagent ~= nil, true, "Global: C_Item.GetItemInfo(34494).isCraftingReagent")
+	local id = testItems[#testItems].id
+	local isCached = C_Item.GetItemInfo(id) ~= nil
+	if not isCached then
+		G_RLF:Print("Item not cached, skipping GetItemInfo test")
+	else
+		testGetItemInfo(id)
+	end
 	assertEqual(type(GetMoney), "function", "Global: GetMoney")
 	assertEqual(type(C_CurrencyInfo.GetCoinTextureString), "function", "Global: C_CurrencyInfo.GetCoinTextureString")
 	assertEqual(type(FACTION_STANDING_INCREASED), "string", "Global: FACTION_STANDING_INCREASED")
@@ -116,43 +126,67 @@ local function testWoWGlobals()
 	assertEqual(type(FACTION_BAR_COLORS), "table", "Global: FACTION_BAR_COLORS")
 end
 
-local function runTestSafely(testFunction, testName)
-	local success, err = pcall(testFunction)
+local function runTestSafely(testFunction, testName, ...)
+	local success, err = pcall(testFunction, ...)
 	assertEqual(success, true, testName, err)
 end
 
-local function testLootDisplay()
-	local module, e, testObj, amountLooted
-	module = G_RLF.RLF:GetModule("Experience")
-	e = module.Element:new(1337)
+local function runExperienceSmokeTest()
+	local module = G_RLF.RLF:GetModule("Experience")
+	local e = module.Element:new(1337)
 	runTestSafely(e.Show, "LootDisplay: Experience")
-	module = G_RLF.RLF:GetModule("Money")
-	e = module.Element:new(12345)
+end
+
+local function runMoneySmokeTest()
+	local module = G_RLF.RLF:GetModule("Money")
+	local e = module.Element:new(12345)
 	runTestSafely(e.Show, "LootDisplay: Money")
-	module = G_RLF.RLF:GetModule("ItemLoot")
-	testObj = testItems[2]
-	amountLooted = 1
-	e = module.Element:new(testObj.id, testObj.link, testObj.icon, amountLooted)
-	runTestSafely(e.Show, "LootDisplay: Item")
-	e = module.Element:new(testObj.id, testObj.link, testObj.icon, amountLooted)
-	runTestSafely(e.Show, "LootDisplay: Item Quantity Update")
-	module = G_RLF.RLF:GetModule("Currency")
-	testObj = testCurrencies[2]
-	e = module.Element:new(testObj.id, testObj.link, testObj.icon, amountLooted)
+end
+
+local function runItemLootSmokeTest()
+	local module = G_RLF.RLF:GetModule("ItemLoot")
+	local testObj = testItems[2]
+	local amountLooted = 1
+	local e = module.Element:new(testObj.id, testObj.link, testObj.icon, amountLooted)
+	if testObj.name == nil then
+		G_RLF:Print("Item not cached, skipping ItemLoot test")
+	else
+		runTestSafely(e.Show, "LootDisplay: Item", e, testObj.name, testObj.quality)
+		e = module.Element:new(testObj.id, testObj.link, testObj.icon, amountLooted)
+		runTestSafely(e.Show, "LootDisplay: Item Quantity Update", e, testObj.name, testObj.quality)
+	end
+end
+
+local function runCurrencySmokeTest()
+	local module = G_RLF.RLF:GetModule("Currency")
+	local testObj = testCurrencies[2]
+	local amountLooted = 1
+	local e = module.Element:new(testObj.id, testObj.link, testObj.icon, amountLooted)
 	runTestSafely(e.Show, "LootDisplay: Currency")
 	e = module.Element:new(testObj.id, testObj.link, testObj.icon, amountLooted)
 	runTestSafely(e.Show, "LootDisplay: Currency Quantity Update")
-	module = G_RLF.RLF:GetModule("Reputation")
-	testObj = testFactions[2]
-	amountLooted = 664
-	e = module.Element:new(amountLooted, testObj)
+end
+
+local function runReputationSmokeTest()
+	local module = G_RLF.RLF:GetModule("Reputation")
+	local testObj = testFactions[2]
+	local amountLooted = 664
+	local e = module.Element:new(amountLooted, testObj)
 	runTestSafely(e.Show, "LootDisplay: Reputation")
 	e = module.Element:new(amountLooted, testObj)
 	runTestSafely(e.Show, "LootDisplay: Reputation Quantity Update")
 end
 
+local function testLootDisplay()
+	runExperienceSmokeTest()
+	runMoneySmokeTest()
+	runItemLootSmokeTest()
+	runCurrencySmokeTest()
+	runReputationSmokeTest()
+end
+
 function TestMode:SmokeTest(...)
-	testItems, testCurrencies, testFactions = ...
+	testItems, testCurrencies, testFactions, testItem = ...
 
 	tests = {}
 	prints = ""