3.1 - doesn't work on SoD links with runes picked
Road-block opened this issue ยท 10 comments
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)
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 ๐ตโ๐ซ
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.
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.
So I think there's 3 ways to handle this.
- Add a 2nd caveat for the player that they have to expand talent order and leave runes empty before they get an import string.
- 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/
- 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
I've got the fix already and will implement soon. There were two things at play here.
-
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
, andsequence
), but he took theranks
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. -
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 byabcdefghjkmnpqrstvw
. (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. :)
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.
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 :)
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.
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