Discussion on SmartRes2's forums has led to a suggestion by xbeeps to update LibResComm-1.0 to support cross realm targets and units. His explanation rightfully suggests that such code should be in the lib, and not in every addon that needs it. xbeeps explains it better than I.
It's better to use unit names rather than units. All API functions that take a unitID can also take a unit name as long as you're grouped, which is always the case.
Anyway, you need to make sure the code is aware of the fully qualified names ("Name-Realm"). I just checked the LibResComm code and it currently isn't, but it can be fixed in the addon, if you want to, because it only affects reception.
When you receive a ress message you need to consider if the name of the target is really correct. Consider this:
Player P1 is on Realm A
Player P2 is on Realm B
Player P3 is on Realm B
P2 resses P3
P2 will transmit a message where it says "RES P3". Because P2 and P3 are on the same realm the name given will be "P3", because that is the fully qualified name as seen from P2.
However, from the point of view of P1, who (also) receives the message, "P3" is known as "P3-B", because P1 and P3 are on different realms.
Another situation is when for example P3 resses P1. When P1 receives that message it will be "P1-A", and the realm needs to be stripped from that name to actually work. Note that P2 will also receive the message, but will *not* have to strip the realm!
Consequently, if nothing is done about the name, it will not be successfull when using the name in API's.
To fix that, you need to convert the names received. In LibHealComm-3.0 i use the following code to do that:
local function extractRealm(fullName)
return fullName:match("^[^%-]+%-(.+)$");
end
-- Convert a remotely generated fully qualified name to
-- a local fully qualified name.
local function convertRealm(fullName, remoteRealm)
if (remoteRealm) then
local name, realm = fullName:match("^([^%-]+)%-(.+)$");
if (not realm) then
-- Apply remote realm if there is no realm on the target
return fullName .. "-" .. remoteRealm;
elseif (realm == playerRealm) then
-- Strip realm if it is equal to the local realm
return name;
end
end
return fullName;
end
Then, when you get a name in, you do something like this:
Where targetName is the name of the target and sender is the name of the player sending the message.
Ideally this should be build into LibResComm-1.0 similar to how it is built into LibHealComm-3.0, so that the fully qualified names delivered by the library are always correct instead of reflecting the sender's realm.
Therefore, I went looking at LRC-1's code and as I know even less about lib coding that addon coding, I ran into trouble with LRC-1's events using both unit and targetName, plus the IsUnitBeingRessed() API that returns the unit and resser. I do not want to break the lib, so I'm zipping up what I have to be assessed (and no doubt fixed) by more knowledgeable minds.
Unfortunately that isn't a viable change, as LibResComm-1.0 must maintain compatibility with the CT_Raid comm format. Even if that weren't the case, it's never appropriate to make a backwards-incompatible change in a public library without incrementing the major version. Either way, you'd need to write a LibResComm-1.1 or LibResComm-2.0 if you wanted to make that kind of change.
I bumped the minor from 48 to 49, and added lines 100, changed lines 123, 142-147, and lines 165-171.
Note: I did not commit this change to the repository; I am posting here for feedback. AFAIK, Goblin Jumper Cables (both) and the Gnomish Army Knife cast Defibrillate as their on-use spell, which has a set casting time of 4 seconds. If this is working, a toc bump and updating LibResComm-1.0.lua can be done.
I do not believe this will break backward compatibility, but that's why I am posting here first.
Are you getting the localised name from spellid and use the name for the detection?
If you do so, we run into translation problems becaue the goblin jumper cables and the army knive have 2 different translations for their spells in german.
Assuming I did it correctly, the only spell that needed pulling the ID was Defibrillate; meaning it is not necessary to get item IDs for the cables or the knife, as all three use the same on-use spell.... I hope.
They don't. On the page you linked, it clearly lists that that particular spell ID is only used for the Gnomish Army Knife. There's also one for Goblin Jumper Cables XL and one for Goblin Jumper Cables. Any time there are multiple spells whose names are identical to each other in English, chances are good that their names aren't identical to each other in at least one other locale.
In any case, I'm not sure what the practical use of including this in LibResComm would be. If you have someone available with a normal res spell, I doubt your engineers are popping their fail-prone trinkets, and if you don't have someone available with a normal res spell, the chances of there being enough engineers in your group that they're overlapping res attempts are slim to none. Other issues include the fact that engineer res fails quite often, and the fact that the CT_Raid doesn't send res comms for engineer res attempts. Strictly speaking though, I guess it wouldn't break compatibility, if properly implemented.
Goblin Jumper Cables XL uses Defibrillate 22999 Defibrillation in german Goblin Jumper Cables uses Defibrillate 8342 Defibrillation in german Gnomish Army Knife uses Defibrillate 54732 Defilibrierenin german
as you can see, they arent translating things consistently like many other examples show.
At Torhal's suggestion on the SmartRes2 forum, I modified LibResComm-1.0 thus: http://paste.wowace.com/2157/ I also bumped the minor from 49 to 50.
Lines added or changed: 117 - 124, 160 - 166 (commented out, unnecessary?), and 168 - 172. If this works (seems to be, but needs more testing) then I will commit a new alpha.
This is to add cross-realm support for targetName and sender. Feedback welcome and appreciated.
You don't need to manually bump the minor; that is automatically done by the packager, as LRC simply uses its SVN revision number as the minor version.
Be warned that using the svn revision as minor as the advantage of auto-bumping, but also a drawback: I doesn't work well if you move (=migrate) or copy(=hard-embed) the file into another SVN repository.
As for the change:
1) define the svn:keywords property on the file (see svn doc)
svn propset svn:keywords Revision myLib-1.0.lua
2) use the keywords in the myLib-1.0.lua:
local MAJOR = "myLib-1.0"
local MINOR = tonumber(string.match("$Revision: 0 $", "%d+"))
local lib = LibStub:NewLibrary(MAJOR, MINOR)
The "$Revision: 0 $" string will automatically be replaced by something like "$Revision: 144 $" on checkout and the string.match catchs the number. Alternatively, some people prefer using string.sub:
local MINOR = tonumber(string.sub("$Revision: 0 $", 12, -3))
Cleaned up and commited to the SVN. However, I noticed the keyword substitution did not work, and I did not touch the code. In my working copy, I undid the manual minor bump as per Phanx's information. That means the faulty code is original. The repository version is r50 as of this writing, but as you can see from the pasty, the minor is still 48.
Neither; it is an SVN property, and is set through your SVN client. However, when you did an SVN checkout, the property should have been automatically set, just like SVN externals, SVN eol-style, and/or any other properties that were previously set by other developers.
Neither; it is an SVN property, and is set through your SVN client. However, when you did an SVN checkout, the property should have been automatically set, just like SVN externals, SVN eol-style, and/or any other properties that were previously set by other developers.
Yeah, found out by asking someone in IRC. SVN:keyword had been removed or not set, and now it is. Build r51 worked great.
Works great; see the SmartRes2 thread in the general addons forum. If there is a moderator or admin, or even Dathrarhek if he's around, this can be pushed to Curse.
Rollback Post to RevisionRollBack
To post a comment, please login or register a new account.
Therefore, I went looking at LRC-1's code and as I know even less about lib coding that addon coding, I ran into trouble with LRC-1's events using both unit and targetName, plus the IsUnitBeingRessed() API that returns the unit and resser. I do not want to break the lib, so I'm zipping up what I have to be assessed (and no doubt fixed) by more knowledgeable minds.
http://paste.wowace.com/2058/
I bumped the minor from 48 to 49, and added lines 100, changed lines 123, 142-147, and lines 165-171.
Note: I did not commit this change to the repository; I am posting here for feedback. AFAIK, Goblin Jumper Cables (both) and the Gnomish Army Knife cast Defibrillate as their on-use spell, which has a set casting time of 4 seconds. If this is working, a toc bump and updating LibResComm-1.0.lua can be done.
I do not believe this will break backward compatibility, but that's why I am posting here first.
If you do so, we run into translation problems becaue the goblin jumper cables and the army knive have 2 different translations for their spells in german.
In any case, I'm not sure what the practical use of including this in LibResComm would be. If you have someone available with a normal res spell, I doubt your engineers are popping their fail-prone trinkets, and if you don't have someone available with a normal res spell, the chances of there being enough engineers in your group that they're overlapping res attempts are slim to none. Other issues include the fact that engineer res fails quite often, and the fact that the CT_Raid doesn't send res comms for engineer res attempts. Strictly speaking though, I guess it wouldn't break compatibility, if properly implemented.
Goblin Jumper Cables uses Defibrillate 8342 Defibrillation in german
Gnomish Army Knife uses Defibrillate 54732 Defilibrieren in german
as you can see, they arent translating things consistently like many other examples show.
Lines added or changed: 117 - 124, 160 - 166 (commented out, unnecessary?), and 168 - 172. If this works (seems to be, but needs more testing) then I will commit a new alpha.
This is to add cross-realm support for targetName and sender. Feedback welcome and appreciated.
I'll look at the actual changes later.
As for the change:
1) define the svn:keywords property on the file (see svn doc)
2) use the keywords in the myLib-1.0.lua:
The "$Revision: 0 $" string will automatically be replaced by something like "$Revision: 144 $" on checkout and the string.match catchs the number. Alternatively, some people prefer using string.sub:
Was wondering specifically about the changes to the library code; but again, thanks for the clarification about the svn keyword autobumping.
Is there any particular reason you wouldn't just do:
(Also, you wrote "send" instead of "sent" so your code would never set the variable actually used by the library.)
Again:
Should be simplified to just:
Other than that, I don't see any functional problems.
http://paste.wowace.com/2167/
Anyway, once that is fixed, could some admin bump a new version to Curse, please?
All fixed up. Now need admin to push to Curse, please and thank you.
Yeah, found out by asking someone in IRC. SVN:keyword had been removed or not set, and now it is. Build r51 worked great.
Now it needs testing and/or pushing to Curse.