Spongeapi: Events with only one transaction

Created on 12 May 2017  路  11Comments  路  Source: SpongePowered/SpongeAPI

Implement some default methods for events, that usually only have one transaction.
In ClickInventoryEvent we need to write this:
event.getTransactions().get(0).getSlot().getProperty(SlotIndex.class, "slotindex").get().getValue();
to get clicked slot. It will be really good, to write something like this:

@Listener
public void onClick(ClickInventoryEvent event, @Getter SlotIndex index) {
}

If i understood right.

event inventory

Most helpful comment

I agree that getting the first transaction without checking is bad practice.
But an eventfilter could do all the checking for you!
So if you only care for events that affect a single slot you don't have to:

if (event.getTransactions.size() == 1) {
    SlotTransaction transaction = event.getTransactions().iterator().next():
    // do stuff...
}

Instead just add a @SingleTransaction SlotTransaction transaction to the method signature. (name is not final^^)

All 11 comments

I disapprove, event.getTransactions().get(0) is a bad practice. To ensure that all transactions are treated you should make a big loop at the root of the method, it's like handling many events of one transaction. I don't see the difference and I don't see why we should add such shortcut methods. These methods would encourage this bad practice.

I agree that getting the first transaction without checking is bad practice.
But an eventfilter could do all the checking for you!
So if you only care for events that affect a single slot you don't have to:

if (event.getTransactions.size() == 1) {
    SlotTransaction transaction = event.getTransactions().iterator().next():
    // do stuff...
}

Instead just add a @SingleTransaction SlotTransaction transaction to the method signature. (name is not final^^)

Some news?

@Deamon5550 I think I have an idea on how to implement this with a @SingleTransaction annotation as the method argument. Something that can be fast byte code wise. I'll give this a shot at some point.

I disagree on this change.

The point of events HAVING transactions is understanding that the changes happening might be multitudes at a time. Whereas it seems easy to just have the concept of getting a single transaction, how does this benefit you? How are you better off having the transaction at element 0 than element 3? It doesn't and can lead to plugins not functioning 100% correctly because they are not using the API as intended.

@Zidane
This is not for getting one transaction when there are multiple but instead filtering events that have only one transaction so you do not have to check it yourself.
Most Events only have a single transaction and its a pain to check and unpack it every time even if you do not care about events with multiple transactions.

I don't understand under what circumstances you only care about 1 transaction?

The only situation I see, is for click gui?

either you want to cancel it, because it was an action with multiple slot changes by default, or you want to get the slot someone clicked.

click gui

exactly

@Faithcaio

This is purely a plugin issue then. The API contract says there will be transactions and gives you no impression that there could simply just be one. I'm all for sugaring but this filter encourages developers to be lazy and not think about how they are handling the event. Remember that this isn't just for SpongeCommon, an implementation may very well fire an event with multiple transactions for a click and your plugin, on that implementation, will likely appear dysfunctional not due to that implementation's fault but due to your plugin ignoring the specification of the API.

Well then we should provide a way for plugins to register new event filters.

@Faithcaio

Now that is something I can get behind as should you do so in your own projects, you take responsibility for that change. Create a new issue for that aspect alone :).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mbax picture mbax  路  3Comments

instalab picture instalab  路  3Comments

Favorlock picture Favorlock  路  6Comments

ryantheleach picture ryantheleach  路  5Comments

lesbleu picture lesbleu  路  4Comments