Authors CurseForge

if not tonumber(index) == nil then

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

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().

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.