Adventure Guide Lockouts

Adventure Guide Lockouts

366k Downloads

Instance ID Missing

NeuroticPixels opened this issue · 9 comments

commented

I implemented this error function to let the user report any missing instanceID in Instances.lua, in case of an event or a new patch with new instances. But you are showing me two instances that are already in it, so I'll need more investigation from you to trace this particular issue since I can't manage to reproduce it.

First, please make sure you have the latest version of the add-on. It is currently at v1.3.3.

If you still get this error, replace Core.lua by this one into the zip: Core.zip

Then do a /reload and make another screenshot of the error.

commented

I received the same messages a few days ago for all of my current lockouts after using the hearthstone at the end of ICC. The first message was:

AdventureGuideLockouts: Onyxia's Lair instanceID is nil...

So i went ahead and used the code from Core.lua (119) with an index of 1 just like the loop in UpdateSavedInstances would call it with the following command:

/run print(tonumber(GetSavedInstanceChatLink(1):match(":(%d+):")))

The response was a value of 249 which is defined in Instances.lua (54) as the instanceID for Onyxia's Lair.


I then changed Core.lua (212) to the following code in order to debug it next time:

error(instanceName .. " instanceID is nil. Index: " .. instanceIndex .. " Chat Link: " .. GetSavedInstanceChatLink(instanceIndex):match(":(%d+):"))

Today I received the following messages again immediately after leaving ICC through the entrance.

Kissingbug472

I forgot to add tonumber() to the error message initially, but that shouldn't be a problem, since the result is only displayed in chat.

commented

Thanks for your details about this issue, it's really helpful.

Can you please do this instead so I can understand what's going on with the match? Replace all of this: https://github.com/Meivyn/AdventureGuideLockouts/blob/master/Core.lua#L111-L115

  local instanceLink = GetSavedInstanceChatLink(instanceIndex)
  local instanceMatch = tonumber(instanceLink:match(":(%d+):"))
  local instanceID = self.instances[instanceMatch]
  if not instanceID then
    error(instanceName .. " instanceID is nil.")
    AddOn.errors.instances = AddOn.errors.instances or {}
    AddOn.errors.instances[instanceName] = {
      instanceIndex = instanceIndex,
      instanceLink = instanceLink,
      instanceMatch = instanceMatch
    }
    debug(AddOn.errors.instances[instanceName])
    return
  end

Also, I'm wondering if these lockouts are properly shown in the Adventure Guide or not?

Now I know this is a confirmed issue since others can reproduce it, but it's still very weird and I feel like this will be a pain to fix. I'm not actively playing anymore, in fact, I cancelled my subscription, but I'm willing to work on that with help from the community. I can't reproduce it anyway.

This 0000000000000000 doesn't make sense at all... I need the whole instanceLink to understand the issue.

commented

Thanks to Blizzard_DebugTools not being loaded by default (I think), this time I got an actual error that seems to be very helpful. It shows the full instance chat link that explains why my debugging returned a series of zeros.

Ewe355

Now we now, that the substring match can be incorrect when the character id is messed up and the matching pattern should be changed.

I got the following pattern from another AddOn...

local lid,lname = link:match(":(%d+):%d+:%d+\124h%[(.+)%]\124h")

... and removed the second part of it:

local instanceID = instanceLink:match(":(%d+):%d+:%d+\124h")

This pattern is more consistent than the current one and should fix this particular issue.
I will not open a pull-request because you may want to change a few other things by yourself.

commented

Looks like Blizzard now uses 0's when the character ID is unknown, instead of using ::.

Thanks for your help, it should be fixed now.

commented

Well, my pattern isn't correct and I added the ìnstanceLink to the error message without the escape of |... I think it's time to sleep, haha. I thought the character ID was only alphanumeric but it looks like it's not.

Full instanceLink looks like

|cffff8000|Hinstancelock:Player-1390-09F2542A:1677:2:4|h[Cathedral of Eternal Night]|h|r

I tried to do :.+:(%d): instead, but for some reason, this matched :1677:2: and returned 2, while :(.+):%d+: returned Player-1390-09F2542A:1677. And whenever I'm adding a third digit, like in your example, or by doing :.+:(%d+):%d+: then it works, but I don't get why. It's also hard to tell if it's "really" working since I wasn't able to reproduce the issue, and for me :(%d+): was working fine.

I am also fairly new to pattern matching, in fact I documented myself yesterday about it.

commented

Full instanceLink looks like

|cffff8000|Hinstancelock:Player-1390-09F2542A:1677:2:4|h[Cathedral of Eternal Night]|h|r

And whenever I'm adding a third digit, like in your example, or by doing :.+:(%d+):%d+: then it works, but I don't get why. It's also hard to tell if it's "really" working since I wasn't able to reproduce the issue, and for me :(%d+): was working fine.

I think your updated pattern ( "%b::(%d+)" ) will work just fine, provided the first and second semicolons exist as at all times to act as opening and closing characters, but i will still try to explain why and how the pattern i suggested ( ":(%d+):%d+:%d+\124h" ) works, so that at least you did understand it fully in the end.

To explain it very simple, just look at the main pattern. If you remove the brackets ( ) from it ( ":%d+:%d+:%d+\124h" ), it will always match the part with the last 3 digits after the character ID followed by |h , because it matches three semicolons : (with digits in-between) and |h literally and doesn't use . to match all characters which can lead to unwanted results (124 is the ASCII code for the pipe symbol). For the example link above, it will return :1677:2:4|h as a result. This pattern will always be correct regardless of how the character ID or anything in front of it looks like, because the position and only occurrence of it will always be the same (with the same guarantee as your %b pattern) and string.match does always return the first match (which is why the patterns you mentioned didn't work).

Looking at the patterns you mentioned, I don't know if you know or fully understand, even if you've already used it in the old and updated pattern, that by adding brackets ( ) into a pattern, you are adding a so called capture to it. This makes sure, that only the content of this capture is returned from the match function and each capture in the pattern is returned as a separate result, which you can see in the original pattern I've used, in which the instance name is also wrapped in a capture.

However, your changes should resolve the issue just fine and if not, I'll let you know. This example shows that there are also inactive (in terms of playing) AddOn developers who are willing to tackle problems without simply disappearing, even if the problem is as small as a wrong matching pattern. 👍

commented

This should do the job. Now. Sleep.

commented

I didn't choose that pattern because I wanted to learn myself how to do it without copying on SavedInstances, something that I've already done in the past when I was clueless about patterns (01afdea) as this allowed me to do that magic match with instanceID instead of tooltipTitle, thus getting rid of these painful locales that were causing a lot of issues. I also opted for something as minimal as possible to see what's can happen if I'm not specific enough. This time, since there is only one :: before the value I want, it should be good until Blizzard adds something between, but this would also happen with any pattern as far as I know.

Thanks to my mentor of wago.io again, I now fully understand how the pattern from SavedInstances is working, including the one you suggested. It's not that complicated when you know it, but it's not really human-readable and can lead to unexpected behavior when you don't have any experience with it. I found the %b:: way by searching documentations on Google, and this is exactly what I needed.

and doesn't use . to match all characters which can lead to unwanted results

This is where I blocked. But come to think of it, what I wanted to do is basically what I've adopted %b::, not :.+:. I didn't know this was that greedy, but now I understand the logic.

and string.match does always return the first match (which is why the patterns you mentioned didn't work).

I'm not sure to understand what you're talking about here. I also mentioned a lot of patterns so I'm a bit confused.

I don't know if you know or fully understand, even if you've already used it in the old and updated pattern, that by adding brackets ( ) into a pattern, you are adding a so called capture to it.

I wasn't at that time, now yes I got this, I was just trying to see what .+ is actually doing in that string since I didn't expect this result.

Let's say I want to retrieve the color without transparency as well, I'll do like:

local instanceLink = "|cffff8000|Hinstancelock:Player-1390-09F2542A:1677:2:4|h[Cathedral of Eternal Night]|h|r"
local color, instanceID = instanceLink:match("^\124c%w%w(%w+)\124Hinstancelock%b::(%d+)")

Maybe there is a better way to do it though.

However, your changes should resolve the issue just fine and if not, I'll let you know. This example shows that there are also inactive (in terms of playing) AddOn developers who are willing to tackle problems without simply disappearing, even if the problem is as small as a wrong matching pattern. +1

It's not really about WoW, I love to get myself into coding, and even if this project is a fork, with the modifications I've done, I think I can call it "my" project, and I'm a bit attached to it.

Either way, thanks for your explications, your time and your help, I appreciate it.