LibResInfo - resurrection info without comms

[Phanx]

Originally Posted by myrroddin Just so I understand, endTime from UnitHasIncomingRes is when that person's Accept timer runs out (60 seconds), and not the cast time of the caster's resurrection spell?Whereas UnitIsCastingRes' endTime is the cast time of the spell (10 or less seconds)? If that is the case, then I wonder, does LRI add the cast time to 60 in order to get the true time out?

No. The values (including endTime) provided by UnitHasIncomingRes refer to whichever resurrection will be available to the specified unit first. If they already have a pending resurrection, then endTime gives you the time that resurrection will expire. If they do not already have a pending resurrection, but at least one resurrection spell is being cast on them (including Mass Res), then endTime gives you the time that the first of those spells will finish casting. You don't need to do any math.

In your example, while Jixx is still casting Resurrection, endTime refers to the time his cast bar will hit full and the spell will actually be cast. After Jixx has cast Resurrection and Phanx has an accept/cancel dialog box on her screen, endTime refers to the time that dialog box will disappear and Phanx will no longer be able to accept the resurrection.

Originally Posted by myrroddin ... perhaps renaming UnitHasIncomingRes' endTime to expiryTime might be in order to lessen confusion.

You can name the variable whatever you want in your code. The documentation will continue to call it endTime because it can refer to either the end time of a spell cast, or the expiry time of a resurrection offer. That's why the first value returned by UnitHasIncomingRes is a string telling you whether the other values are related to a spell cast or a resurrection offer. At the most basic level, the idea is that you can perform a simple boolean check on the return from UnitHasIncomingRes to find out if you need to cast a res on the unit:

if LibResInfo:UnitHasIncomingRes(unit) then
    -- don't bother, someone else's res will get to them first
else
    -- they still need a res
end

Originally Posted by myrroddin The other way to look at it, just to add confusion, is that UnitHasIncomingRes's endTime is not 60 + cast time seconds, and is just end time of the caster's resurrection.

Why would it be 60 + cast time seconds? If the res spell is still being cast, then endTime is the end time of the spell cast.

Here is the basic process:

function lib:UnitHasIncomingRes(unit)
    if the unit can click "accept" to take a res that was already cast, then:
        return "PENDING", time the res will stop being available

    elseif *one* res spell is being cast on the unit, then
        return "CASTING", time the spell will finish casting, info about the unit casting the spell

    elseif *more than one* res spell is being cast on the unit, then
        determine which spell will finish casting first, and
        return "CASTING", time *that* spell will finish casting, info about the unit casting *that* spell

    end
end

Originally Posted by myrroddin Personally, I would have endTime in both UnitHasIncomingRes and UnitIsCastingRes = cast time, and make a note on the API page that authors should add 60 to get the Accept expiration time. My reasoning is that authors tend to want the cast time not the expiry time, which I pointed out is simple addition anyway.

Again, you are either not reading the documentation, or not reading the documentation. If the spell has already finished casting, then there is no cast time to provide, because there is no spell being cast.

Also, UnitHasIncomingRes and UnitIsCastingRes are different functions that provide information about different things.

UnitHasIncomingRes tells you about the res the specified unit will be able to take first. If they already have a res waiting to be accepted, then it tells you about that res. If they don't have a res waiting to be accepted, then it tells you about the first res spell that will finish casting on them.

UnitIsCastingRes tells you about the res spell being cast by the specified unit, even if the target of that spell already has a res ready to accept and 20 other people are already casting res spells on the same target.

Read the documenation and stop over-thinking it. It works exactly the way the documentation says it works. You don't need to blindly print the callback args and try to figure out what they mean on your own, because they mean exactly what the documentation says they mean.

[myrroddin]

Alright, got it. I had a mental chuckle, but I got it. Now to wait until you either change the APIs and callbacks, or update the documentation to match the code. *Rubs hands together gleefully* Can't wait! ... Well, I can, and will, but you know what I meant.

[myrroddin]

Would it break things in my local copy if I rearranged the callback arguments to your proposed standard arguments? Just wondering about the debug or APIs.

[Phanx]

I went ahead and committed the changes. I'd still like to do some testing; I'll be online later tonight from 10 or 11 PM PST onward. Updating the documentation pages now.

[myrroddin]

I would say all the testing was a success. You can safely tag a release for tomorrow's 5.2 patch, if you haven't already done so, or had such plans.

Once again, thank you for all the help, testing, recoding, and copious deaths!

[Phanx]

Everything should be stable now, and the documentation matches the actual functionality, so feel free to tag releases and whatnot.

[Gronzig]

I am getting an error in r43, line 307, from comparing a number to nil. I believe this line should be changed from

if resCasting[target] > 1 then

to

if resCasting[target] and resCasting[target] > 1 then

Also, it looks like line 319 should be changed from

resCasting[guid] > 0 then

to

if resCasting[guid] and resCasting[guid] > 1 then

[Phanx]

That's really just a band-aid, but I'm probably going to go with it, because I just don't have time to go through the extensive in-raid testing required to figure out what the actual problem is. There's still going to be the same outdated, mismatching data in the cache. I'm pretty sure it's related to the current implementation of Mass Res support, and I'll probably just end up removing Mass Res support from the existing callbacks, and just fire a single callback for each Mass Res cast, instead of trying to keep track of which players are affected by every Mass Res.

[myrroddin]

I am getting the following whenever people cast Mass Resurrection. I know you wanted to possibly rewrite the MR callback, and hopefully this helps.

26x Libs\LibResInfo-1.0\LibResInfo-1.0-11.lua:222: attempt to compare nil with number
Libs\LibResInfo-1.0\LibResInfo-1.0-11.lua:222: in function "UnitIsCastingRes"
SmartRes2\SmartRes2-Release 2.5.4.lua:477: in function "?"
Ace3-Release-r1086\CallbackHandler-1.0\CallbackHandler-1.0-6.lua:147: in function <Ace3\CallbackHandler-1.0\CallbackHandler-1.0.lua:147>
<string>:"safecall Dispatcher[6]":4: in function <string>:"safecall Dispatcher[6]":4
<in C code>
<string>:"safecall Dispatcher[6]":13: in function "?"
Ace3-Release-r1086\CallbackHandler-1.0\CallbackHandler-1.0-6.lua:92: in function "Fire"
Libs\LibResInfo-1.0\LibResInfo-1.0-11.lua:487: in function "?"
Libs\LibResInfo-1.0\LibResInfo-1.0-11.lua:105: in function <Libs\LibResInfo-1.0\LibResInfo-1.0.lua:104>
 
Locals:
nil

[Phanx]

It's the same issue with Mass Res screwing up the cache because it doesn't actually fire events. I've been slowly writing a new version from scratch that handles caching better, and doesn't attempt to fire individual callbacks for all units that might be affected by Mass Res, but it's not done yet.

Anyway, I've added another bandaid for that error, so if you update your working copy you shouldn't get the error anymore.

[myrroddin]

Phanx, you have mentioned you are writing a new version, or at least modifying the existing code. If I can make a suggestion to the API, since I think the code already calculates this: expose both the current casterID and the first casterID for UnitHasIncomingRes.

local status, endTime, currentCasterID, currentCasterGUID, firstCasterID, firstCasterGUID = LibResInfo:UnitHasIncomingRes(unit)
--[[
    status is either "CASTING" or "PENDING"
    endTime is the time the current caster's spell ends
    currentCasterID and currentCasterGUID are the unitIDs or GUIDs for whomever is the current res caster on "unit"
    firstCasterID and firstCasterGUID are the same as above, but for whomever's cast lands first
    unit is "player", "target", "raid10", etc
]]--

This would make comparing isFirst's targetUnit to find out who is the first casterUnit on targetUnit much simpler.

The reason I ask is because I am running into the situation where SmartRes2's collision notification system is giving erroneous results if more than one caster is casting on the same unit, and even more erroneous results when there are more than one collision caster with more than one collision targets. Mass Resurrection's collision doesn't seem to be adversely affected.

My work in progress solution is building a table

local doingRessing = {}
function MyAddOn:LibResInfo_ResCastStarted(targetUnit, targetGUID, casterUnit, casterGUID, endTime)
    local _, _, _, isFirst = LibResInfo:UnitIsCastingRes(casterUnit)
    if not doingRessing[casterUnit] then
        if targetUnit then -- class spell. use isFirst for Mass Res since there is no targetUnit
            doingRessing[casterUnit] = {
                targetID = targetUnit,
                isFirst = isFirst
            }
        end
    end
    -- if not isFirst and targetUnit then iterate through table to find out who IS first
    -- send chat message to casterUnit with firstCasterID and targetUnit
end
 
function MyAddOn:LibResInfo_ResCastFinished(targetUnit, targetGUID, casterUnit, casterGUID)
    if doingRessing[casterUnit] then
        doingRessing[casterUnit] = nil
    end
end

I am going on vacation for the weekend, and will not be able to apply this code and test until next week at the earliest. It is just a thought that if LRI calculates who is the first caster (target or not) in order to provide true/false for isFirst, then exposing both sides would be beneficial.

[myrroddin]

The other option is to provide firstCasterID and firstCasterGUID at the end of UnitIsCastingRes rather than UnitHasIncomingRes. Or do both, whichever is easier given current code.

[myrroddin]

Thought I would explain better. The issue is because of strings, which get overwritten, and thus cannot be compared accurately, at least in the current API.

-- scenario #1
-- Bob is casting on Sally
-- Ted is casting on Sally
 
local endTime, targetUnit, targetGUID, isFirst = LibResInfo:UnitIsCastingRes(casterUnit)
local targetName = UnitName(targetUnit)
local casterName = UnitName(casterUnit)
 
-- Bob is first
targetName == "Sally"
casterName == "Bob"
isFirst == true
 
-- now Ted
targetName == "Sally"
casterName == "Ted"
isFirst = false/nil
 
print("Hey %t!, %s is going to finish casting on %t first!"):format(casterName, casterName, targetName)
-- what about Bob?
 
-- scenario # 2
-- add more casters and more targets
 
-- Bob is first
-- now Ted
-- Joe is first on Bill
-- Mike is first on Sam
-- Rachel is not first on Bill

if you parse through the existing API, the strings get overwritten fast, and no longer are accurate by the time you are doing prints

[Phanx]

I don't plan to add this to the API, but you should be able keep track of it yourself with minimum effort by either:

(1) storing the endTime value in a variable, and comparing it against incoming callbacks -- if the endTime supplied by the callback is a larger value (later time) then the callback res is not the first on the target, and you shouldn't overwrite your variable.

(2) calling LRI:UnitHasIncomingRes(targetGUID) when you get a callback -- if the caster info from the API doesn't match the caster info from the callback, then the callback res is not the first on the target.

[myrroddin]

Phanx, I am wondering about the update status to the Mass Resurrection code. I still see the occasional error regarding line 211. Last time we talked, you suggested that you were going to rewrite LibResInfo to have separate callbacks for Mass Resurrection. Is there updated news?

Here is the ticket someone posted on SmartRes2's ticket tracker with the LRI error. Unless it can be fixed or bypassed in SR2... http://www.wowace.com/addons/smartre...pare-nil-with/

[myrroddin]

I have seen some odd behaviour ever since patch 5.4.1 hit. The debug prints are showing up, and I did not enable them. I tried the slash commands "lri debug 0" and "lri debug"; it says that is invalid output. I figure this is something simple, possibly something I did that I don't remember doing, but I can't spot the fix.

Also, I would like to do some more testing, as I keep getting nil errors periodically. I think it has something to do with Mass Resurrection, but occasionally see it for normal class spells. Granted, there are some times when the Combat Log, and therefore LibResInfo, cannot get information, but it would be nice to track down what and where, if only to "catch" it. BugSack gets a bit annoying with its "Uh Oh"!

[Phanx]

Debugging is turned on automatically when you're running LRI as a standalone library. You should be embedding it in your addon instead. Also, the command to set the debugging level is just "/lri N", not "/lri debug N"; use "/lri 0" to turn it off.

[Phanx]

I have a new version ready for more extensive testing; check out revision 60 or later from SVN. There are a number of "under the hood" changes that should help avoid data desynchronization, and the following significant changes:

(1) There is now support for pre-cast Soulstones.

lib:UnitHasIncomingRes(unit) will now return "SELFRES" as its first value if the unit has a Soulstone available, and the LibResInfo_ResPending callback will now pass a fourth argument (boolean true) if the pending resurrection is a Soulstone.

(2) Mass Resurrection callbacks are now separate from normal res callbacks.

The new callbacks are very similar to the existing callbacks:

• LibResInfo_MassResStarted, casterUnit, casterGUID
• LibResInfo_MassResCancelled, casterUnit, casterGUID
• LibResInfo_MassResFinished, casterUnit, casterGUID

lib:UnitHasIncomingRes(unit) will now return "MASSRES" instead of "CASTING" as its first value if the first res that will land on the unit is from Mass Res.

The new Mass Res callbacks/API have not been tested at all since I'm limited to PTR testing (my live account is not currently active) and don't have a PTR guild. If you have access to a character with Mass Res on the PTR and can help me test, please PM me your BattleTag and when you're available so we can set up a time. I'm generally able to get on the PTR between 10 PM and 2 AM Pacific time (GMT-8).

Developers, if you want to install r60 or later as a standalone addon for testing, please let me know if you run across any odd behavior. Debugging messages are automatically enabled and sent to ChatFrame3 when the lib is installed standalone. Use "/lri 3" to change the debugging level (0=off, 5=spam) or "/lri ChatFrame1" to change the output frame.

Edit:
Also, the last version has been tagged at r58. If you need to make a release with your addon over on CurseForge, change your .pkgmeta file to point to /tags/r58/LibResInfo-1.0 instead of /trunk/LibResInfo-1.0/LibResInfo-1.0 (yes, that is one fewer "LibResInfo-1.0" folders, not a typo).

[myrroddin]

Oi! Looks like my plate just got bigger, along with better. I'm currently debugging chat prints and why they aren't working, but I will definitely get on this.

I will try to make or transfer a character to the PTR. Trouble is that my work week is about to start, and I won't be available at the same times as you. I'll see what I can do, however.



*Edit: wouldn't the pkgmeta's tag: latest work rather than specifying a directory path? Or, because the repo is here, will that not work?

[Phanx]

Originally Posted by myrroddin *Edit: wouldn't the pkgmeta's tag: latest work rather than specifying a directory path? Or, because the repo is here, will that not work?

It should work, or tag: r58, though you will have to change the path either way, since the tags path does not include the top-level LibResInfo-1.0 folder... that was an oversight on my part, but I'm not going to bother changing it since if you're already changing to point to tag: latest it's not really more work to change the path too.

(Really, I wish WoWI would just fix the regex in their packager so I don't need that extra folder in /trunk )