Event changes in Legion

[Resike]

Originally Posted by Seerah While more "compact", it is slower. Which especially matters in combat.

But easier to read in forums, then six billion underscores.

[Fizzlemizz]

Originally Posted by Resike But easier to read in forums, then six billion underscores.

local x = { strsplit("-", GUID) }
return tonumber(x[5])

or am I missing something?

[Gello]

That would create an extreme amount of garbage due to the temporary tables. And it makes a compelling case for them to pass the numerical spellID in that last parameter if it's truly random/unused. String manipulation can create excessive garbage if not done with a little care.

In a test of 1000 random GUIDs run through 100 times (for 100k parses):

return tonumber((select(5, strsplit("-", GUID))))

Took 118ms and created 80k of garbage.

local x = { strsplit("-", GUID) }
return tonumber(x[5])

Took 247ms and created 8335k of garbage.

local _,_,_,_,x = strsplit("-",GUID)
return tonumber(x)

Took 110ms and created 80k of garbage.

local x = GUID:match("(%d+)%-(%x+)$")
return tonumber(x)

Took 380ms and created 80k of garbage.

local x = GUID:match("^%d+%-%d+%-%d+%-%d+%-(%d+)")
return tonumber(x)

Took 127ms and created 38k of garbage.

local x = GUID:match("%d+%-%d+%-%d+%-%d+%-(%d+)%-%x+")
return tonumber(x)

Took 147ms and created 38k of garbage.

Unfortunately I can't post the testing code since these forums are throwing a CloudFlare blocked page when attempting to do so.

[lightspark]

TBH, I'm a bit surprised by select performance O_o

Anyway, nice test, thank you

[p3lim]

Originally Posted by lightspark TBH, I'm a bit surprised by select performance O_o Anyway, nice test, thank you

Me too, it used to be a lot worse than that.

The results from string.match is not that surprising however, being specific with your pattern will reduce the amount of searching and validation needed, thus increasing "performance".

Originally Posted by Gello Unfortunately I can't post the testing code since these forums are throwing a CloudFlare blocked page when attempting to do so.

Use http://www.curseforge.com/paste or https://gist.github.com/ (or any other paste website that is not filled with ads tbh).

[Resike]

I don't think select was that bad even back then. It's just a common thing to hate it in this forums. My guess would be it's started by Phanx.

[Fizzlemizz]

Thank you for running the tests Gello, nice to see the results side by side.

[endx7]

Originally Posted by Resike I don't think select was that bad even back then. It's just a common thing to hate it in this forums. My guess would be it's started by Phanx.

I don't know how much things have or haven't changed in the cost of select, but I believe most of the overhead is in performing the function call (with select itself just doing some clever stack manipulation). Typically select has to compete with simple return value unpacking, which incurs no function call overhead at all. Here we can't avoid the string manipulation, unfortunately.

Edit: Hm, I guess it is only 8ms per 1000 select calls though.

[galvin]

Didn't even think to do this instead.
Blizzard uses UnitCastingInfo() in their code with cast bars. Unless I'm missing something.

So just do SpellID = select(10, UnitCastingInfo(Unit)) or
_, _, _, _, _, _, _, _, _, SpellID = UnitCastingInfo(Unit)

This should produce zero garbage.

I tested it and gave back the spell id at the start and end of a cast.

This works on a UNIT_SPELLCAST_SUCCEEDED event. Not sure if lag would cause UnitCastingInfo() to return no info.

EDIT: Not reilable on a UNIT_SPELLCAST_SUCCEEDED sometimes UnitCastingInfo() for spellID returns nil.

Could do this not sure if it uses less memory or not

-- Set events for UNIT_SPELLCAST_START and SUCCEEDED here.

function SpellCasting(Event, Unit, Name, Rank, CastID)
  if Event == 'UNIT_SPELLCAST_START' then
    local _, _, _, _, _, _, _, _, _, SpellID = UnitCastingInfo(Unit)
    LastSpell[CastID] = SpellID
  else
    SpellID = LastSpell[CastID]
    LastSpell[CastID] = nil
  end

  print('>>', Event, SpellID)
end

[Resike]

Originally Posted by galvin Didn't even think to do this instead. Blizzard uses UnitCastingInfo() in their code with cast bars. Unless I'm missing something. So just do SpellID = select(10, UnitCastingInfo(Unit)) or _, _, _, _, _, _, _, _, _, SpellID = UnitCastingInfo(Unit) This should produce zero garbage. I tested it and gave back the spell id at the start and end of a cast. This works on a UNIT_SPELLCAST_SUCCEEDED event. Not sure if lag would cause UnitCastingInfo() to return no info. EDIT: Not reilable on a UNIT_SPELLCAST_SUCCEEDED sometimes UnitCastingInfo() for spellID returns nil. Could do this not sure if it uses less memory or not -- Set events for UNIT_SPELLCAST_START and SUCCEEDED here. function SpellCasting(Event, Unit, Name, Rank, CastID) if Event == 'UNIT_SPELLCAST_START' then local _, _, _, _, _, _, _, _, _, SpellID = UnitCastingInfo(Unit) LastSpell[CastID] = SpellID else SpellID = LastSpell[CastID] end print('>>', Event, SpellID) end

There are a lot of other cast related event types, when castinginfo return nil, or just simply not reliable. Thats when you need to get access fro the spellid and the lineid from the event itself.

The main issue i see is that every player runs with a long macros filled with multiple spells and spam them while in combat:

/cast Avatar
/cast Bloodbath
/cast Bladestorm

Only one will trigger a spell every other line will generate a "UNIT_SPELLCAST_FAILED" event, any you need to listen this event for your castbar, and thats where the parsing the string six billion times uncessessary will be so bad. When a 40 man raid (and their pets) spamming their 255 long macros filled with their spellbook.

And the "UNIT_SPELLCAST_FAILED" is just one example from the many cast related spells.

[siweia]

UNIT_SPELLCAST_SENT and UNIT_SPELLCAST_START currently shows in same time, even you do have lag in the BETA.
I used the time of CURRENT_SPELL_CAST_CHANGED instead of UNIT_SPELLCAST_SENT to retrieve lag.

[Nevcairiel]

As a follow up to UNIT_SPELLCAST_* events, the 5th argument does represent the actual spellID now. It was always supposed to, but was apparently bugged initially. So you don't need to parse the GUID just for that.

[galvin]

Thank god!, woot!

[Nevcairiel]

Originally Posted by galvin Thank god!, woot!

It really isn't that big of a deal.