bfa - error when searching icon

[mrbuds]

With branch 8.0 from github

Make new icon, uncheck "automatic icon", click choose, type "inf"

5x integer overflow attempting to store inf
[C]: ?
WeakAurasOptions\OptionsFrames\IconPicker.lua:36: in function <WeakAurasOptions\OptionsFrames\IconPicker.lua:30>
WeakAurasOptions\OptionsFrames\IconPicker.lua:74: in function <WeakAurasOptions\OptionsFrames\IconPicker.lua:74>

Locals:
(*temporary) = inf

No error on live server with master branch

[InfusOnWoW]

Blizzard did indeed change what tonumber does on 8.0. That's a strange change to make.

[InfusOnWoW]

So what is happening here is that lua can handle infinte numbers in integers, but passing them from lua to C++ is a error. Previously tostring("inf") would just return nil. Now, it returns infinite which results in a error if passed to e.g. GetSpellInfo. That's easy to fix, we just need to check if the tonumber conversion resulted in a finite number, or a infinite one.

The not so easy part, is that it isn't just the IconPicker, we convert user input to numbers in lots of different places, and this won't be the only place affected.

[mrbuds]

Wow .. I was just looking for the icon of the spell "infected" :D

[emptyrivers]

The quick fix would be to swap out tonumber for something like:

local oldtonumber = tonumber
function tonumber(s)
  local n = oldtonumber(s)
  if n and math.abs(n) < math.huge then
    return n
  end
end

[InfusOnWoW]

Yeah, but we can't do that in the global environemnt. The hard part, or actually tedious part, is to look at all the places we call tonumber and consider whether we want our own tonumber or not.

I've talked to some other addon atuhors, which told me that beta/ptrs often have additional math checking enabled, which is not enabled for live. Thus the bug might be just on the 8.0 beta. I'm waiting for our contact at Blizzard to confirm that, and if so, I'll probably ignore this bug.

[InfusOnWoW]

The word from Blizzard is a bit vage, but suggested it's likely just a beta vs live difference. Thus I'm in favor of just ignoring the problem.

[emptyrivers]

This should be fixed in the next release.