[Bug] An error that seems to occur only during instance (TellMeWhen#1876)

[ghost]

Hi,

Sorry for the following issue history. But I had created an issue for the wrong addon. Thus, the TellMeWhen addon author has redirected me to yours.

Best regards

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

NeoAnotherOne commented yesterday •

TellMeWhen v9.0.5.

It seems to occur only during instance: dungeon or raid, rarely, and not in open world.

Sorry, but, the only log that I have is the following, I hope that it would be sufficient.

Message: invalid key to 'next' Time: Sat Mar 20 13:57:36 2021 Count: 1 Stack: invalid key to 'next' [string "=[C]"]: in function `(for generator)' [string "@interface\AddOns\TellMeWhen\Lib\LibDogTag-Unit-3.0\LibDogTag-Unit-3.0.lua"]:49: in function <...MeWhen\Lib\LibDogTag-Unit-3.0\LibDogTag-Unit-3.0.lua:46> [string "@interface\AddOns\TellMeWhen\Lib\LibDogTag-Unit-3.0\LibDogTag-Unit-3.0.lua"]:56: in function <...MeWhen\Lib\LibDogTag-Unit-3.0\LibDogTag-Unit-3.0.lua:55>

Locals: (*temporary) =

{ } (*temporary) = "party1target"

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

ascott18 commented yesterday This is a bug in https://github.com/parnic/LibDogTag-Unit-3.0. It has been reported at least 8 years ago - https://www.curseforge.com/wow/addons/libdogtag-unit-3-0/issues/32

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

NeoAnotherOne commented 23 hours ago • I am trying this (Take a table copy before processing it):

local function fireEventForDependents(event, unit, ...) local wackyDependents = normalUnitsWackyDependents[unit] if wackyDependents then local idx01 = 0 local wackyDependentsCopy = {} for unit in pairs(wackyDependents) do idx01 = idx01 + 1 wackyDependentsCopy[idx01] = unit end for idx02 = 1, #wackyDependentsCopy do local unit = wackyDependentsCopy[idx02] DogTag:FireEvent(event, unit, ...) end end end

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

ascott18 commented 13 hours ago Yeah, that would fix it for sure but isn't very performant. Some of those tables can get up to be pretty large - 60+ entries in a 40 man raid from what I saw in testing last night.

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

NeoAnotherOne commented 11 hours ago This appears to actually correct the problem. But I totally agree with you that it is not efficient. However, you said earlier that this bug is at least 8 years old. So, while waiting for a more effective correction, this kind of patch would temporarily fix the problem. It's a little better, if it's a little slower, but bug-free. Finally, this is my personal opinion :) Anyway, thank you very much for your addon.

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

ascott18 commented 1 hour ago • You're posting this on the wrong addon. Again, this is a bug in LibDogTag-Unit-3.0, not TellMeWhen.

@ascott18 ascott18 closed this 1 hour ago

[parnic]

Thanks for the report. I've never seen this in the 8 years it's existed, but since you're seeing it, perhaps you could try out some alternative fixes?

Does this work?

local function fireEventForDependents(event, unit, ...)
    local wackyDependents = normalUnitsWackyDependents[unit]
    if wackyDependents then
        unit = next(wackyDependents, nil)
        while unit ~= nil do
            local nextUnit = next(wackyDependents, unit)
            DogTag:FireEvent(event, unit, ...)
            unit = nextUnit
        end
    end
end

[ghost]

Hi, and thanks a lot for your response.

I have 'tested' your code in a few instances and it seems to work. No more bug occurred during my tests. With the original code, I had at least one bug "Message: invalid key to 'next'" per instance.

Best regards

[Backupiseasy]

I have also seen this error only once since adding LDT to Threat Plates. But there have been reports of users that got this error. I don't know why it's happening as - as far as I understand it - this error mostly occurs when elements from wackyDependents are removed while iterating over it. But that does not seem to happen in LDT ...

[parnic]

@Backupiseasy are you still seeing it with LDT-Unit 90000.5+?

[Backupiseasy]

Difficult to say as with the current LDT-Unit, I cannot enable LDT because of this error https://www.curseforge.com/wow/addons/libdogtag-unit-3-0/issues/36.

I also got the error only once, so it does not occur frequently for me.

[parnic]

Bah, I apparently don't get emails for issues created on LDT/LDT-Unit on Curse.

So having both LDT 90000.4+ and LDT-Unit 90000.5+ installed still generates that error?

[Backupiseasy]

I have the same e-mail issue for TP (not for TP Classic though) ...

Yes, I installed IceHud, LDT and LDT-Unit today and only enabled these three addons/libraries and the error still was there.

[ascott18]

Can't reproduce with only the current packaged version of TMW installed:

[parnic]

@ascott18 I see it if I install LDT and LDT-Unit as their own addons on retail. One "easy" fix is to update LDT's Cleanup.lua to use @project-date-integer@ instead of @file-date-integer@. But I'm also not seeing DogTag_MINOR_VERSION be anything other than nil, so that's also strange.

[ascott18]

Oh yeah I see it now doing that.

[ascott18]

This is the result of an unfortunate omission in LDT's Cleanup.lua. LDT-Unit has this (equivalent) code, but LDT doesn't:

MINOR_VERSION = _G.DogTag_MINOR_VERSION
_G.DogTag_MINOR_VERSION = nil

[ascott18]

Fixed in https://github.com/parnic/LibDogTag-3.0/commit/eb2cb83df8db00ab91d3db8cd276158470988b61

[ascott18]

Also, we've come full circle - the reason this isn't broken as an embed is because of my "side note" in https://github.com/parnic/LibDogTag-3.0/issues/2:

As a side note, BigWigs packager appears to be setting this timestamp to the time of the last commit to the entire repo, not the last commit to the specific file being processed. However, we don't really care about this.

This side note only pertains to the way the replacement is done when the lib is pulled in as a pkgmeta external.

This is what Cleanup.lua looks like as an embed in TMW:

This is what it looks like as a standalone addon:

So,

However, we don't really care about this.

Famous last words...

[ascott18]

@parnic project-date-integer may be a better solution long term anyway, though - I can't think of a reason to not use it.

[Backupiseasy]

Exactly that, yes.

[ghost]

Hi,

I am a newbie in lua, thus sorry if the following is wrong.

I have done a little test outside of the library to check the fix snippet.

It seems that the 'n = next(t, e)' still produces an error (if the key does not exists anymore in the table). The 'n = t[e] and next(t, e)' seems more safe.

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

local t = {}

t["Hello"] = "World"
t["Goodbye"] = "Universe"

local e = next(t, nil)
while (e ~= nil) do
  local n = next(t, e)
  print(e)
  t = {}
  e = n
end

==>

Hello invalid key to 'next'

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

local t = {}

t["Hello"] = "World"
t["Goodbye"] = "Universe"

local e = next(t, nil)
while (e ~= nil) do
  local n = t[e] and next(t, e)
  print(e)
  t = {}
  e = n
end

==>

Hello Goodbye

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

[parnic]

Yes - I suspected this was going to come up, but since testing of the in-game fix didn't produce an error, I was assuming that, while technically still "not perfect", it was in practice good enough. Copying the entire table is not something we want to do as that can/will cause slow-downs as well as generate garbage that will need to be collected later.

@NeoAnotherOne Are you still seeing errors with the latest version or is this academic?

[ghost]

Hi,

I don't have done new instances since my few tests, one week ago.

Thus, sorry, it was only academic. I will stop my lua learning :) and I will redo some instances.

Thanks for your help