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 Server-Sent Events #16

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add Server-Sent Events #16

wants to merge 8 commits into from

Conversation

MrToirol
Copy link
Contributor

This PR adds a /events endpoint to the API that users can use as an EventSource. This endpoint relays and broadcasts multiple Towny events in real time, using server-sent events (SSE) through Javalin. The complete list and data structure can be found in the docs.

Comment on lines +35 to +39
CompletableFuture.allOf(
clients.stream()
.map(client -> CompletableFuture.runAsync(() -> client.sendEvent(event, message)))
.toArray(CompletableFuture[]::new)
);
Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified, the result of the futures is ignored so combining them into one is redundant.

(requires adding a plugin field obviously)

Suggested change
CompletableFuture.allOf(
clients.stream()
.map(client -> CompletableFuture.runAsync(() -> client.sendEvent(event, message)))
.toArray(CompletableFuture[]::new)
);
for (final SseClient client : clients) {
plugin.getServer().getAsyncScheduler().runNow(plugin, task -> client.sendEvent(event, message));
}

import net.earthmc.emcapi.manager.SSEManager;


public class PlayerConnectionListener implements Listener {
Copy link
Member

Choose a reason for hiding this comment

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

I think that player join/quit events are too intrusive, this class should be removed

Choose a reason for hiding this comment

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

I shadow Warriorrrr's opinion, those events are intrusive though, it can prove helpful to have it...

@Owen3H
Copy link
Contributor

Owen3H commented Jan 25, 2025

Imo, the event names could cause some confusion, for example "DeleteNation" sounds like the event deletes the nation.
Instead I think they could be a bit more consistent and convey what happened a bit better.

RenameTown -> TownRenamed
DeleteTown -> TownDeleted
NewTown -> TownCreated

RenameNation -> NationRenamed
DeleteNation -> NationDeleted
NewNation -> NationCreated
NationRemoveTown -> NationTownRemoved or NationRemovedTown
NationAddTown -> NationTownAdded or NationAddedTown

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.

4 participants