Strange Talent Frame Error.

kaytotes · 12-01-14, 01:15 PM

One of my users has reported an error that I can't seem to be able to track down. It seems to be pointing at the Blizzard Talent Frame but I'm in no way hooking into it and I think (hope) I'm largely taint free. Instead of pastebin I hope its okay to point directly at my github for code. It appears to occur with every other addon disabled besides my BlizzImp.

https://github.com/uutfboomheadshot/...aster/BlizzImp

This is the full error :

Code:

Message: ...rface\AddOns\Blizzard_TalentUI\Blizzard_TalentUI.lua:717: attempt to concatenate local 'tier' (a nil value)
Time: 12/01/14 19:36:07
Count: 2
Stack: ...rface\AddOns\Blizzard_TalentUI\Blizzard_TalentUI.lua:717: in function `PlayerTalentFrame_SelectTalent'
...rface\AddOns\Blizzard_TalentUI\Blizzard_TalentUI.lua:40: in function `OnAccept'
Interface\FrameXML\StaticPopup.lua:3945: in function `StaticPopup_OnClick'
[string "*:OnClick"]:1: in function <[string "*:OnClick"]:1>

Locals: tier = nil
id = nil
(*temporary) = PlayerTalentFrameTalents {
0 = <userdata>
clearInfo = PlayerTalentFrameTalentsClearInfoFrame {
}
tier2 = PlayerTalentFrameTalentsTalentRow2 {
}
unspentText = <unnamed> {
}
talentGroup = 2
tier3 = PlayerTalentFrameTalentsTalentRow3 {
}
summariesShownWhenNoPrimary = true
learnButton = PlayerTalentFrameTalentsLearnButton {
}
tier5 = PlayerTalentFrameTalentsTalentRow5 {
}
MainHelpButton = PlayerTalentFrameTalentsTutorialButton {
}
tier7 = PlayerTalentFrameTalentsTalentRow7 {
}
tier1 = PlayerTalentFrameTalentsTalentRow1 {
}
tier4 = PlayerTalentFrameTalentsTalentRow4 {
}
bg = PlayerTalentFrameTalentsBg {
}
tier6 = PlayerTalentFrameTalentsTalentRow6 {
}
}
(*temporary) = "tier"
(*temporary) = nil
(*temporary) = nil
(*temporary) = "attempt to concatenate local 'tier' (a nil value)"

Phanx · 12-01-14, 01:45 PM

This seems the most likely issue:

Code:

	-- Target Frame Smooth Fade
	if( UnitExists("target") ) then
		if( tFrameHidden == true ) then
			UIFrameFadeIn( TargetFrame, fadeTime, 0, 1 );
			tFrameHidden = false;
		end
	else
		UIFrameFadeOut( TargetFrame, fadeTime, 1, 0 );
		tFrameHidden = true;
	end

UIFrameFadeIn and UIFrameFadeOut are used by the default UI on secure frames, and calling them from addon code introduces taint into the entire system, which can break other parts of the default UI. This has (historically, at least) been a major issue with the glyph frame, and since that's attached to the talent frame, it seems possible it's related.

========================================================================

Some other issues:

Code:

	PVPReadyDialog.leaveButton.Show = function() end -- Fixes button not disappearing (Why?)

This may taint the entire dialog, since you are overwriting a method, which could potentially prevent the rest of it from working; everything related to PVP queues is annoyingly delicate. A better solution would be:

Code:

	PVPReadyDialog.leaveButton:HookScript("OnShow", PVPReadyDialog.leaveButton.Hide)

========================================================================

Code:

ConsoleExec( "CameraDistanceMaxFactor 9" )

There's nothing wrong with this, but it would be better to just do it directly:

Code:

SetCVar("CameraDistanceMaxFactor", 9)

========================================================================

Code:

		if(bRepair == true) then
			if(repCost <= GetMoney()) then
				RepairAllItems();
				print("|cffffff00Items Repaired");
			end
		end

Again, nothing wrong, per se, but you should just merge those if statements:

Code:

		if(bRepair == true) and (repCost <= GetMoney()) then
			RepairAllItems();
			print("|cffffff00Items Repaired");
		end

========================================================================

Code:

	local _,_,iQuality,_,_,_,_,_,_,_,iPrice = GetItemInfo( itemLink );
	local _, iCount = GetContainerItemInfo( bags, bagSlot );

Globals are bad -- especially a global _ which can break the entire UI if Blizzard leaks a global _ too. It happened around the beginning of Cataclysm, with cataclysmic (lol) results.

========================================================================

In blizzImpBars.lua:

Code:

	if ( visibilityChanged ) then
			UIParent_ManageFramePositions();
			UpdateContainerFrameAnchors();
		end

Calling UIParent_ManageFramePositions from an addon may cause problems, and will certainly cause problems if you do it in combat, since that function moves secure frames.

========================================================================

In blizzImpPVP.lua, another dangerous global leak:

Code:

message = CreateFrame( "MessageFrame", "CustomMessageFrame", UIParent );

This is actually doubly dangerous, as you're not only leaking a global "message", but also giving a frame a global name of "CustomMessageFrame". Global variables, including frame names, should always (a) be unique and (b) clearly identify which addon they belong to, so they won't collide with other addons' globals (or the default UI's globals) and so that if they appear in an error message or in a /fstack tooltip, the user knows which addon to blame for them.

And a less dangerous one:

Code:

local function InitMessage()
		killBlowCount = 0;

========================================================================

In several places in several files, I see you're manually keeping track of the combat state by registering for PLAYER_REGEN_DISABLED and PLAYER_REGEN_ENABLED and toggling a boolean; it would be a lot simpler (and more accurate) to just check InCombatLockdown().

========================================================================

In blizzImpUF.lua:

Code:

	local isInVehicle = UnitControllingVehicle("player");
	if( isInVehicle == true ) then
		SetUnitFrames();
	end

	if ( UnitHasVehiclePlayerFrameUI("player") ) then
		SetUnitFrames();
	end

Again, not a problem, just unnecessarily duplicative. Compare:

Code:

	if( UnitControllingVehicle("player") or UnitHasVehiclePlayerFrameUI("player") ) then
		SetUnitFrames();
	end

kaytotes · 12-01-14, 03:36 PM

Thank you for the code review Phanx. I've cleaned up those 3 global issues and merged the if statements.

I'd removed UIFrameFadeIn / Out in my local build trying to solve it to no avail but it turns out the taint was caused by ToggleTalentFrame() in my Micro Menu code.

Much appreciated!