is easier to read and understand when written that way:
if tonumber(index) then
You could also convert index for once to avoid needless function calls.
index = tonumber(index)
if index then
currentVoiceChannel = index
But actually you are checking function parameters, so you could simply do this:
function RV:SetVoiceChannel(index)
index = tonumber(index)
assert(index, ":SetVoiceChannel(index): index must be a number")
currentVoiceChannel = index
wipe(RV.talkers)
RV.events:Fire("RAIDVOICE_UPDATE")
end
--------------
String pattern matching are expansive so you would like to do them only once. And check string pattern methods: string.gsub purpose is to do replacements, not extracting values. Use string.find or string.match for that.
E.g. instead of doing:
local recievedChannel = tonumber(string.gsub(message, "!CH(%d*);(.*)", "%1"),10)
Just do:
local recievedChannel = tonumber(string.match(message, "!CH(%d*)")
Actually your code parses a full string and splits it into parts. This can be done in one call to string.match (adding "^" and "$" to match the whole string):
local chNumber, chMessage = string.match(message, "^!CH(%d+):(.*)$")
You don't need to recall CHAT_MSG_ADDON with the new message, just use it in place of the existing one (hence message=channelMessage).
And last, channel messages should only occurs on GUILD distribution, so you could check it to prevent unnecessary processing and ensure consistency.
All this applied to your code:
function RV:CHAT_MSG_ADDON(event, prefix, message, distribution, sender)
if prefix ~= 'RAIDVOICE' then return end
if distribution == "GUILD" then
local channelNumber, channelMessage = string.match(message, "^!CH(%d+):(.*)$")
channelNumber = tonumber(channelNumber)
if channelNumber then
if channelNumber == currentVoiceChannel then
-- Same channel, parse the message
message = channelMessage
else
return -- Ignore message on other channels
end
end
end
-- continue with processing...
-----
I think you should change this:
if playerSpeaking then SendAddonMessage( "RAIDVOICE", playerName, "RAID" ) end
By this:
if playerSpeaking then RV:SpeaktoChannel(playerName) end
So it respects the channel voice setting.
----
Considering SpeakToChannel, GetNumPartyMembers() always returns a positive number when you are in a party or a raid, so there is no need to check GetNumRaidMembers().
function RV:SpeakToChannel(message)
if currentVoiceChannel == 0 then
-- Channel #0: raid/party
if GetNumPartyMembers() > 0 then
SendAddonMessage("RAIDVOICE", message, "RAID")
end
elseif currentVoicChannel == 100 then
-- Channel #100: battlegrounds
if select(2, IsInIstance()) == "pvp" then
SendAddonMessage("RAIDVOICE", message, "BATTLEGROUND")
end
else
-- Any other channel: guild channel
SendAddonMessage("RAIDVOICE", "!CH"..currentVoiceChannel..";"..message, "GUILD")
end
end
You don't need to recall CHAT_MSG_ADDON with the new message, just use it in place of the existing one (hence message=channelMessage).
And last, channel messages should only occurs on GUILD distribution, so you could check it to prevent unnecessary processing and ensure consistency.
All this applied to your code:
function RV:CHAT_MSG_ADDON(event, prefix, message, distribution, sender)
if prefix ~= 'RAIDVOICE' then return end
if distribution == "GUILD" then
local channelNumber, channelMessage = string.match(message, "^!CH(%d+):(.*)$")
channelNumber = tonumber(channelNumber)
if channelNumber then
if channelNumber == currentVoiceChannel then
-- Same channel, parse the message
message = channelMessage
else
return -- Ignore message on other channels
end
end
end
-- continue with processing...
[
Does your regular expression take this example?:
CH2!;!CLEAR
I think the message would then be ;!CLEAR not !CLEAR or am i wrong?
EDIT:
okay, got it should be
string.match(message, "^!CH(%d+);(.*)$")
Considering SpeakToChannel, GetNumPartyMembers() always returns a positive number when you are in a party or a raid, so there is no need to check GetNumRaidMembers().
I use the GetRealNumPartyMembers() and GetRealNumRaidMembers() to check if player is in party or raid (not checking for negative value)
Because,for examples If you send message to raid and you are not part of raid, but in battleground, this causes the error you are not in party to appear in console.
Oh, I see. Anyway, does GetRealNumPartyMembers() return 0 when you are in a non-battleground raid ? If it doesn't, there's no need to use GetRealNumRaidMembers().
It should, Blizzard LFG Frame uses similar conditions....
Anyway i have other question, is there a smart way to determine that I just joined party/raid?
I want to add a table holding all raidvoice capable players.This could be useful to see which players see you speaking. My idea was that this new player with query the party/raid, causing them to add him to this table. But if i use PARTY_MEMBERS_CHANGED it will trigger for each player which i do not want, as it would cause excessive communication. I came up with this...
IIRC, PARTY_MEMBERS_CHANGED is only triggered once when you join a party/raid. It is then triggered when someone joins/leaves the party/raid or when players are moved around in raid groups.
Your code looks fine. It should cause only new joiners to send a "!PING". Obviously, in case of mass invitation it would cause a lot of "!PING" to be sent.
As it turns out this causes a glitch in my addon raidvoice, because he is currently set thus, that recieving response to !PING he prints player name, addon version and channel from recieved response every time i join a party. By desing a slashcommand /raidvoice ping sengs !PING and response is immediately printed when recieved, but this obviously is tampered with addition of libraidvoice sending !PING on party join. Is there a neat way to send command store responses somewhere and print them only when explicitely asked? I have an idea myself, but i dont like it very much.
Rollback Post to RevisionRollBack
To post a comment, please login or register a new account.
is easier to read and understand when written that way:
You could also convert index for once to avoid needless function calls.
But actually you are checking function parameters, so you could simply do this:
--------------
String pattern matching are expansive so you would like to do them only once. And check string pattern methods: string.gsub purpose is to do replacements, not extracting values. Use string.find or string.match for that.
E.g. instead of doing:
Just do:
Actually your code parses a full string and splits it into parts. This can be done in one call to string.match (adding "^" and "$" to match the whole string):
You don't need to recall CHAT_MSG_ADDON with the new message, just use it in place of the existing one (hence message=channelMessage).
And last, channel messages should only occurs on GUILD distribution, so you could check it to prevent unnecessary processing and ensure consistency.
All this applied to your code:
-----
I think you should change this:
By this:
So it respects the channel voice setting.
----
Considering SpeakToChannel, GetNumPartyMembers() always returns a positive number when you are in a party or a raid, so there is no need to check GetNumRaidMembers().
Does your regular expression take this example?:
CH2!;!CLEAR
I think the message would then be ;!CLEAR not !CLEAR or am i wrong?
EDIT:
okay, got it should be
string.match(message, "^!CH(%d+);(.*)$")
I use the GetRealNumPartyMembers() and GetRealNumRaidMembers() to check if player is in party or raid (not checking for negative value)
Because,for examples If you send message to raid and you are not part of raid, but in battleground, this causes the error you are not in party to appear in console.
Anyway i have other question, is there a smart way to determine that I just joined party/raid?
I want to add a table holding all raidvoice capable players.This could be useful to see which players see you speaking. My idea was that this new player with query the party/raid, causing them to add him to this table. But if i use PARTY_MEMBERS_CHANGED it will trigger for each player which i do not want, as it would cause excessive communication. I came up with this...
http://paste.wowace.com/dq7wiykljshbn3ut/
Your code looks fine. It should cause only new joiners to send a "!PING". Obviously, in case of mass invitation it would cause a lot of "!PING" to be sent.