stujones11 / minetest-3d_armor

Visible player armor & wielded items for minetest
Other
56 stars 98 forks source link

[shields] sending sound effect to all players #139

Closed aerozoic closed 6 years ago

aerozoic commented 6 years ago

The on_damage function of shields is sending the sound effect to ALL players on the server. P.S. I'm using multiskin branch. :large_orange_diamond:

stujones11 commented 6 years ago

Looking at the code, the sound should only be heard up to 10 nodes away. The only other thing that can cause this issue is if the audio file is not monophonic which seems unlikely since these are simply the default metal/wood dig sounds from MTG. If those are working okay then I see no reason for them to behave any differently here.

I will check this myself as soon as get a chance, however, it will require setting up a local server and logging in with two or more clients so I can't say when that will be. In the meantime you can look at the master branch of shields which does have an option to disable the sound effects. AFAIK the master shields mod should be interchangeable with the multiskin branch but the code changes are very small anyway.

aerozoic commented 6 years ago

Thanks! I never even looked at the master branch since i use multiskin. I will get that update immediately. :large_orange_diamond:

stujones11 commented 6 years ago

You are welcome, could you please confirm that you are using the default MTG sounds and that general metal and wood 'digging' sounds do not exhibit this behaviour? It would be very helpful.

aerozoic commented 6 years ago

Yes i'm using default MTG 4.16 stable. I only ever hear glass walking sound and metal dig sound. Server and client are 4.16 stable. Here is the repo for my server if you wish to investigate: https://github.com/UGXaero/UGXrealms :large_orange_diamond:

stujones11 commented 6 years ago

Thanks for the info, I will try to look into this over the weekend. I have already verified that the default sounds are all mono as I expected so I will need to rig up some kind of test environment. However, I might just end up nuking this 'feature' completely as it seems to be more trouble than it's worth :)

stujones11 commented 6 years ago

I have pushed what I believe to be a fix for this cc6fff2b040849aae33019721d6cf9cd6fa372cb and have cherry-picked that into the multiskin branch. I would appreciate if you could confirm that this fixes the issue for you.

aerozoic commented 6 years ago

Confirmed this fixes the issue. I'd still like to have that option to disable it though. :large_orange_diamond:

stujones11 commented 6 years ago

thank you for the confirmation, I have now backported both 4baed2ca2245879bdbc925eeed34134ff7832d79 and c812e0ac5682cd06eaa4869837325cbb0e46247a to the multiskin branch.

aerozoic commented 6 years ago

Thanks! :large_orange_diamond: