Talent Sequence

Talent Sequence

35.1k Downloads

3.1 - doesn't work on SoD links with runes picked

Road-block opened this issue ยท 10 comments

commented

Example import string for Hunter: https://www.wowhead.com/classic/talent-calc/hunter/5-05051_156q976tka6qk/1BD0A1e
Addon shows static popup of "invalid link"

The link is apparently not invalid, the parsing is wrong.

As a FYI the full sequences for the url generation are:
abcdefghjkmnpqrstvwzxyilou468-~! left to right, top to bottom
ABCDEFGHJKMNPQRSTVWZXYILOU579_^. as above but shorthand for when a specific talent is fully filled

A 5 points talent would be denoted as - for example - aaaa until 4/5 then a single A when 5/5 (a very rudimentary url shortening trick they use)

commented

Ah I appreciate the info! I think I understand what's happening there though regex hurts my brain. I'm not sure why the "%2" instead of "" though, but I'm also in Python brain at the moment haha

I'm trying to rewire back to how Lua works and dredging up the scant knowledge I haphazardly researched when making adjustments to the TalentSequence 2 fork, but it all just makes my brain hurt ๐Ÿ˜ตโ€๐Ÿ’ซ

commented

it's not strictly regex, but that particular one Road-block used is capturing two groups: the first is everything between the underscore and the slash, and the second is the slash itself. %2 is the position of the capture group to use as the replacement, which is just the /. I think python uses \2 but other engines may use something like $2.

You could probably do something like input = input:gsub("(_[%l%d]+)","") instead. I would probably have gone with

 local _, _, class, rank, _, sequence = string.find(talentsString, "/talent%-calc/(%l+)/([0-9%-]+)(_[%l%d]+)?/(.+)")

but it really does not matter in the end with these one-shot code paths.

commented

There's a chance the parsing is not wrong but the addition of rune picks to the url is throwing a spanner in the works.

commented

So I think there's 3 ways to handle this.

  1. Add a 2nd caveat for the player that they have to expand talent order and leave runes empty before they get an import string.
  2. Detect the presence of a runes fragment in the url and strip it before trying to parse the talent string. In the example above this is the _156q976tka6qk bit ie the start of the rune string is delimited by _ and ends at /
  3. Parse it out and save runes too? (I don't see much use in importing runes for a talent sequence so [2] would get my vote
commented

I've got the fix already and will implement soon. There were two things at play here.

  1. Fusionpit went and simplified some of the code (I readily admit I'm new and am not efficient with my code because Idk really how to be). In the process, he added some Regex for parsing the link into its different variables (class, ranks, and sequence), but he took the ranks portion (5-05051_156q976tka6qk) and limited it to numbers only. This meant that since the runes are added with the ranks using an _ rather than their own url-space at the end of the url, the parse would return nil for all values and present invalid link.

  2. The way the ranks are parsed/used, the _156q976tka6qk never gets touched. It technically gets added on to the last talent tree's talent rank selections, but the indexing never touches it since it's based off the indices grabbed by abcdefghjkmnpqrstvw. (The ones you listed are for WotLK -- Classic only has a max of 19 talents for any of its trees (IE: Combat Rogue))

Lastly, just to be clear, I don't believe there are any plans at implementing a use for saving/displaying which runes to take, it's just the addition of runes shouldn't (after the update!) affect the Talents aspect. :)

commented

I know they don't need all of them but I pulled the hashes directly from their javascript and regardless if they make use of the whole length, that's the reference.

Likewise for runes they go chest[1], hands[2], legs[3] and use 0123456789abcdefghjkmnpqrstvwxyz as a hash.
Implementation details don't matter as like you said there's no reason to get rune info for TalentSequence ๐Ÿ˜„

The easiest solution is to modify the regex that validates the url to include an optional rune pattern and strip it (ie remove the (_156q976tka6qk)/ part first if it exists then split it as it does currently.

commented

I found just letting the regex that validates the /rank/ portion take any ol' input to be easier... but again I'm not very efficient ๐Ÿ˜…

I didn't(/don't) know how to strip the excess and don't think it's hurting anything lol, but the update should be out and looks like passed along to Curse etc. Should be set :)

commented

Ok as an example, simply running your url through this first, will solve the problem.

Let's assume the inputstring is the example given https://www.wowhead.com/classic/talent-calc/hunter/5-05051_156q976tka6qk/1BD0A1e

Doing this before splitting it to parts will strip the runes encoding (or do nothing if the runes pattern is not there)

local input = "https://www.wowhead.com/classic/talent-calc/hunter/5-05051_156q976tka6qk/1BD0A1e"
input = input:gsub("(_[%l%d]+)(/)","%2")

Now your input is sanitized.

commented

I haven't seen the new implementation but taking the previous code, you had this.

    local _, _, class, rank, sequence = string.find(talentsString, "/talent%-calc/(%l+)/([0-9%-]+)/(.+)")
    if (class == nil or rank == nil or sequence == nil) then
        StaticPopup_Show(INVALID_LINK)
        return
    end

All it needed to keep working without further changes was to make it like this

    talentsString = talentsString:gsub("(_[%l%d]+)(/)","%2") -- strip the rune pattern IF it is found, nothing happens if not there
    local _, _, class, rank, sequence = string.find(talentsString, "/talent%-calc/(%l+)/([0-9%-]+)/(.+)")
    if (class == nil or rank == nil or sequence == nil) then
        StaticPopup_Show(INVALID_LINK)
        return
    end
commented

Ahh thanks for the further clarification :)

In any case, it should be working now, will just have to keep an eye on it in case Wowhead does some funky things with that portion of the url in the future. Thanks guys!