Error with the Anniversary event

Question

Error with the Anniversary event

nebularg opened this issue 5 years ago · 8 comments

nebularg commented 5 years ago

17x AdventureGuideLockouts\Core.lua:202: table index is nil
AdventureGuideLockouts\Core.lua:202: in function `UpdateSavedInstances'
AdventureGuideLockouts\Core.lua:385: in function <AdventureGuideLockouts\Core.lua:377>

You're missing an entry for the new "raids". I just added a nil check since it doesn't show in the Adventure Guide.

Some info:

Dump: value=GetSavedInstanceInfo(1)
[1]="Caverns of Time - Anniversary",
[2]=462518370,
[3]=465126,
[4]=151,
[5]=true,
[6]=false,
[7]=524353537,

Dump: value=GetSavedInstanceChatLink(1)
[1]="|cffff8000|Hinstancelock:Player-60-0B479537:2235:151:7|h[Caverns of Time - Anniversary]|h|r"

Dump: value=GetSavedInstanceEncounterInfo(1, 1)
[1]="Memories of Azeroth: Burning Crusade",
[3]=true,
[4]=false
Dump: value=GetSavedInstanceEncounterInfo(1, 2)
[1]="Memories of Azeroth: Wrath of the Lich King",
[3]=true,
[4]=false
Dump: value=GetSavedInstanceEncounterInfo(1, 3)
[1]="Memories of Azeroth: Cataclysm",
[3]=true,
[4]=false

Meivyn · Answer 1 · 2019-11-07T08:29:39.000Z

Thanks for reporting this. I don't want to add a nil check 'cause these errors are very useful and I don't want to prevent these errors from triggering.
I have to think about how I can handle this specific case in a better way.

goto (continue) was implemented in Lua 5.2 and since WoW is using 5.1 I can't make it that simple, and this is not the first time I have to skip an iteration. This is a pain.

Also GetSavedInstanceInfo[2] isn't the real instance ID, it's more like a lockout ID. It's not unique and changes every week so I can't use it.

Meivyn · Answer 2 · 2019-11-07T10:29:40.000Z

You could try to change line 136 to this:

local instanceName, instanceID, instanceReset, instanceDifficulty, locked, extended, instanceIDMostSig, isRaid, maxPlayers, difficultyName, numEncounters, numCompleted, difficulty, instanceLinkID

Line 145 to this:

instanceLinkID = tonumber(GetSavedInstanceChatLink(instanceIndex):match(":(%d+)"))
instanceID = self.instances[instanceLinkID]

And line 202 (after previous edit) to this:

if instanceLinkID ~= 2235 and (locked or extended) then

Based on your GetSavedInstanceChatLink dump, it should work but this isn't a perfect workaround. I'll get into it maybe tomorrow.

nebularg · Answer 3 · 2019-11-07T11:22:42.000Z

I'm not sure the addon breaking when it hits an unknown id is useful to anyone. And like I said, I already fixed it for me.

if instanceLinkID ~= 2235 and (locked or extended) then

I'm not sure why you'd want to manually exclude things when self.instances already serves as a whitelist.

Meivyn · Answer 4 · 2019-11-08T10:27:49.000Z

Basically I just want to capture the error for the user to report without breaking the whole function, by skipping that iteration. But I don't know how I can do that.

Meivyn · Answer 5 · 2019-11-10T21:01:10.000Z

I've found this in SecureHandlers.lua:

    if not instanceID and instanceName then
      -- SoftError(message)
      -- Report an error message without stopping execution
      local function SoftError_inner(message)
        local func = geterrorhandler();
        func(message);
      end
      local function SoftError(message)
        securecall(pcall, SoftError_inner, message);
      end
      SoftError("instanceID is nil (" .. instanceName .. ")")
    elseif locked or extended then
      self.instancesLockouts[instanceID] = self.instancesLockouts[instanceID] or {}

Seems to work good, not sure if there's a more elegant way than that.

nebularg · Answer 6 · 2019-11-10T21:13:34.000Z

Sure, you can also simplify it to just geterrorhandler()("message") or use CallErrorHandler("message") from SharedXML/Util.lua. If you're going that route it would be good if you saved the error so it only fired once a session

Meivyn · Answer 7 · 2019-11-17T23:27:30.000Z

Didn't know about CallErrorHandler. I will use this, thanks.
Also need to think about how to save the error for the session.

Apart from this, I would just like to handle this in a more logic way. To illustrate:

instanceID = self.instances[tonumber(GetSavedInstanceChatLink(instanceIndex):match(":(%d+)"))]
-- report the error to the user, skip that iteration from that line and go to the next one, basically to prevent the remaining of the iteration running for nothing
if not instanceID then
  if instanceName then
    CallErrorHandler("instanceID is nil (" .. instanceName .. ")")
  end
  continue -- break the iteration, not the entire loop
end

I have found a "workaround" for my encounters table:

while GetSavedInstanceEncounterInfo(instanceIndex, encounterIndex) do
  if instanceID == 1023 then
    if self.faction == "Alliance" and encounterIndex == 1 or self.faction == "Horde" and encounterIndex == 2 then
      encounterIndex = encounterIndex + 1 -- Fixes https://github.com/Meivyn/AdventureGuideLockouts/issues/1
    end
  end
  local bossName, _, isKilled = GetSavedInstanceEncounterInfo(instanceIndex, encounterIndex)
  tinsert(encounters, {
    bossName = bossName,
    isKilled = isKilled
  })
  encounterIndex = encounterIndex + 1
end

But this isn't applicable in that case.

Meivyn · Answer 8 · 2019-12-18T08:47:40.000Z

After some help from people on WoW AddOns; Discord, I'll split my instances/world bosses logic into separate functions, and I'll use this to report the error so it'll be fired only once a session:

local function error(...)
  AddOn.errors = AddOn.errors or {}
  if not AddOn.errors[...] then
    print("|cffff0000" .. AddOnName .. ":|r", ...)
    AddOn.errors[...] = true
  end
end

function AddOn:GetInstanceLockout(instanceIndex)
  local instanceID = self.instances[tonumber(GetSavedInstanceChatLink(instanceIndex):match(":(%d+):"))]
  if not instanceID then
    error(instanceName .. " instanceID is nil. Please report this at https://github.com/Meivyn/AdventureGuideLockouts/issues")
    return nil
  end
  return {} -- table populated by lockout info
end

function AddOn:UpdateSavedInstances()
  self.instancesLockouts = {}

  local savedInstances = GetNumSavedInstances()
  local lockout

  for instanceIndex = 1, savedInstances + #self.worldBosses do
    if instanceIndex <= savedInstances then
     lockout = self:GetInstanceLockout(instanceIndex)
    else
     lockout = self:GetWorldBossLockout(instanceIndex - savedInstances)
    end

   if lockout and (lockout.locked or lockout.extended) then
     self.instancesLockouts[lockout.instanceID] = self.instancesLockouts[lockout.instanceID] or {}
     tinsert(self.instancesLockouts[lockout.instanceID], lockout)
    end
  end
end

I gave up on CallErrorHandler as it's not showing anything using default UI without BugSack.

I still have to think about how to handle the specific "non-EJ instance" case, by doing nothing instead of reporting an error to the user. I think I'll have to do some sort of blacklist to achieve that.

Share to