The event-system implementation, Monitoring and you

I hope that I can write more meta-related topics in this forum, otherwise I will copy my question to github.

As all the issues regarding the Monitoring state of events come up, I thought about adding something to those orders. What if an Order actually had a flag if the cancelled state is allowed to be modified or not?

The implementation (In this case “Sponge”) could then check if one of the listeners modified the state, give out a warning in console and set it back to its previous state.

That way we can be 100% sure that plugin developers do not set an event to cancelled by “accident”.

How does that sound? (This topic is meant to be as a pure discussion.)

There were a lot discussions on irc, …
Also on the initial repo commit:

As you can see by the comments then, I have been following these conversations as well :wink:
What I didnt really see though is that people talked about blocking this completely in the implementation.

Like I said, if this seems to be the wrong place, I can continue on github :wink:

I think no …
Sponge-Dev have not the time to read all …
We must give them a short summary, or?

I hope that we can get a MONITOR state with an enforced no-event-modifications policy, so it actually means what it says.

3 Likes

This. Dear god yes this. One of my biggest pet peeves with Bukkit was it’s event priority system and the fact that no one seemed to understand it. That meant that almost everyone wrote their stuff at Priority.MONITOR and then bitched when stuff broke because X dev did their priorities the right way and Y dev just used MONITOR to overwrite anything because they think their plugin is a special snowflake.

2 Likes

I don’t think it will help. Devs who make the mistake of setting an event cancelled when they’re at the “monitor” level could just as easily make the mistake of not putting themselves in the monitor level when that’s where they should be, and then cancel the event from the wrong level. Maybe if “monitor” were the default…? I’m dubious.

The correct priority for your event handler is always a crapshoot - at best you can look at what other plugins have theirs set to (assuming their code is public) and adjust accordingly. I don’t see any good solution - maybe something in a server config file that would allow for adjusting plugin priority on specific events? But that sounds like it might be too technical for your typical server owner to use well.

This outlook is one of the things I hated most about Bukkit - “We won’t add this because it’s too complicated.”

If you don’t understand config/advanced/priority-overrides.conf, then why are you in the advanced folder!?
I think a feature for server admins that know what they’re doing to tweak event priorities of poorly-written plugins could be very useful.

Also, a simple way to implement the MONITOR read-only enforcing would be to pass a cloned event to all handlers registered at that level. Inefficient? Kind of. Easy? Yes. Compatible? Yes.
Ultimately, getting it to not clone and just throw UnsupportedOperationExceptions would be better, but a clone could work at least in the meantime.

1 Like

They’re in that folder because they read on some forum thread that some other guy had used a file in there to fix a similar problem. To make matters worse, they decide to copy/paste his edits and modify them instead of actually reading the documentation, because they’re lazy and just want to get it done. This situation sucks, but it’s reality - the spectrum of server owners runs from technically capable devs and system admins to sloppy 12 year olds who want shortcuts and hand-holding.

Complicated means mistakes and loads of questions. Mistakes hurt servers and the players on them, and loads of questions distract the project owners with help desk stuff when they could be applying their valuable technical skills to fixing bugs and adding wanted features.

Each feature has a “cost of ownership”. Maybe there’s a way to implement that last suggestion of mine in a way that makes it easy for most server owners to use, but I don’t see it. I think the cost would outweigh the value.

A basic ‘on switch’ for those type of features could be not creating the file in the first place.
If you know about priority-overrides, then you can cd config/advanced and touch priority-overrides.conf. Then you can go to editing it with whatever editor, and all the people that don’t know how to use the feature and were just trying to find and edit every single conf file won’t know it exists.

You don’t think the page which documents what goes in the file will tell readers how to generate it? Thanks to the internet, we can’t keep information from anyone - unfortunately, we also can’t force them to understand it before they take action.

If they’re on the page that talks about how to use the file, then they should have less likelihood of frakking everything up.
Or maybe someone should just make a ‘Spongevanced’ fork that adds all these features that have been deemed too complex for the average idiot, and doesn’t offer any support. And saying ‘Spongevanced’ on the official Sponge forums gets you banned. :stuck_out_tongue:

1 Like

Wouldn’t it be good that the event tracks what plugins have “touched” them. That has always been an isue with bukkit. Because plugins just see that the event is canceled but they can’t know why. Lets say:

*Player clicks a shop sign in spawn

  • Worldguard cancels the interact event.
  • ShopPlugin checks if worldguard cancels the event and sets status to allow.

*Player clicks a shop sign in a town he isn’t allowed in

  • Towny cancels the event
  • ShopPlugin doesn’t know about towny and and disallows the shoptrantaction
3 Likes

Wouldn’t that mean that this shop plugin needs to be aware of which protection plugins exist and need to be updated for every new protection plugin being created?
Doesn’t sound that good to me…

Not really, but it gives plugins the opportunity to find out what plugin has canceled what. It also could be helpful for debugging.

1 Like

Why would a plugin need to know about the different kinds of others? If we add a tracker to events, I would use the plugin ID instead - that way you can code in the ID without having to add that plugin as a dependency (like event.getHistory().contains("WorldGuard").

2 Likes

And then you would have to update this whenever a new plugin with the same functionality like WorldGuard is created (or instead you let the server admin provide a list of plugin id’s for which the event shall be ignored).

But then I would prefer something like a ‘cancle reason’ instead.
For example the pluginId wouldn’t help much if WorldGuard for example cancles the event for different reasons and you only want to ignore it for some of those possible reasons (ex. if the interact event is cancelled because of a region protection you want to bypass it/still handle it for your shop plugin, but if it’s cancelled because of some other feature (let’s for example assume the WorldEdit wand would be part of WorldGuard and it cancels the event if the player is currently using this item), then you maybe don’t want to handle it as shop interaction).
More useful would be for WorldGuard to provide some ‘cancle reasons’ like “WGProtection” or “WGWand”, depending on situtation.

However that would mean that all plugins would need to handle the event all the time and each plugin being able to add their own ‘cancle flag’ + reason, because you might want to ignore “WGProtection”, but not if some other plugins would also additionally want to cancle the event as well, for some other reason.
Also plugins might have to be able to add multiple of those ‘cancle flags + reasons’ (ex. player clicking something with the world guard wand in a protected world guard region), because some plugin might ignore one of those reasons but not the other.

So basicly all events would have to handle a List of ‘CancleReasons’, containing each the wanted cancle state + reason.

Not sure if all this would really be worth it…

I think Sponge should implement a very simple API for that:

@EventHandler(runAfter = {"WorldGuard", "Towny"}, runBefore = {"AnotherPlugin"})
2 Likes

One plugin can also have more than one listener (e.g. highest and lowest priority) for different modules of it…

I actually implemented something very similar to this in my Event API (commit) a few days ago, that allows you to “lock” events, events are locked on the PRE_MONITOR and POST_MONITOR priorities, the only real downside of this is that the person who makes a new event class, must apply the locking themselves (look at CancellableEvent as an example), and is not automatically applied.

1 Like