changeset 89:939569ded66f

- Also include revision numbers when sending/displaying version data. - Do a tweaked variant of the "ensure useful unique tags" code in _addLootEntry. - When debugging comms, send to raid *and* guild instead of either. - Remove flaky optimization in fixup_unique_replacements. Thanks to Hyndron for helping me test this with genuine non-contrived data.
author Farmbuyer of US-Kilrogg <farmbuyer@gmail.com>
date Thu, 28 Jun 2012 19:44:53 +0000
parents c9f955f9a285
children 92e0db376858
files core.lua gui.lua
diffstat 2 files changed, 110 insertions(+), 92 deletions(-) [+]
line wrap: on
line diff
--- a/core.lua	Fri Jun 22 02:39:22 2012 +0000
+++ b/core.lua	Thu Jun 28 19:44:53 2012 +0000
@@ -165,7 +165,7 @@
 }
 local my_name				= UnitName('player')
 local comm_cleanup_ttl		= 4   -- seconds in the communications cache
-local revision_large		= nil -- defaults to 1, possibly changed by revision
+local version_large			= nil -- defaults to 1, possibly changed by version
 local g_LOOT_ITEM_ss, g_LOOT_ITEM_MULTIPLE_sss, g_LOOT_ITEM_SELF_s, g_LOOT_ITEM_SELF_MULTIPLE_ss
 
 
@@ -177,10 +177,14 @@
 do local _G = _G setfenv (1, addon)
 
 	commrev			= '17'
-	revision		= _G.GetAddOnMetadata(nametag,"Version") or "?"  -- "x.yy.z", etc
+	version			= _G.GetAddOnMetadata(nametag,"Version") or "?"  -- "x.yy.z", etc
 	ident			= "OuroLoot2"
 	identTg			= "OuroLoot2Tg"
 	status_text		= nil
+	revision		= "@project-revision@"
+	--@debug@
+	revision		= "DEVEL"
+	--@end-debug@
 
 	tekdebug		= nil
 	if _G.tekDebug then
@@ -383,14 +387,14 @@
 -- integral form for comparison.  The result doesn't need to be meaningful as
 -- long as we can reliably feed two of them to "<" and get useful answers.
 --
--- This makes/reinforces an assumption that revision_large of release packages
+-- This makes/reinforces an assumption that version_large of release packages
 -- (e.g., 2016001) will always be higher than those of development packages
 -- (e.g., 87), due to the tagging system versus subversion file revs.  This
 -- is good, as local dev code will never trigger a false positive update
 -- warning for other users.
 do
 	local r = 0
-	for d in addon.revision:gmatch("%d+") do
+	for d in addon.version:gmatch("%d+") do
 		r = 1000*r + d
 	end
 	-- If it's a big enough number to obviously be a release, then make
@@ -398,7 +402,7 @@
 	while r > 2000 and r < 2000000 do
 		r = 1000*r
 	end
-	revision_large = math.max(r,1)
+	version_large = math.max(r,1)
 end
 
 -- Hypertext support, inspired by DBM broadcast pizza timers
@@ -872,7 +876,7 @@
 	end
 
 	_init(self)
-	self.dprint('flow', "version strings:", revision_large, self.status_text)
+	self.dprint('flow', "version strings:", version_large, self.revision, self.status_text)
 	self.load_assert = nil
 	self.OnInitialize = nil   -- free up ALL the things!
 end
@@ -1049,21 +1053,20 @@
 	--	print("created plugin", plugin:GetName())
 	--end
 
-	local olrev = tonumber("@project-revision@") or 0
 	local err = [[Module '%s' cannot register itself because it failed a required condition: '%s']]
 	function addon:ConstrainedNewModule (modname, minrev, mincomm, mindata)
 		if not addon.author_debug then
-			if minrev and minrev > olrev then
+			if minrev and tonumber(minrev) > (tonumber(self.revision) or math.huge) then
 				self:Print(err,modname,
-					"revision "..olrev.." older than minimum "..minrev)
+					"revision "..self.revision.." older than minimum "..minrev)
 				return false
 			end
-			if mincomm and mincomm > tonumber(self.commrev) then
+			if mincomm and tonumber(mincomm) > tonumber(self.commrev) then
 				self:Print(err,modname,
 					"commrev "..self.commrev.." older than minimum "..mincomm)
 				return false
 			end
-			if mindata and mindata > opts.datarev then
+			if mindata and tonumber(mindata) > opts.datarev then
 				self:Print(err,modname,
 					"datarev "..opts.datarev.." older than minimum "..mindata)
 				return false
@@ -1543,9 +1546,9 @@
 				end
 			end
 
+			if not itemstring then return end    -- "PlayerX selected Greed", etc, not looting
 			self.dprint('loot', "CHAT_MSG_LOOT, person is", person,
 				", itemstring is", itemstring, ", count is", count)
-			if not itemstring then return end    -- "PlayerX selected Greed", etc, not looting
 
 			-- Name might be colorized, remove the highlighting
 			if person then
@@ -1737,7 +1740,7 @@
 	self:Print("Now %s; threshold currently %s.",
 		self.enabled and "tracking" or "only broadcasting",
 		self.thresholds[self.threshold])
-	self:broadcast('revcheck',revision_large)
+	self:broadcast('revcheck',version_large)
 end
 
 -- Note:  running '/loot off' will also avoid the popup reminder when
@@ -1847,10 +1850,17 @@
 			-- how did this happen?
 			return
 		end
+		if source == my_name then
+			source = _G.UNIT_YOU
+		end
 		local from_text, to_text
 		if from_whom then
+			if from_whom == my_name then
+				from_whom = _G.UNIT_YOU
+			end
 			from_text = addon:colorize (from_whom, from_class)
-			to_text = addon:colorize (e.person, e.person_class)
+			to_text = e.person == my_name and _G.UNIT_YOU or e.person
+			to_text = addon:colorize (to_text, e.person_class)
 		else
 			if olddisp then
 				from_text = addon.disposition_colors[olddisp].text
@@ -1910,18 +1920,18 @@
 	self.sender_list.active = {}
 	self.sender_list.names = {}
 	self:broadcast('ping')
-	self:broadcast('revcheck',revision_large)
+	self:broadcast('revcheck',version_large)
 end
 
-function addon:_check_revision (otherrev)
+function addon:_check_version (otherrev)
 	self.dprint('comm', "revchecking against", otherrev)
 	otherrev = tonumber(otherrev)
-	if otherrev == revision_large then
+	if otherrev == version_large then
 		-- normal case
 
-	elseif otherrev < revision_large then
+	elseif otherrev < version_large then
 		self.dprint('comm', "ours is newer, notifying")
-		self:broadcast('revcheck',revision_large)
+		self:broadcast('revcheck',version_large)
 
 	else
 		self.dprint('comm', "ours is older, (possibly) yammering")
@@ -2043,8 +2053,8 @@
 		possible_st:SetData(g_loot)
 	end
 
-	self.status_text = ("%s communicating as ident %s commrev %s"):
-		format (self.revision, self.ident, self.commrev)
+	self.status_text = ("%s(r%s) communicating as ident %s commrev %s"):
+		format (self.version, self.revision, self.ident, self.commrev)
 	self:RegisterComm(self.ident)
 	self:RegisterComm(self.identTg, "OnCommReceivedNocache")
 
@@ -2329,6 +2339,8 @@
 	end
 
 	-- Adding anything original to g_loot goes through this routine.
+	-- More precisely, anything new on the EOI tab hits this; it does not
+	-- necessarily need to be a looted item.
 	function addon._addLootEntry (e)
 		setmetatable(e,loot_entry_mt)
 
@@ -2340,6 +2352,11 @@
 		e.hour = h
 		e.minute = m
 		e.stamp = time_t --localuptime
+		if e.kind == 'loot' then
+			if (not e.unique) or (#e.unique==0) then
+				e.unique = e.id .. (e.disposition or e.person) .. _G.date("%Y/%m/%d %H:%M",e.stamp)
+			end
+		end
 		local index = #g_loot + 1
 		g_loot[index] = e
 		return index
@@ -2463,16 +2480,6 @@
 				info[i] = nil
 			end
 			pprint('improv', "final:", winner[1], winner[2])
-			--@debug@
-			-- purely a sanity check
-			if winning_index == 1 then
-				if winner[2] ~= info.LOCAL then
-					pprint('improv',
-						"things locally generated are not locally generated?",
-						info.LOCAL, winner[2])
-				end
-			end
-			--@end-debug@
 			--[[
 			A:  winner was generated locally
 			   >winning_index == 1
@@ -2480,57 +2487,57 @@
 			B:  winner was generated remotely
 			   >need to scan and replace
 			Detecting A is strictly an optimization.  We should be able to do
-			this code safely in all cases.
+			this code safely in all cases.  Important to note:  a local winner
+			will always be at index 1, but a winner at index 1 does not necessarily
+			mean it was locally generated (e.g., if the local itemfilter drops
+			it but a remote player does an improv).  Just do the general case
+			until/unless this becomes a problem.
 			]]
-			if
---@debug@
-true or
---@end-debug@
-				winning_index ~= 1 then
-				--XXX this branch still needs to be tested with live data
-				local cache = g_uniques:SEARCH(exist)
-				local looti,hi,ui = cache.loot, cache.history, cache.history_may
-
-				-- Active loot
-				if looti and g_loot[looti].unique == exist then
-					pprint('improv', "found and replaced loot entry", looti)
-					g_loot[looti].unique = winner[2]
+--XXX this branch still needs to be tested with live data
+			local cache = g_uniques:SEARCH(exist)
+			local looti,hi,ui = cache.loot, cache.history, cache.history_may
+
+			-- Active loot
+			if looti and g_loot[looti].unique == exist then
+				pprint('improv', "found and replaced loot entry", looti)
+				g_loot[looti].unique = winner[2]
+			else
+				-- If sharded, filtered, or the improv was done by the local
+				-- player, then the "previous" unique would not have made it
+				-- into the tables to begin with.  So don't flag an error.
+				pprint('improv', "No active loot found", looti,
+					looti and g_loot[looti].unique, winning_index)
+			end
+
+			-- History
+			if hi ~= g_uniques.NOTFOUND then
+				hi = addon.history.byname[hi]
+				local hist = addon.history[hi]
+				if ui and hist.unique[ui] == exist then
+					-- ui is valid
 				else
-					-- If winning_index == 1, the "previous" unique would not
-					-- have made it into the tables to begin with.  So don't
-					-- flag an error.  Still should be looked at.
-					pprint('improv', "Um.", looti, g_loot[looti].unique, winning_index)
-				end
-
-				-- History
-				if hi ~= g_uniques.NOTFOUND then
-					hi = addon.history.byname[hi]
-					local hist = addon.history[hi]
-					if ui and hist.unique[ui] == exist then
-						-- ui is valid
-					else
-						ui = nil
-						for i,ui2 in ipairs(hist.unique) do
-							if ui2 == exist then
-								ui = i
-								break
-							end
+					ui = nil
+					for i,ui2 in ipairs(hist.unique) do
+						if ui2 == exist then
+							ui = i
+							break
 						end
 					end
-					if ui then
-						pprint('improv', "found and replacing history entry", hi,
-							ui, hist.name)
-						assert(exist ~= winner[2])
-						hist.when[winner[2]] = hist.when[exist]
-						hist.id[winner[2]] = hist.id[exist]
-						hist.count[winner[2]] = hist.count[exist]
-						hist.unique[ui] = winner[2]
-						hist.when[exist] = nil
-						hist.id[exist] = nil
-						hist.count[exist] = nil
-					end
+				end
+				if ui then
+					pprint('improv', "found and replacing history entry", hi,
+						ui, hist.name)
+					assert(exist ~= winner[2])
+					hist.when[winner[2]] = hist.when[exist]
+					hist.id[winner[2]] = hist.id[exist]
+					hist.count[winner[2]] = hist.count[exist]
+					hist.unique[ui] = winner[2]
+					hist.when[exist] = nil
+					hist.id[exist] = nil
+					hist.count[exist] = nil
 				end
 			end
+
 			pprint('improv', "finished with", exist, "into", winner[2])
 			flib.del(winner)
 			flib.del(info)
@@ -2817,9 +2824,14 @@
 					g_uniques[e.unique] = { loot = i, history = g_uniques.NOTFOUND }
 				end
 			else
+				-- The usual cause: when only source is from an older client
+				-- and the disposition did not trigger addhistory, then not
+				-- even a stub history entry happens.  Code has now been added
+				-- to try harder to prevent this, but it's still best to not
+				-- simple ignore it.
 				trouble = true
-				pprint('loot', "ERROR precache loop found bad unique tag!",
-					i, "tag", tostring(e.unique), "from?", tostring(e.bcast_from))
+				pprint('loot', "ERROR precache loop found missing/outdated unique tag!",
+					i, "tag <"..tostring(e.unique).."> from?", tostring(e.bcast_from))
 			end
 		end
 		UpdateAddOnMemoryUsage()
@@ -2830,7 +2842,8 @@
 			self:Print("Note that there were inconsistencies in the data;",
 				"you should consider submitting a bug report (including your",
 				"SavedVariables file), and regenerating or preening this",
-				"realm's loot history.")
+				"realm's loot history.  If you keep seeing this message, type",
+				"'/ouroloot fix ?' and try some of those actions.")
 		end
 		g_uniques:SETMODE('probe')
 		self._cache_history_uniques = nil
@@ -3345,7 +3358,7 @@
 		if n > 0 then
 			local msg = {t,...}
 			-- tconcat requires strings, but T is known to be one already
-			-- can't use #msg since there might be holes
+			-- can't use #msg since there might be nil holes
 			for i = 2, n+1 do
 				msg[i] = tostring(msg[i] or "")
 			end
@@ -3362,8 +3375,11 @@
 	function addon:broadcast(tag,...)
 		local msg = assemble(tag,...)
 		self.dprint('comm', "<broadcast>:", msg)
-		-- the "GUILD" here is just so that we can also pick up on it
-		self:SendCommMessage(self.ident, msg, self.debug.comm and "GUILD" or "RAID")
+		self:SendCommMessage(self.ident, msg, "RAID")
+		-- this is what lets us debug our own message traffic:
+		if self.debug.comm then
+			self:SendCommMessage(self.ident, msg, "GUILD")
+		end
 	end
 	-- whispercast(<to>, 'tag', <stuff>)
 	function addon:whispercast(to,...)
@@ -3384,17 +3400,19 @@
 
 	OCR_funcs.ping = function (sender)
 		pprint('comm', "incoming ping from", sender)
-		addon:whispercast (sender, 'pong', addon.revision, 
-			addon.enabled and "tracking" or (addon.rebroadcast and "broadcasting" or "disabled"))
+		local what = addon.enabled and "tracking" or
+			(addon.rebroadcast and "broadcasting" or "disabled")
+		addon:whispercast (sender, 'pong', addon.version, what, addon.revision)
 	end
-	OCR_funcs.pong = function (sender, _, rev, status)
-		local s = ("|cff00ff00%s|r %s is |cff00ffff%s|r"):format(sender,rev,status)
+	OCR_funcs.pong = function (sender, _, ver, status, opt_rev)
+		local s = ("|cff00ff00%s|r %s(r%s) is |cff00ffff%s|r"):
+			format (sender, ver, opt_rev or "?", status)
 		addon:Print("Echo: ", s)
 		adduser (sender, s, status=="tracking" or status=="broadcasting" or nil)
 	end
 	OCR_funcs.revcheck = function (sender, _, revlarge)
 		addon.dprint('comm', "revcheck, sender", sender)
-		addon:_check_revision (revlarge)
+		addon:_check_version (revlarge)
 	end
 
 	OCR_funcs['17improv'] = function (sender, _, senderid, existing, replace)
@@ -3505,7 +3523,7 @@
 				local d = select(i,...)
 				OCR_data[i] = (d ~= "") and d or nil
 			end
-			addon.dprint('comm', ":... processing", tag, "from", sender, "with arg count", n)
+			addon.dprint('comm', ":...processing", tag, "from", sender, "with arg count", n)
 			return f(sender,tag,unpack(OCR_data,1,n))
 		end
 		addon.dprint('comm', "unknown comm message", tag, "from", sender)
--- a/gui.lua	Fri Jun 22 02:39:22 2012 +0000
+++ b/gui.lua	Thu Jun 28 19:44:53 2012 +0000
@@ -741,7 +741,7 @@
 	g_uniques = assert(uniques_pointer, "something went wrong at startup")
 	g_generated = nil
 	tabgroup_tabs = {}
-	window_title = "Ouro Loot " .. self.revision
+	window_title = "Ouro Loot " .. self.version
 	-- TabGroup stretches out the tabs to fill the row but only if >75% of the
 	-- row is already full.  It turns out that not doing this looks like ass.
 	-- If we won't have enough tabs to trigger this on its own, pad out the tab
@@ -2179,6 +2179,7 @@
 			local pair = GUI:Create("InlineGroup")
 			pair:SetLayout("List")
 			pair:SetRelativeWidth(0.49)
+			pair:SetTitle("Keybinding for '/ouroloot'")
 			local editbox, checkbox
 			editbox = mkbutton("EditBox", nil, opts.keybinding_text,
 				[[Keybinding text format is fragile (ALT then CTRL then SHIFT)!  Relog to take effect.]])
@@ -2794,7 +2795,7 @@
 	OnShow = function (dialog, addon)
 		local thistable = StaticPopupDialogs[dialog.which]
 		-- StaticPopup_Resize does not take extraFrame into account, so we
-		-- hook the sizing method that _Resize calls at the end.
+		-- monkeypatch the sizing method that _Resize calls at the end.
 		dialog.saved_setheight = dialog.SetHeight
 		dialog.SetHeight = function (d, h)
 			return d.saved_setheight(d,h+35)
@@ -2878,9 +2879,7 @@
 	OnAlt = function (dialog, addon)
 		-- hitting escape also calls this, but the 3rd arg would be "clicked"
 		-- in both cases, not useful here.
-		local helpbutton = dialog.button2
-		local ismousing = MouseIsOver(helpbutton)
-		if ismousing then
+		if MouseIsOver(dialog.button3) then
 			-- they actually clicked the button (or at least the mouse was over "Help"
 			-- when they hit escape... sigh)
 			addon:BuildMainDisplay('help')