Errors on UI reload

[Gnarfoz]

This is in reference to #4.

I still get the all-black artifact window on UI reloads. While the current version of the library itself does not throw errors anymore in this situation, it still causes them. That blame is non-obvious because the stack trace only contains Blizzard code:

10x ...eBlizzard_ArtifactUI\Blizzard_ArtifactUI.lua:41: attempt to compare number with nil
...eBlizzard_ArtifactUI\Blizzard_ArtifactUI.lua:41: in function `ArtifactUI_CanViewArtifact'
...ddOns\Blizzard_ArtifactUI\Blizzard_ArtifactPerks.lua:683: in function `EvaluateRelics'
...ddOns\Blizzard_ArtifactUI\Blizzard_ArtifactPerks.lua:533: in function <...ddOns\Blizzard_ArtifactUI\Blizzard_ArtifactPerks.lua:531>
[C]: in function `SetShown'
...eBlizzard_ArtifactUI\Blizzard_ArtifactUI.lua:172: in function `SetTab'
...eBlizzard_ArtifactUI\Blizzard_ArtifactUI.lua:138: in function `EvaulateForgeState'
...eBlizzard_ArtifactUI\Blizzard_ArtifactUI.lua:70: in function <...eBlizzard_ArtifactUI\Blizzard_ArtifactUI.lua:67>
[C]: in function `Show'
FrameXML\UIParent.lua:2374: in function `SetUIPanel'
FrameXML\UIParent.lua:2192: in function `ShowUIPanel'
FrameXML\UIParent.lua:2086: in function <FrameXML\UIParent.lua:2082>
[C]: in function `SetAttribute'
FrameXML\UIParent.lua:2868: in function `ShowUIPanel'
FrameXML\UIParent.lua:1498: in function <FrameXML\UIParent.lua:907>

Locals:
(*temporary) = nil
(*temporary) = false
(*temporary) = "attempt to compare number with nil"

However, discussion with the helpful Blizzard UI people on #wowuidev IRC revealed that C_ArtifactUI.GetRelicInfo and C_ArtifactUI.GetEquippedArtifactRelicInfo cause ARTIFACT_UPDATE (without the newItem arg) events to fire, which causes UIParent to load the ArtifactUI in a situation where no artifact is selected, causing C_ArtifactUI.GetTotalPurchasedRanks to return nil until one is selected again (by opening the window or visiting the forge).
...at least that's my understanding. If I entirely grasped this, I would have sent a pull request instead of opening an issue. ;-)

You seem to already have taken precautions, and the only path to a call to ScanRelics() without first going through PrepareForScan() is in private.ARTIFACT_UPDATE when the newItem arg is not true. (But that would mean that event is firing without that arg for some other reason than calls to C_ArtifactUI.GetRelicInfo.)

To make this less rambling:
They suggested, as an ugly workaround, to hook the OnShow of the artifact UI and if C_ArtifactUI.GetTotalPurchasedRanks returns nil at that time, then immediately hide the window and... return right there. The goal being to avoid EvaulateForgeState() from being called.
In essence, adding a if not C_ArtifactUI.GetTotalPurchasedRanks() then ArtifactFrame:Hide() return end right here https://github.com/Gethe/wow-ui-source/blob/live/AddOns/Blizzard_ArtifactUI/Blizzard_ArtifactUI.lua#L68.

I hope this makes sense.

[Gnarfoz]

After some fiddling, this seems to do the trick for me. I just put it in the main chunk of my "collection of random things" addon, since it only touches stuff defined in UIParent.lua. No need to wait for ADDON_LOADED.

--C_ArtifactUI.GetTotalPurchasedRanks() shenanigans
local orig_show
local new_show
local counter = 0

local function new_show(self)
    if C_ArtifactUI.GetTotalPurchasedRanks() then 
        orig_show(self)
    else
        ArtifactFrame:Hide()
        counter = counter + 1
        print ("GTPR is nil, count: " ..counter)
    end
end

local function hook_artifact_show()
    if not orig_show then
        orig_show = ArtifactFrame:GetScript("OnShow")
        ArtifactFrame:SetScript("OnShow", new_show)
    end
end
hooksecurefunc("ArtifactFrame_LoadUI", hook_artifact_show)

(Never mind the debug print.)

No more black window on UI reload!

[Rainrider]

I'm not sure I follow. C_ArtifactUI.GetRelicInfo() does not raise any event by itself. When ARTIFACT_UPDATE fires without its first argument, it means changes to the currently viewed/available artifact (and there should have been an ARTIFACT_UPDATE with true before that as you can't change relics or traits without opening the artifact ui first).

I posted a guess on the WoWI forum about what might be causing all this and sadly haven't got a reply.

I do get the error myself but can't reproduce it reliably. Can you confirm LAD is the only reason for the error to occur? If not, maybe this is more suitable for BlizzBugsSuck.

Also, who are those Blizzard UI people on #wowuidev?

[Gnarfoz]

Yeah, it's entirely unintuitive because there's no way to know. But that's exactly what's happening, using C_ArtifactUI.GetRelicInfo or C_ArtifactUI.GetEquippedArtifactRelicInfo causes ARTIFACT_UPDATE to fire.

TheDanW and Guill are Blizzard UI guys in #wowuidev, and I talked through this mostly with TheDanW.
My initial post was basically a summary of our conversation. You can find logs of it here, starting from 22:09.20. Just Ctrl-F for TheDanW and you'll quickly find the interesting bits.

I'm using LAD with @Hekili's eponymous addon. The author had replaced the original version with a slightly modified one a while ago, but I made sure to replace it with the current 1.1.1 version of LAD (minor version: 11).
I disabled all other addons that use C_Artifact.anything or ArtifactFrame or ArtifactUI. (RelicInspector, Broker Artifact, ArtifactTraitLink, AskMrRobot. They either don't use the offending functions or only do things in response to clicking things, anyway.)
Only disabling Hekili (and with it, LAD) made a difference.

Using the aforementioned workaround (suggested by TheDanW) I can see that something is causing the ArtifactFrame to be shown quite a bunch of times on load/reload. Not sure what that could be if not LAD? I disabled everything else...
If you don't see how that would be possible (I don't see it), then maybe BlizzBugsSuck is indeed more appropriate...
It's a confusing issue to say the least.

[Rainrider]

Thanks for the log.

But that's exactly what's happening, using C_ArtifactUI.GetRelicInfo or C_ArtifactUI.GetEquippedArtifactRelicInfo causes ARTIFACT_UPDATE to fire.

Can't reproduce this at all and it seems odd they made it like that but I'll take TheDanW's word for it.

As the API calls in question are generally available, I think the fix is better suited for BlizzBugsSuck. LAD should actually prevent the default UI from loading ArtifactUI, so I wonder how it still manages to do that.

[Gnarfoz]

Good news, everyone!

[19:21:33] <TheDanW> i added ARTIFACT_RELIC_INFO_RECEIVED in 7.2 that'll trigger instead of ARTIFACT_UPDATE when relic info is received, the default UI won't respond to the event unless its visible

Well, good news some time in mid-2017. ;)

[Rainrider]

Lol and they are getting paid for that?

[Gnarfoz]

Tomorrow is the the day \o/

[Rainrider]

ARTIFACT_RELIC_INFO_RECEIVED is still unaccounted for in LAD as I didn't manage to spend enough time on the PTR. This should not be critical though, I hope.

[Rainrider]

I'm closing this as 7.2 was released.