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

Add ability to play sounds and send particles via the API #317

Closed
Yeregorix opened this issue Jun 3, 2020 · 24 comments
Closed

Add ability to play sounds and send particles via the API #317

Yeregorix opened this issue Jun 3, 2020 · 24 comments
Labels
type: feature New feature or request

Comments

@Yeregorix
Copy link
Contributor

As the title say, I suggest to add ways to play sounds and to send particles to a player using the API. In my case I want to write a note block music plugin. Why can't I write my plugin at the server level ? Because I want the music the keep playing when the player changes server. Anyway, these functionalities could be also useful for a lot of other applications until then impossible at the proxy level.
Sending sounds or particles almost always requires a position as parameter. And this position is often relative the player position. So I also suggest to add a way to get the player position. This position would be deduced by Velocity by tracking position packets. I don't know how heavy it would be to track these packets so if it's a potentially heavy task it could be interesting to make it optional and controlled by a config file or by plugins using the API, or both.

@hugmanrique
Copy link
Contributor

hugmanrique commented Jun 3, 2020

Velocity is a proxy, not a full-blown server implementation. Player position update is one of the most sent packets, and decoding it would incur a significant performance loss. Consider synchronizing the noteblock music through a database or pubsub service on server switch.

@Yeregorix
Copy link
Contributor Author

Yeregorix commented Jun 3, 2020

There is a difference between a full-blown server implementation and the ability to play sounds and to send particles. I'm talking about a functionality that can be used in various situations, eg. sound on chat ping, sound on join, etc. These are interesting functionalities for a proxy. Also it is possible to play sound globally in Minecraft. This could at least be a thing in the API. Tracking the position is another problem, I keep thinking it could be a very interesting possibility and I'm aware that the position packets are some of the most sent packets but there are also ones of the smallest and easiest to decode individually. The main difficulty is to handle all types of position packets because they are numerous. Also I suggested a way to enable or disable this tracking, if done through the API and per player this would seriously reduce the performance cost.

@mdcfe
Copy link

mdcfe commented Jun 3, 2020

Also I suggested a way to enable or disable this tracking, if done through the API and per player this would seriously reduce the performance cost.

Having a config option that drastically reduces performance for the benefit of a few plugins doesn't seem ideal (especially when these plugins could easily just be implemented on the backend), and enabling tracking through an API is even worse since administrators might not even be able to pin down which plugin enables it. Position tracking on the proxy is a massive burden and is pointless when the backend already does this, and as @hugmanrique already said, you can definitely achieve this with a plugin on the backend server communicating with other servers on server switch.

@Yeregorix
Copy link
Contributor Author

Yeregorix commented Jun 3, 2020

I know this is possible on the backend but this was the opportunity to publish cool plugins directly on the proxy so it doesn't need to be added to each backend servers. Doing it on the backend is what I planned originally and what I will do if my suggestion is rejected. If I suggested it here, it is to add useful functionalities for other plugin devs and servers owners.
I totally understand the fear of the performance impact but I think it's slightly premature to say that the impact will be significant without testing. Anyway, this suggestion is about playing sounds and particles effects. Tracking player position is another problem which is often but not always required to play these effects. Plugins might use a constant predefined position or could use their own messaging system to choose the position by contacting backend servers.

@hugmanrique
Copy link
Contributor

Plugins might use a constant predefined position or could use their own messaging system to choose the position by contacting backend servers.

You proved our point: why would the proxy contact the backend server to get a position to send a particle effect instead of, you know, the backend server sending it directly.
All your previous examples can also be implemented (imo) more easily on the backend server.

@Yeregorix
Copy link
Contributor Author

Yeregorix commented Jun 3, 2020

When an algorithm comes down to one method call yes it can obviously be done on the backend, but it's rarely the case, otherwise tell me why a plugin would call sendMessage on proxy when it could be done on the backend. If you really consider that something that can be done on the backend should never be done on the proxy then let's remove sendMessage, sendTitle, command API, etc.
Velocity is not just a proxy, it is also an API that allow building plugins which are not limited to only move player from a server to another.

@wiim00
Copy link

wiim00 commented Jun 3, 2020

Why remove those methods and the Command API?
There are just basic things that can be better implemented at the proxy level.
Velocity is not a Minecraft server. It is a proxy, and I think it should stay that way.
It should provide the highest possible performance without any unnecessary features that not everyone uses.

@astei astei added the type: feature New feature or request label Jun 3, 2020
@astei
Copy link
Contributor

astei commented Jun 3, 2020

I feel like some of the comments here are missing the point. Performance is a very valid and strong concern for me, but the sound and particle APIs directly wouldn't cause performance problems. (The player position packets would, though - they're sent frequently and would increase the amount of garbage allocated by the proxy unless we started pooling packets. This would be a major internal change and I'm not interested in it. However, at the moment, packet decoding is fairly cheap compared to the much more expensive costs of compression and encryption.)

Regarding the position API, all the attempts to add it to BungeeCord were utterly unsuccessful. I especially invite you to look at SpigotMC/BungeeCord#2645, the most recent attempt.

My problem with this is that I am concerned about how the new APIs will be maintained. Velocity 1.1.0 supports Minecraft 1.7.2 to Minecraft 1.15.2. That's an incredibly wide range, and each version introduces (or removes) sounds and particles. Introducing a sound and particle API into Velocity would add additional work for us when updating to a newer version of Minecraft, and it is not clear to me how much value Velocity users will get out of the additional work we will have to invest in order to implement a position, particle and sound API. (And if we add these APIs, it opens the door for us to implement what is essentially a full-service client manipulation API.)

tl;dr: I am not convinced Velocity should provide these APIs. The onus is on @Yeregorix to prove this will provide value for Velocity users.

@Yeregorix
Copy link
Contributor Author

I just took a look at the current protocol for sounds and particles and indeed I agree that keeping the ability to spawn particles for all versions is hazardous since particles ids are int based and those ints are magic values that can easily changes. However sounds ids are string based and will probably stay string based for ever as it is necessary to allow custom sounds using resource packs. So having an API method with a string parameter should be enough, the client will just ignore the sound if the id is not recognized.

About proving the usefulness of such features, of course I can't give plugins without any API 😅 but I can give examples. Here are a few ones:

  • Playing sounds when a player join the proxy.
  • Playing sounds when you get a friend invitation.
  • Playing sounds when someone mention your name in chat.
  • Playing sounds when someone is banned.
  • Playing sounds when you receive a private message.
  • Playing sounds when an admin is broadcasting an important message.

And so on, I can keep going if you want. Note that you can replace "Playing sounds" with "Spawning particles".
Sounds and particles are "cosmetics" effects that are commonly added in many situations. Of course for plugins some functionalities are implemented by server plugins and the sounds/particles are sent from here. But there are a lot of functionalities that are used to be implemented by proxy plugins because it's easier than using server plugins: friends, chat mentions, bans, private messages, etc. In a such situation it would be interesting to be able to directly send sounds/particles from the proxy instead of contacting a backend server to do it.

@Mystiflow
Copy link

What if we can create events for certain packets and only register the packets if a handler list is utilised by at least 1 plugin and if necessary track really intensive usages and log a warning in console?

@hugmanrique
Copy link
Contributor

The main goal of this issue is to add methods to send sounds and particles, not being able to track them. I can see the former being accepted as an API, but I can’t think of any uses a ParticleSendEvent or SoundSendEvent would have. Mind sharing some use cases? Maybe I misunderstood the events.

@MrIvanPlays
Copy link
Contributor

MrIvanPlays commented Jun 6, 2020

Player positioning is not big deal. Yes, client sends constantly packets, but if you're running the proxy with the proper java GC flags, no memory leaks or much additional memory would occur. As tux mentioned, decoding packets is times cheaper compared to doing other stuff. Sounds and particles are fairly cheap. There won't be a very big performance hit.

About BungeeCord, just please... don't mention... when someone wants to do whatever change either md ignores it or replies after a month you've opened it stating "overall no change in performance occurrs, except you making the proxy do more of (insert random shit here) which I don't like"

@Gabik21
Copy link
Contributor

Gabik21 commented Jun 6, 2020

From my point of view it seems that there is only a very small portion of users that need this particular feature. If you implement features like friends on the proxy and you want sounds, you are likely best off using a message broker that can notify a backend server to play a sound.

I also think that performance is a valid concern here and I see the users wanting that API in the position where they need to implement it and provide proof that on a server with an adequate scale there is no notable performance degradation. Also a higher object allocation rate also means more CPU time spent on garbage collection, that kind of timings also need to be included in a thorough before and after performance analysis.

@astei
Copy link
Contributor

astei commented Jun 17, 2020

I've been hearing word of a proposal that may make position tracking unviable on the proxy level, as movement will become predictive and be double-checked by the server (similar to modern FPS games). That means this API may not even be viable in the future unless we port similar logic to Velocity, which I'm not keen on (it introduces more version-specific logic I'm not keen on adding).

Having heard of this proposal, it makes the case for adding the ability for the proxy to play sounds and send particles rather weak, if it's not going to work past 1.16. You are of course free to attempt to implement this in your plugins in accordance with the versioning policy.

I'm torn on whether or not to close this issue. There seems little appetite for this feature, and my concerns about how the API will be maintained remain a sticking point. That being said, PRs are welcome if anyone is willing to undertake the work and commit to actively maintaining the relevant APIs and proxy bringup required.

@Yeregorix
Copy link
Contributor Author

The server does double-check the position but when the server decides that the client position is incorrect the server sends a packet to force sync it. There is no need to implement double-checks at proxy level, It only needs to track packets in both direction.

I originally proposed this addition to make so that everyone benefits but It seems that I'm almost the only one interested so I will try things on my side when I get the time. I'm very busy this summer so It will not be done any soon, you can close this issue. I will maybe come back someday if I have something I'm happy of.

@astei
Copy link
Contributor

astei commented Jun 23, 2020

Very well then, closing this issue due to a general lack of interest.

@astei astei closed this as completed Jun 23, 2020
@floge07
Copy link

floge07 commented Aug 25, 2023

Just started using Velocity 2 days ago...
Was testing around and saw that the Player had a playSound method. What a fake out xD

I get now that it's just part of the adventure api (which I also didn't know before) of the Audience class and not every platform may support a feature.

@4drian3d
Copy link
Contributor

It would be interesting to implement a SoundService API (using the Adventure Services) that could be implemented by a plugin to "inject" this functionality in the playSound methods by means of packets or plugin messages with connection to the backend server

@FatSaw
Copy link

FatSaw commented May 1, 2024

It would be nice if it were possible to at least start and stop the sound. I personally don’t need checks like “whether this sound exists on this version of the client”.

@Timongcraft
Copy link
Contributor

It would be nice if it were possible to at least start and stop the sound. I personally don’t need checks like “whether this sound exists on this version of the client”.

If you've read the above messages, you know that you also need coordinates, which are updated relatively, or an entity id to play a sound. This would entail intercepting more packets, etc.

@Gerrygames
Copy link
Contributor

The client player id is transmitted via the join game packet which is already decoded and handled by velocity. It could be used to play a sound at the client players position. This would only work for modern versions tho.

@FatSaw
Copy link

FatSaw commented May 5, 2024

If you've read the above messages, you know that you also need coordinates, which are updated relatively, or an entity id to play a sound. This would entail intercepting more packets, etc.

I can't send "playsound" even if I have the coordinates or if the position is not important at all (if high volume values are used). If it will not implemented fully, then at least with an explicit indication of the coordinates. Something is better than nothing.

@Timongcraft
Copy link
Contributor

Since there were a few people in the past here and on Discord who were interested in this, could this be reopened?
(Or closed as not planned instead of completed.)

@Timongcraft
Copy link
Contributor

For those still interested a pr for sounds is open here: #1422

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New feature or request
Projects
None yet
Development

No branches or pull requests