Speaker Peripheral issues.
Wojbie opened this issue ยท 14 comments
Speaker Peripheral has 2 issues that i have found. Both are related to playing longer sounds.
- When playing a long sound and breaking Speaker sound will keep playing. This is caused by song been played at position using /playsound command.
Expected behavior would be for sound to stop playing when Speaker is broken.
- This also happens with Noisy pockets - sound plays in spot player was when pocket ran a command. This lets people to play long music disk sounds and leave area filling one spot with cacophony of music.
Expected behavior would be for sounds to follow player holding pocket and stop when pocket is turned off/dropped.
I would suggest rewrite of the speaker so that sound follows player and some way to stop sound that is playing (as suggested in #251) to be added. This would allow for Speaker/Noisy pocket to stop playing sound as they are broken/leave player inventory.
This would sadly require moving away from simply playsound on server thread and into custom music playing setup with packets and stuff i believe.
EDIT: In meantime i would suggest removing playSound from peripheral and limiting it to playNote for now. This removes most of stated concerns due to fact that note sounds are all short.
@Wojbie hmm... That would be interesting
Assuming it's too tricky to detect what the source speaker's position / state is, a workaround may be to limit the playtime of all sounds to something like 5s max. Being able to play whole songs with one command seems a bit off to me anyway, and it certainly makes things easy for griefers.
Any reason why we need a new block for this peripheral, by the way? Wouldn't it've been suitable to just use regular Note Blocks?
Another thing, had to guess at the expected value ranges for pitch / volume through trial and error and tips from Wojbie - they aren't documented. The peripherals don't error when handed out-of-range values, either.
@Wojbie: they're played using world.playSound from Java, not using commands. I am not sure of any way to get some reference to the sound offhand (bear in mind I'm not that good at forge), but I will do some reading.
@BombBloke: at the moment, speakers are set up to play exactly the resource locator you give it. They could be whitelisted. I just submitted the PR and waited for feedback on that among others. I would be happy to submit a fix PR to whitelist, etc, or someone else could do it, but we should discuss this more.
About the pitch and volume: yeah, I/someone else should document those. Essentially, it is the ordinal of the note in the 2 full notebook octaves (including semitones), i.e C is 1, C# is 2, D is 3, and 12 is the next C. The values are (probably by minecraft code) computed mod 24.
I did some research when this issue was first created, but never got round to implementing anything so I thought I'd type it up here:
There isn't really a feasible way to handle a speaker being destroyed or moved with current implementation. Instead, we'd have to implement our own ITickableSound
and create our own playing packets. In order to handle speakers moving (in pockets/turtles) or being destroyed (or detached when in pockets/turtles) I think we'd have to create a "speaker registry". This would keep track of each speaker and where they are, as well as associating sounds with a given speaker. This way when a speaker is moved/destroyed we know what sounds to update.
This does seem a little overkill though just for playing sounds, so I'd really appreciate other people's solutions :).
I know vanilla has a stop sound command, is there way we could run something similar to that? I don't believe it provides any filtering, but for things like music discs its probably the only one playing.
@KnightMiner I don't really see "probably" as good enough though :p. You're effectively giving computers the ability to stop any sound on the server, even in a limited form.
Can the same functionality be automatically targetted at specific sounds, though? Doing that eg 5s after each sound begins would pretty much solve the problem, in my book.
But it'd need to be at specific sounds - outright squelching everything as the stop "command" does is an obvious no-go.
It can. ISound has a ISound#stop. We can keep track of which ISounds are playing on which speakers, and perhaps send a packet to the client when a speaker is destroyed, telling the client to stop the sound.
As @SquidDev mentioned, this does sound like overkill.
If the registry allows for sounds following speakers (and stopping if the speakers are destroyed), then I don't see much point in the 5s thing. That was just an alternate suggestion I put forward as one who hasn't actually looked at any of the underlying code - I didn't think there'd be a need to track sounds if there happened to be a way to truncate them before initial playback.
There may be a way to stop them after 5s - but as I see it there may not be a way to do that without stopping all sounds that have been playing after 5s. If anyone knows another way of doing this, point it out if you could :)
Just revisiting this: what are your guys' thoughts on the sound registry and custom packets? @BombBloke we can look at stopping sounds after 5s, but if we implement that we'd need some sort of registry anyway, so we may as well do a more complete solution (which would be a bit nicer imo) such as stopping a sound only when a speaker is broken. Thoughts?
@DigDugFTW to continue the thread from #439, /playsound clear
is indiscriminate (iirc) in what it stops playing -- we'd need to keep track of every sound we play server side (and client side, I think) via some sort of registry, and then send a packet to stop the sound.
So. I posted it in wrong issue thread. Copying here to make sense.
Ok radical idea but here it is. How about having tile itself store sound info it's playing and limiting it to one sound at time when using playsound so whenyou try to play new sound it stops last
We could skip that with playnote and allow multiple sounds due to predictive shortness of notes?