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

The EssentialsXSpawn API provides no means of determining where a player will respawn. #5823

Open
gibsonpil opened this issue Jun 6, 2024 · 2 comments
Labels
type: enhancement Features and feature requests.

Comments

@gibsonpil
Copy link
Contributor

gibsonpil commented Jun 6, 2024

Feature description

I propose that the logic used in EssentialsSpawnPlayerListener is moved to a method called getUserSpawn in the SpawnStorage class. Additionally, a method to access that method should be added to EssentialsSpawn and IEssentialsSpawn, similarly to what was done for getSpawn and setSpawn.

How the feature is useful

As of now, developers who want to work out where EssentialsXSpawn will respawn players are forced to replicate the logic in EssentialsSpawnPlayerListener. Having such a requirement makes EssentialsXSpawn harder to support for plugin developers and could result in compatibility issues should the logic EssentialsXSpawn uses ever changes.

I'm willing to implement this and submit it via pull request should this feature be accepted.

Edits

  • I took a closer look at the Essentials source code and made a few modifications to my proposal. I think that using an Essentials User instead of a Bukkit Player would probably make more sense. Additionally, moving the logic for determining where a player will spawn to SpawnStorage would probably be more sensible than putting it in EssentialsSpawn.
@gibsonpil gibsonpil added the type: enhancement Features and feature requests. label Jun 6, 2024
@pop4959
Copy link
Member

pop4959 commented Jun 7, 2024

I don't necessarily hate the idea, but is there a reason why you don't just listen to the event after Essentials? I haven't taken a deep look, but what about it makes it impossible to support, and also what are you looking to use this for (your specific use case would be helpful to know).

@gibsonpil
Copy link
Contributor Author

I don't necessarily hate the idea, but is there a reason why you don't just listen to the event after Essentials?

That's a good option for some plugins, but it only works if you're trying to infer spawn location after a player respawns.

I haven't taken a deep look, but what about it makes it impossible to support, and also what are you looking to use this for

Fair question. My main issue with the way such a thing would currently need to be done is that you'd have to clone the logic EssentialsX uses. However, since EssentialsX is a constantly updating project, this can change at times.

For instance, with 1.16 and the introduction of Respawn Anchors, an option was added to the configuration to allow EssentialsXSpawn users to toggle respawning at them. Had somebody, hypothetically, taken the approach of cloning EssentialsXSpawn's respawning logic prior to that update, their code would no longer work as intended, and worse still, the issue would be subtle and could potentially go unnoticed. In order to avoid such a thing developers would have to routinely monitor EssentialsSpawnPlayerListener for functional changes. I believe that adds unnecessary burden on developers, especially for something that could be remedied fairly cleanly on EssentialsX's end.

(your specific use case would be helpful to know).

Personally I've hit snags in two personal projects trying to add EssentialsXSpawn integration. One is a fork of Dynmap that I'm trying to add player spawn location markers to, and the other is a server moderation suite I'm writing from scratch that includes a /getspawn command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Features and feature requests.
Projects
None yet
Development

No branches or pull requests

2 participants