Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Will be a separate addon] Wire Advanced Microphone and Speaker #2722

Closed
wants to merge 31 commits into from

Conversation

stepa2
Copy link
Contributor

@stepa2 stepa2 commented Aug 10, 2023

! This PR will be made into separate addon. It can be used as-is until I get to make this addon.
I'll close this PR when I make the addon.

  • Can reproduce player voice.
    • Tested.
  • Can reproduce sounds received by EntityEmitSound (both clientside and serverside).
  • Can reproduce sounds played by sound.Play.
  • Has tools.
  • Optimize player voice reproduction.
  • Optimize voice (again): cache who can hear whom, update each N ticks (N = convar).

What will not be implemented in this PR:

  • sound.PlayFile and sound.PlayURL support - should be implementable
  • ENT:StopSound hooking - no way to hook engine StopSounds. Wait for Request for GM:EntityStopSound hook, SoundLooped function Facepunch/garrysmod-requests#2176
  • Looping sound support - needs StopSound hooks
  • Change of volume when distance from source to microphone changes
  • Customizable soundlevel of speaker
  • Player voice volume changing based on distance to microphone: via PLY:SetVoiceVolumeScale with wrapper

@Denneisk
Copy link
Member

Denneisk commented Aug 10, 2023

Is this intended to mimic the functionality of the env_microphone? Was always wishing for something like this so this is awesome to see.

@stepa2
Copy link
Contributor Author

stepa2 commented Aug 10, 2023

Yes, it is. In fact, I plan to make it better, as env_microphone does not supports looped sounds and soundscapes, for example.

Copy link
Contributor

@Vurv78 Vurv78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is draft but putting these here since these are design decisions

lua/entities/gmod_wire_adv_microphone.lua Outdated Show resolved Hide resolved
-- because some sounds are played clientside only

-- array(Entity(gmod_wire_adv_microphone))
-- Array instead of lookup table because sounds are emitted more often than microphones switched on or off,
Copy link
Contributor

@Vurv78 Vurv78 Aug 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any notable difference in speed between ipairs and pairs in this case with a small amount of items?

Cause I'd really prefer a lookup table over using table.RemoveByValue and table.insert. Even if I know there won't be more than maybe ten microphones placed in a server at once. At worst you could maintain both a lookup table and an array for the best of both worlds.

I know ipairs can jit but that's just on the x86_64 branch. Although you could use a manual for loop, but you aren't doing that.

Copy link
Contributor

@Grocel Grocel Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can only agree on this. And I just don't get it why it such a problem to just change it. It is not like you are going to need to add like extra 1000 lines of code for it.

@thegrb93
Copy link
Contributor

How's the PlayerCanHearPlayersVoice going to work? It seems right now it doesn't mute players that are too far away from the mic, but it will force you to be able to hear anyone close enough to it if you are close enough to a speaker?

Aren't you able to hear players at any distance by default in sandbox? If this is intended for DarkRP, maybe the hook should only be added when in DarkRP?

@stepa2
Copy link
Contributor Author

stepa2 commented Aug 12, 2023

Yes, it should work (and probably does) exactly as you supposed it does.
I don't know what defaults are in vanilla GMod, I know in ULX voice chat is global by default.
However, both ULX and SAM allow enabling local voice chat in any gamemode, so this is for Sandbox too.

@github-actions
Copy link

This pull request has been marked as stale as there haven't been any changes in the past month. It will be closed in 15 days.

@github-actions github-actions bot added the Stale label Sep 11, 2023
@github-actions github-actions bot removed the Stale label Sep 12, 2023
@stepa2
Copy link
Contributor Author

stepa2 commented Oct 2, 2023

So, I need to test player voice reproduction and PR will be ready

Copy link

This pull request has been marked as stale as there haven't been any changes in the past month. It will be closed in 15 days.

@github-actions github-actions bot added the Stale label Nov 23, 2023
Copy link

This pull request has been marked as stale as there haven't been any changes in the past month. It will be closed in 15 days.

@github-actions github-actions bot added the Stale label Feb 19, 2024
@stepa2 stepa2 marked this pull request as ready for review February 20, 2024 19:22
@github-actions github-actions bot removed the Stale label Feb 20, 2024
@stepa2
Copy link
Contributor Author

stepa2 commented Mar 3, 2024

Can this PR me merged?

@Aloncheeto
Copy link

please wiremod gods make this happen

@Denneisk
Copy link
Member

Denneisk commented Mar 7, 2024

May you fix the lint errors (trailing whitespace)?

It seems okay, there's still a few concerns about performance but I think it's alright for the purpose it has. HasValue and RemoveByValue are used very sparingly at least.

@stepa2
Copy link
Contributor Author

stepa2 commented Mar 7, 2024

Fixed all warnings except goto (used instead of continue).

@Denneisk Denneisk requested a review from thegrb93 March 9, 2024 15:21
Copy link
Member

@Denneisk Denneisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's alright as it is. There's room for micro-optimizations but either way it's just going to be a heavy addition.

Would like to see a stress test done but not holding you to it.

@stepa2
Copy link
Contributor Author

stepa2 commented Mar 10, 2024

This microphone system was used on a live server from the first commit, and there were no performance issues reported.

The most intensive usage pattern is: 5-7 microphones and screens active, 2-3 connected pairs, 1-2 players spoke into each connected microphone.

I don't know how and with whom to do a proper stress testing, though.

Copy link
Contributor

@Grocel Grocel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That table.HasValue issue is still there. But whatever, just pull that thing and have somebody else optimize it if needed afterwards in the case the author still refuses to make the requested changes. Better than being stuck having pullrequest-drama on Discord.

@thegrb93
Copy link
Contributor

thegrb93 commented Apr 4, 2024

This needs a lot of cleanup. I would prefer something like.

local MicrophoneSystem = {}
MicrophoneSystem.ActiveMics = {}
MicrophoneSystem.ActiveSpeakers = {}

local PlayerSpeakerCache = {
    __index = {
        reset = function(self)
            -- Clear all of our tables
            for v in pairs(self.hearableMics) do self.hearableMics[v] = nil end
            for v in pairs(self.hearableSpeakers) do self.hearableSpeakers[v] = nil end
            for v in pairs(self.hearablePlayerMics) do self.hearablePlayerMics[v] = nil end
        end,
        calcHearableDevices = function(self)
            -- Calculate distances to devices
        end,
        calcHearablePlayers = function(self)
            -- Using the speakers this player can hear, see what corresponding mics other players can hear and put this players in self.hearablePlayerMics
        end,
        canHearPlayer = function(self, ply)
            return self.hearablePlayerMics[ply] ~= nil
        end,
        canHearSound = function(self, snd, soundpos)
        end
    },
    __call = function(t, ply)
        return setmetatable({
            player = ply,
            hearableSpeakers = {},
            hearableMics = {},
            hearablePlayerMics = {}
        }, t)
    end
}
setmetatable(PlayerSpeakerCache, PlayerSpeakerCache)
MicrophoneSystem.PlayerSpeakerCaches = WireLib.RegisterPlayerTable(setmetatable({}, {
    __index = function(t,k) local r=PlayerSpeakerCache(k) t[k]=r return r end
}))

function MicrophoneSystem.AddActiveMic(ent)
    MicrophoneSystem.ActiveMics[MicrophoneSystem.ActiveMics+1] = ent
    MicrophoneSystem.AddHooks()
end

function MicrophoneSystem.RemoveActiveMic(ent)
    table.RemoveByValue(MicrophoneSystem.ActiveMics, ent)
    MicrophoneSystem.RemoveHooks()
end

function MicrophoneSystem.AddActiveSpeaker(ent)
    MicrophoneSystem.ActiveSpeakers[MicrophoneSystem.ActiveSpeakers+1] = ent
    MicrophoneSystem.AddHooks()
end

function MicrophoneSystem.RemoveActiveSpeaker(ent)
    table.RemoveByValue(MicrophoneSystem.ActiveSpeakers, ent)
    MicrophoneSystem.RemoveHooks()
end

function MicrophoneSystem.AddHooks()
    if #MicrophoneSystem.ActiveMics == 0 or #MicrophoneSystem.ActiveSpeakers == 0 then return end
    hook.Add("Think", "Wirelib_Microphones", MicrophoneSystem.Think)
    hook.Add("PlayerCanHearPlayersVoice", "Wirelib_Microphones", MicrophoneSystem.PlayerCanHearPlayersVoice)
end

function MicrophoneSystem.RemoveHooks()
    if #MicrophoneSystem.ActiveMics ~= 0 and #MicrophoneSystem.ActiveSpeakers ~= 0 then return end
    hook.Remove("Think", "Wirelib_Microphones")
    hook.Remove("PlayerCanHearPlayersVoice", "Wirelib_Microphones")
end

function MicrophoneSystem.Think()
    for _, ply in player.iterator() do
        MicrophoneSystem.PlayerSpeakerCaches[ply]:reset()
    end
    for _, ply in player.iterator() do
        MicrophoneSystem.PlayerSpeakerCaches[ply]:calcHearableDevices()
    end
    for _, ply in player.iterator() do
        MicrophoneSystem.PlayerSpeakerCaches[ply]:calcHearablePlayers()
    end
end

function MicrophoneSystem.PlayerCanHearPlayersVoice(talker, listener)
    if MicrophoneSystem.PlayerSpeakerCaches[listener]:canHearPlayer(talker) then
        return true
    end
end

@thegrb93
Copy link
Contributor

thegrb93 commented Apr 4, 2024

Note how 'PlayerCanHearPlayersVoice' is a single line, which is necessary because that hook runs very frequently iirc.

@thegrb93
Copy link
Contributor

thegrb93 commented Apr 4, 2024

Also the PlayerCanHearPlayersVoice hook and calcHearablePlayers should only be used if global voice is not enabled, and it needs to not conflict with DarkRP's hook.

@Denneisk
Copy link
Member

Denneisk commented Apr 4, 2024

Note how 'PlayerCanHearPlayersVoice' is a single line, which is necessary because that hook runs very frequently iirc.

It would be better to have everything necessary inlined for performance.

@thegrb93
Copy link
Contributor

thegrb93 commented Apr 5, 2024

You can replace the canHearPlayer call with the line inside that function, sure.

@stepa2 stepa2 marked this pull request as draft April 16, 2024 21:38
@stepa2 stepa2 changed the title Wire Advanced Microphone and Speaker [Will be a separate addon] Wire Advanced Microphone and Speaker Apr 16, 2024
@Hazeofdream
Copy link
Contributor

why are you requesting my review, i aint a wiremod dev my guy

@Denneisk
Copy link
Member

Is it alright to close this if you're planning on separating it to its own addon?

@stepa2
Copy link
Contributor Author

stepa2 commented May 17, 2024

I'll close it

@stepa2 stepa2 closed this May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants