Getting event call cascade?

Sponge Sponge Development
1

So, @Stefan came up with the following point:

Obviously both events have their meaning and it would be silly to not call one or the other. So I cause / trigger field to events could help in that kind of situation. It would be a bit like a stacktrace and allow you to see the event call cascade.

So, event would have the following additional field (I called it cause…):

public abstract class Event {

    protected Event cause;

    public Event getCauseEvent();

}

Then, on your PlayerDisconnectEvent instance you could call

playerDisconnectEvent.getCauseEvent();

and would get the PlayerKickEvent that caused it (or null if there was no cause).

To come back to the problem at the beginning of the thread, you could simply check

if(!(event instanceof PlayerKickEvent)) {
    event.player.ripIntoTinyLittlePiecesAndDropItemsOnFloor();
}

Any thoughts on it?

2

I think using an enum in the event (PlayerDisconnectEvent) would be a much better idea, you already know what event type you are handling, and having a cause inside an event seems quite unnecessary to me.

Your idea: https://gist.github.com/N3xuzDK/038fcb3d6b28f1046438
Me idea: https://gist.github.com/N3xuzDK/e8de7e1093006fdbb6e5

3

I disagree. In Bukkit, PlayerQuitEvent and PlayerKickEvent were actually completely different - and were called at different points in the pipeline. It confused me too for a while, admittedly.

The PlayerKickEvent was an event that happened BEFORE the user got kicked. This was cancellable, so if a player was about to be kicked, you could stop that from happening. This event was server-driven - at this point, the client actually had not been kicked yet. User cleanup should not have been performed here, as there was no guarantee that the user was going to be kicked.

The PlayerQuitEvent, on the other hand, was an event that happened AFTER the user got disconnected, which include being kicked. You can’t stop a user from disconnecting - indeed, the user had gone by this point. This is the logical place to clean up any user resources.

Essentially, the two events don’t serve to confuse, they are two different events for two different point when kicking a player. There is no reason to have an equivalent method to the PlayerKickEvent for when a client disconnects of it’s own accord - because you aren’t going to do anything with it that you would do during disconnect, you sure can’t stop them from disconnecting. Therefore, there would be little point to having an umbrella PlayerDisconnectEvent for this - it feels to me you’re sugggesting to make another event object for the sake of it.

The PlayerQuitEvent or whatever the equivalent is in Sponge could have a “disconnect reason” enum or something in the event itself that states whether the player left of their own accord or was kicked. That’d be a better way around it, because then you just had to monitor the disconnect method for the reason, and ignore the kick event, if any.

(I did work on that for Bukkit at one point, but it sat there for 4/5 months (with some interaction), got bitrotted, and after rebasing it multiple times for multiple versions, I just gave up - funnily enough, it was to solve this problem…)

1 Like
4

No, I am not suggesting a new event object, I’m suggesting that every event can have an optional cause. This is just a specific example. So, the disconnect event could have the cause that the player was kicked.

That seems way too static if you’re saying what I think you’re saying.

5

I’m fairly sure I’m saying what you think I’m saying, and say it’s very static, but I doubt you are really ever going to add a new event for when a player gets disconnected from a server.

Also read my previous post, I updated it with some examples.

6

You shouldn’t be using an event within an event though as you suggest with the getCauseEvent() method. Once the event has passed, the object should expire - particually if it’s cancellable - you should only be able to react to an event, you should never store it.

It might serve to confuse some as well - they might get the PlayerKickEvent out of the PlayerDisconnectEvent and try to cancel it, or change the kick reason then - and it wouldn’t work for reasons that might be obvious to us, but not to them. @N3xuz_DK has the right idea, though you could always make it a more concrete object if you really wanted to with more information - as long as it’s clear that the object does not affect the current event.

I personally would have thought that an enum was fine though - if you really need it, you can get all the information that you need by suscribing to the kick event and getting the information as it is being processed…

7

Makes sense, but, as the enum is very static, there is no way of having a custom cause for the player disconnecting.

I was thinking about Throwables while writing this…

8

They are used for exceptions, and a user disconnecting is not an exceptional condition. They are also slow if you use them withing a try/catch block - getting the stack trace can be particually slow.

Just re-reading your original post - that’s in fact what you are asking for - essentially a stack trace. It might be better if the player kick method accepted a cause as a parameter. However, I’m unsure as to what should be passed in there - an enum is good for machines, a string is good for flexibility. You could perhaps have a Cause object that contains the name/class of the plugin, along with a string for the reason. This could then be exposed in the PlayerKickEvent. I don’t think the later event necessarily would need to know about it though - if you needed to see if a player was kicked for a particular reason, subscribe to the kick event last (whatever the priority would be), when the event can/will no longer be cancelled/altered.

I don’t know if that’s entirely necessary though. In the example you gave (combat logging), you’d only really need to know whether the player was kicked by the server or whether he quit by himself, and that enum does that well enough.

If you can give good examples as to why you might need more information - then I’d glady accept that getting the cause, and possibly the plugin that causes the kick, would be useful. I’m struggling to see why though…

9

I only meant a similar system. Not using throwables xD I’m not supid LOL