I went back to look at the code for possible Cataclysm changes, and noticed a few things that needed changing:
Some of the spellIDs were using high rank numbers from live. Fixed.
UNIT_SPELLCAST_FAILED() and UNIT_SPELLCAST_INTERRUPTED() were not registered. Pointed both to UNIT_SPELLCAST_STOP() This is important because ResComm_Ressed() was being fired on failed and interrupted casts.
This ticket, 1.5 years old, was never resolved, even though workarounds were suggested. I am concerned about my fixes (lines 161, 185, 218, 219, 270-278 ) and whether I got it correct.
I read in passing something about this, arg1, arg2, etc being changed or removed in Cataclysm, and I wonder if lines 46-49 were going to be affected
Not going to do a commit until the changes in the paste pass muster.
In the meantime, I sent a PM to DathRarhek earlier today to see if he is still around to maintain this library. If not, I may take it over.
This ticket, 1.5 years old, was never resolved, even though workarounds were suggested. I am concerned about my fixes (lines 161, 185, 218, 219, 270-278 ) and whether I got it correct.
Your change on line 161 won't actually do anything, since you didn't add any captures to the pattern passed to string.gmatch, so reason will always be nil.
Your change on line 185 will similarly do nothing, since reason will always be nil.
Your changes on lines 270-278 will only work in English locales; the string value returned by the HasSoulstone API are localized. The "Reincarnate" value (shown to a dead shaman with Reincarnation not on cooldown) may be taken from the global string USE_SOULSTONE, but since there are no global strings matching "Use Soulstone" or "Twisting Nether", that may not hold true for other locales. You would need to find people who play in each locale and have access to a shaman, a warlock, and a character with the Darkmoon Card: Twisting Nether trinket to get the localized values for you.
Lines 267268:
if not HasSoulstone() then return end
local reason = HasSoulstone()
Just a minor thing, and this is probably the original code from DathRarhek, but you could easily make that more efficient by only calling one function:
local reason = HasSoulstone()
if not reason then return end
And while you're making changes, I'd also recommend doing something about lines 134140:
if not target then
if not reportBugOnce then
print("Possible bug in LibResComm-1.0. Please file a ticket at http://www.wowace.com/addons/librescomm-1-0/tickets/")
reportBugOnce = true
end
target = "Target Unknown"
end
I've seen the message printed once or twice since I started using LRC about two years ago, and I think it may be caused by using mouseover-targeted casting on a corpse after the player released their spirit, since in this situation UNIT_SPELLCAST_SENT won't report the target and the lib's WorldFrame OnClick hook won't fire since you're not actually clicking on the corpse. You could try to scan the tooltip at this time (target unknown), or you could just quit out. Either way, there's not really any point in sending res comms with the name "Target Unknown".
Why would that be affected? The change you read about pertains to global variables that will no longer be set. The code on lines 4649 does not do anything with global variables. You can name local variables anything you want, except for a small number of keywords reserved for the Lua language itself (such as "function", "for", "if", "then", "else", "end", "local", etc).
Thank you for the feedback, especially since the ticket was posted by you. I'm still learning pattern matching from James "Cladhaire" Whitehead II's book, World of Warcraft Programming Second Edition. Tricky stuff translating what DathRarhek wrote into something of which I can make sense.
You are correct (naturally) about HasSoulstone() returning localized values, so here's the change (yes, the original HasSoulstone() was DathRarhek's):
function lib:popupFuncCanRes()
local reason = HasSoulstone()
if not reason then return end
lib.Callbacks:Fire("ResComm_CanRes", playerName, reason)
commSend("CANRES")
commSend("CANRES "..reason)
end
Rather than using an if/then to send individual commSends, just append whatever is returned to CANRES. I also used your shortened code. Also, hopefully I got the new return for reason correct in the pattern matching. Still pulling my hair out trying to learn that.
function lib.eventFrame:CHAT_MSG_ADDON(prefix, msg, distribution, sender)
if prefix ~= "CTRA" then return end
if sender == playerName then return end
local target
local real_sender, realm = string.split("-", sender, 2)
-- if the sender is on the current realm, real_sender will be nil - set it to sender
sender = real_sender or sender
for cmd, targetName, reason in msg:gmatch("(%a+)%s?([^#]*)(%a+)%s?") do
-- A lot of garbage can come in, make absolutely sure we have a decent message
if cmd == "RES" and targetName ~= "" and targetName ~= UNKNOWN then
<snip>
elseif cmd == "CANRES" then
lib.Callbacks:Fire("ResComm_CanRes", sender, reason)
<snip>
end
end
end
And finally, your other suggestion to deal with "target unknown" comms (I left in the original code, just commented out):
if not target then
if GameTooltipTextLeft1:IsVisible() then
target = select(3, GameTooltipTextLeft1:GetText():find(corpseOf))
--[[
if not reportBugOnce then
print("Possible bug in LibResComm-1.0. Please file a ticket at http://www.wowace.com/addons/librescomm-1-0/tickets/")
reportBugOnce = true
end
target = "Target Unknown"
]]--
end
end
Rather than using an if/then to send individual commSends, just append whatever is returned to CANRES. I also used your shortened code. Also, hopefully I got the new return for reason correct in the pattern matching. Still pulling my hair out trying to learn that.
You shouldn't even need to modify the pattern matching line; the targetName capture should catch anything appended to CANRES comms, since that capture would match "RES Myrroddin" and "CANRES Use Soulstone" just the same.
However, you probably shouldn't just send the raw, localized value from UseSoulstone(), as this will cause problems in regions that support multiple language clients. For example, on a European server, you could have a party with one French client, one English client, one Spanish client, and two German clients. Sending "Usa Piedra de alma" to someone who doesn't speak Spanish wouldn't make much sense. A better solution would be to reverse-translate the value before sending it, and then translate it into the current user's locale upon receipt. This would also enable you to send, for example, "Soulstone" to the callback instead of the full "Use Soulstone" string which only really makes sense in the context of a dialog giving you an choice of actions.
local soulstoneToken = {
["Use Soulstone"] = "SS",
["Reincarnate"] = "RE",
["Twisting Nether"] = "TN",
}
local soulstoneText = {
["SS"] = "Soulstone", -- match item name
["RE"] = GetSpellInfo(20608) -- match spell name "Reincarnation"
["TN"] = "Twisting Nether",
}
commSend("CANRES" .. soulstoneToken[reason])
lib.Callbacks:Fire("ResComm_CanRes", playerName, soulstoneText[reason])
And finally, your other suggestion to deal with "target unknown" comms (I left in the original code, just commented out):
You should probably add a "return" in there if the target's still nil after that, since a "hey I'm casting a res!" comm isn't really useful without a target.
if not target then
if GameTooltipTextLeft1:IsVisible() then
target = select(3, GameTooltipTextLeft1:GetText():find(corpseOf))
end
if not target then return end -- still nothing
end
That makes sense, mostly. I didn't know different locales could play on the same servers in Europe. However, (and sorry if I am becoming a pain) I understand the changes here:
function lib:popupFuncCanRes()
local reason = HasSoulstone()
if not reason then return end
local soulstoneToken = {
["Use Soulstone"] = "SS",
["Reincarnate"] = "RE",
["Twisting Nether"] = "TN",
}
local soulstoneText = {
["SS"] = "Soulstone", -- match item name
["RE"] = GetSpellInfo(20608) -- match spell name "Reincarnation"
["TN"] = "Twisting Nether",
}
commSend("CANRES" .. soulstoneToken[reason])
lib.Callbacks:Fire("ResComm_CanRes", playerName, soulstoneText[reason])
end
But what I can gather of HasSoulstone() is that is is player-exclusive, even though that doesn't make any logical sense if you wanted to check other units. And because I am doing this to learn comms for this lib and something else I'm trying, I must ask if the following is correct, if I do not need to change the pattern match line:
function lib.eventFrame:CHAT_MSG_ADDON(prefix, msg, distribution, sender)
if prefix ~= "CTRA" then return end
if sender == playerName then return end
local target
local real_sender, realm = string.split("-", sender, 2)
-- if the sender is on the current realm, real_sender will be nil - set it to sender
sender = real_sender or sender
for cmd, targetName in msg:gmatch("(%a+)%s?([^#]*)") do
-- A lot of garbage can come in, make absolutely sure we have a decent message
if cmd == "RES" and targetName ~= "" and targetName ~= UNKNOWN then
local endTime = select(6, UnitCastingInfo(sender)) or (GetTime() + 10)*1000
if endTime and targetName then
endTime = endTime / 1000
activeRes[sender] = targetName
lib.Callbacks:Fire("ResComm_ResStart", sender, endTime, targetName)
end
elseif cmd == "RESNO" then
if activeRes[sender] then
target = activeRes[sender]
activeRes[sender] = nil
end
lib.Callbacks:Fire("ResComm_ResEnd", sender, target)
elseif cmd == "RESSED" then
if activeRes[sender] then
target = activeRes[sender]
activeRes[sender] = nil
end
lib.Callbacks:Fire("ResComm_Ressed", sender)
elseif cmd == "CANRES" then
local soulstoneToken = {
["Use Soulstone"] = "SS",
["Reincarnate"] = "RE",
["Twisting Nether"] = "TN",
}
local soulstoneText = {
["SS"] = "Soulstone", -- match item name
["RE"] = GetSpellInfo(20608) -- match spell name "Reincarnation"
["TN"] = "Twisting Nether",
}
lib.Callbacks:Fire("ResComm_CanRes", sender, soulstoneText[reason])
elseif cmd == "NORESSED" then
lib.Callbacks:Fire("ResComm_ResExpired",sender)
end
end
end
I reverted said pattern match back to DathRarhek's code, but then thought "it can't work that way!" when it came to the altered CanRes callback, simply because reason doesn't seem to be passed through, unless I'm misunderstanding something.
About the translations, "Soulstone" has multiple links on WowHead, depending on strength, so that's a bit of confusion.
But what I can gather of HasSoulstone() is that is is player-exclusive, even though that doesn't make any logical sense if you wanted to check other units.
HasSoulstone() is player-exclusive, correct.
Let's say you're using a Spanish client, and the guy you're playing with is using a German client.
When you die, the copy of LRC running in your client calls HasSoulstone() and gets the string "Usa piedra de alma" back. Now, if you just send "CANRES Usa piedra de alma" to your German group member, that's not very helpful. So, under my proposal, LRC will use the soulstoneToken table to look up "Usa piedra de alma" and get the token "SS", which is locale-independent, and uses less comm bandwidth by virtue of being shorter. LRC will then send the comm "CANRES SS".
Now, the copy of LRC running in the German client receives the comm "CANRES SS". His LRC will use the soulstoneText table to look up the token "SS" and get the localized string "Seelenstein". Finally, LRC will pass both the token ("SS") and the string ("Seelenstein") along with the "ResComm_CanRes" callback.
Passing the string allows addons to display text that makes sense to the local user (eg. the German guy sees "Seelenstein" even though you see "Usa piedra de alma") without having to maintain their own translation tables.
Passing the token in addition to the string allows addons to do things other than show text, without having to maintain translations. This wasn't in my original suggestion, but it did occur to me while I was typing out this explanation. For example, an addon could easily map "SS" to the Soulstone item icon, "RE" to the Ankh item icon, and "TN" to the Darkmoon Card: Twisting Nether item icon, and show icons instead of text to indicate which type of self-res was available. I've seen addons that show the icon of the res spell being cast, so this would be a logical extension. Similarly, an addon could color a frame purple for "SS", some other color for "RE", and another color for "TE".
I reverted said pattern match back to DathRarhek's code, but then thought "it can't work that way!" when it came to the altered CanRes callback, simply because reason doesn't seem to be passed through, unless I'm misunderstanding something.
Right. In the code you'd posted, reason would always be nil, because it was the third in a series of variables assigned to the return values from a string match with only two captures.
About the translations, "Soulstone" has multiple links on WowHead, depending on strength, so that's a bit of confusion.
I've never heard anyone in-game say "Can I get a Greater Soulstone please?" or "Hey, my Demonic Soulstone only has 3 minutes left!". Everyone just says "soulstone", regardless of what rank it is. Therefore, you should use the item name that's just "Soulstone" with no adjectives.
Here is a pastey with all of the changes we've discussed applied. I didn't test it extensively, but it loads without errors, so the rest of it probably works too. I'll report back with any changes that need to be made if something doesn't work. Feel free to look through it and ask about anything you don't understand.
Also, the translations for "Corpse of X" aren't necessary anymore (if they ever were) as there is a global string available for this. This change is also included.
Turns out DathRarhek stopped playing WoW about six months ago, and he assigned me manager/author for LibResComm-1.0, and I am wondering if Phanx would like to be added, as (s)he has contributed as much or more than I to this project.
That's the good news. The bad news is that while testing r53 in ICC last night, I got an error that I've never seen before. What is an unfinished capture, and how do I fix it? I created a ticket for myself as reference.
Next question is just general about repositories and/or SVN. LRC has always had its build as the tag, and I don't want to mess with end users. What should I use as the tag name to propagate to Curse when it's time?
Finally, the tag name doesn't really matter at all. Most users will never even look at the version number of any addon they're using. Using the revision number is fine.
r61 is up on Curse. I used WoW Global Finder and interestingly, there was a long standing typo. The code had both combatresSpell and combatResSpell. That's fixed. I also localized a slew of global variables and APIs so LibResComm-1.0 shouldn't pollute the global name space anymore.
For those interested in helping, even if you do not use LRC, there are 4 strings that need to be translated into other languages, and I'm looking for help, please and thank you. Phanx translated one of the strings already.
Mass Resurrection
100 yd range
10 sec castBrings all dead party and raid members back to life with 35% health and 35% mana. A player may only be resurrected by this spell once every 10 minutes. Cannot be cast in combat or while in a battleground or arena.
I did think about it, but there are some drawbacks to including this in a comm lib:
"A player may only be resurrected by this spell once every 10 minutes."
Since it casts on all non-ghost characters within the group, then there is no point in sending it across the addon com.
Anyone who has released will be further than 100 yards anyway, because they will be outside the instance.
Dead people further than 100 yards probably don't need Mass Resurrection but rather a local area cast. Instance combat zones are not bigger than 100 yards.
Mass Resurrection is supported in SmartRes2, although not as a key bind, but rather as res bars showing caster and targets.
There's an issue with the localization of LibResComm-1.0. Not sure if you're using the localization application on WoW Ace or just doing it manually, but... Well, easier to explain:
local LOCALE = GetLocale()
if LOCALE == "deDE" then
[COLOR=Red]{[/COLOR]
["Reincarnate"] = "Reinkarnation",
["Soulstone"] = "Seelenstein",
["Twisting Nether"] = "Wirbelnder Nether",
["Use Soulstone"] = "Seelenstein benutzen",
}
See the red curly bracket? That throws an error. There's no variable defined for the table. The code in those blocks should look something like:
local LOCALE = GetLocale()
if LOCALE == "deDE" then
L["Reincarnate"] = "Reinkarnation"
L["Soulstone"] = "Seelenstein"
L["Twisting Nether"] = "Wirbelnder Nether"
L["Use Soulstone"] = "Seelenstein benutzen"
}
Thanks, Storm. I am trying to sort out using the keyword substitutions on the lua programming thread. I know about the error. That's the problem with not having an active WoW account. :(
You can take a look at ButtonFacade or BlizzFacade to see how I handled it for the localization application. Seems to me to be simplest and most effective.
I have an informal request. I don't know what constraints you're operating under, so this may not be feasible, but: could "release" tags be delayed until somebody (you or whoever) has had a chance to login with the alpha code and test it for syntax errors and whatnot?
I have an informal request. I don't know what constraints you're operating under, so this may not be feasible, but: could "release" tags be delayed until somebody (you or whoever) has had a chance to login with the alpha code and test it for syntax errors and whatnot?
From now on, yes. I was a bit constrained because my guild, even though I am not currently active, was asking for updates. I was kind of under the gun, as it were.
Normally, yeah, I would, and will from now on, gets alphas up for testing, and leave them there for a while.
I would like to apologize for all the snafus. I am truly sorry.
If you can't log in to test, ask someone else to do it. My account isn't currently active either, but Akkorian is almost always willing to test whatever.
In the meantime, I sent a PM to DathRarhek earlier today to see if he is still around to maintain this library. If not, I may take it over.
Here is the full code: http://paste.wowace.com/2472/
Your change on line 161 won't actually do anything, since you didn't add any captures to the pattern passed to string.gmatch, so reason will always be nil.
Your change on line 185 will similarly do nothing, since reason will always be nil.
Your changes on lines 270-278 will only work in English locales; the string value returned by the HasSoulstone API are localized. The "Reincarnate" value (shown to a dead shaman with Reincarnation not on cooldown) may be taken from the global string USE_SOULSTONE, but since there are no global strings matching "Use Soulstone" or "Twisting Nether", that may not hold true for other locales. You would need to find people who play in each locale and have access to a shaman, a warlock, and a character with the Darkmoon Card: Twisting Nether trinket to get the localized values for you.
Lines 267268:
Just a minor thing, and this is probably the original code from DathRarhek, but you could easily make that more efficient by only calling one function:
And while you're making changes, I'd also recommend doing something about lines 134140:
I've seen the message printed once or twice since I started using LRC about two years ago, and I think it may be caused by using mouseover-targeted casting on a corpse after the player released their spirit, since in this situation UNIT_SPELLCAST_SENT won't report the target and the lib's WorldFrame OnClick hook won't fire since you're not actually clicking on the corpse. You could try to scan the tooltip at this time (target unknown), or you could just quit out. Either way, there's not really any point in sending res comms with the name "Target Unknown".
Why would that be affected? The change you read about pertains to global variables that will no longer be set. The code on lines 4649 does not do anything with global variables. You can name local variables anything you want, except for a small number of keywords reserved for the Lua language itself (such as "function", "for", "if", "then", "else", "end", "local", etc).
You are correct (naturally) about HasSoulstone() returning localized values, so here's the change (yes, the original HasSoulstone() was DathRarhek's):
Rather than using an if/then to send individual commSends, just append whatever is returned to CANRES. I also used your shortened code. Also, hopefully I got the new return for reason correct in the pattern matching. Still pulling my hair out trying to learn that.
And finally, your other suggestion to deal with "target unknown" comms (I left in the original code, just commented out):
You shouldn't even need to modify the pattern matching line; the targetName capture should catch anything appended to CANRES comms, since that capture would match "RES Myrroddin" and "CANRES Use Soulstone" just the same.
However, you probably shouldn't just send the raw, localized value from UseSoulstone(), as this will cause problems in regions that support multiple language clients. For example, on a European server, you could have a party with one French client, one English client, one Spanish client, and two German clients. Sending "Usa Piedra de alma" to someone who doesn't speak Spanish wouldn't make much sense. A better solution would be to reverse-translate the value before sending it, and then translate it into the current user's locale upon receipt. This would also enable you to send, for example, "Soulstone" to the callback instead of the full "Use Soulstone" string which only really makes sense in the context of a dialog giving you an choice of actions.
You should probably add a "return" in there if the target's still nil after that, since a "hey I'm casting a res!" comm isn't really useful without a target.
But what I can gather of HasSoulstone() is that is is player-exclusive, even though that doesn't make any logical sense if you wanted to check other units. And because I am doing this to learn comms for this lib and something else I'm trying, I must ask if the following is correct, if I do not need to change the pattern match line:
I reverted said pattern match back to DathRarhek's code, but then thought "it can't work that way!" when it came to the altered CanRes callback, simply because reason doesn't seem to be passed through, unless I'm misunderstanding something.
About the translations, "Soulstone" has multiple links on WowHead, depending on strength, so that's a bit of confusion.
HasSoulstone() is player-exclusive, correct.
Let's say you're using a Spanish client, and the guy you're playing with is using a German client.
When you die, the copy of LRC running in your client calls HasSoulstone() and gets the string "Usa piedra de alma" back. Now, if you just send "CANRES Usa piedra de alma" to your German group member, that's not very helpful. So, under my proposal, LRC will use the soulstoneToken table to look up "Usa piedra de alma" and get the token "SS", which is locale-independent, and uses less comm bandwidth by virtue of being shorter. LRC will then send the comm "CANRES SS".
Now, the copy of LRC running in the German client receives the comm "CANRES SS". His LRC will use the soulstoneText table to look up the token "SS" and get the localized string "Seelenstein". Finally, LRC will pass both the token ("SS") and the string ("Seelenstein") along with the "ResComm_CanRes" callback.
Passing the string allows addons to display text that makes sense to the local user (eg. the German guy sees "Seelenstein" even though you see "Usa piedra de alma") without having to maintain their own translation tables.
Passing the token in addition to the string allows addons to do things other than show text, without having to maintain translations. This wasn't in my original suggestion, but it did occur to me while I was typing out this explanation. For example, an addon could easily map "SS" to the Soulstone item icon, "RE" to the Ankh item icon, and "TN" to the Darkmoon Card: Twisting Nether item icon, and show icons instead of text to indicate which type of self-res was available. I've seen addons that show the icon of the res spell being cast, so this would be a logical extension. Similarly, an addon could color a frame purple for "SS", some other color for "RE", and another color for "TE".
Right. In the code you'd posted, reason would always be nil, because it was the third in a series of variables assigned to the return values from a string match with only two captures.
I've never heard anyone in-game say "Can I get a Greater Soulstone please?" or "Hey, my Demonic Soulstone only has 3 minutes left!". Everyone just says "soulstone", regardless of what rank it is. Therefore, you should use the item name that's just "Soulstone" with no adjectives.
Here is a pastey with all of the changes we've discussed applied. I didn't test it extensively, but it loads without errors, so the rest of it probably works too. I'll report back with any changes that need to be made if something doesn't work. Feel free to look through it and ask about anything you don't understand.
http://paste.wowace.com/2477/
Also, the translations for "Corpse of X" aren't necessary anymore (if they ever were) as there is a global string available for this. This change is also included.
That's the good news. The bad news is that while testing r53 in ICC last night, I got an error that I've never seen before. What is an unfinished capture, and how do I fix it? I created a ticket for myself as reference.
Next question is just general about repositories and/or SVN. LRC has always had its build as the tag, and I don't want to mess with end users. What should I use as the tag name to propagate to Curse when it's time?
The last " and ) were switched.
And sure, if you want to add me as an author.
Finally, the tag name doesn't really matter at all. Most users will never even look at the version number of any addon they're using. Using the revision number is fine.
Adding you as an author, Phanx.
For those interested in helping, even if you do not use LRC, there are 4 strings that need to be translated into other languages, and I'm looking for help, please and thank you. Phanx translated one of the strings already.
i think it would be a valid addition to LibResComm.
See the red curly bracket? That throws an error. There's no variable defined for the table. The code in those blocks should look something like:
I've commented out the section on my local copy.
Normally, yeah, I would, and will from now on, gets alphas up for testing, and leave them there for a while.
I would like to apologize for all the snafus. I am truly sorry.