ArcanePlugins / Treasury

🏦 A powerful multi-platform library for next-level plugin integrations.
https://hangar.papermc.io/ArcanePlugins/Treasury
Other
57 stars 14 forks source link

Transaction events should not be an optional economy API feature #163

Closed lokka30 closed 2 years ago

lokka30 commented 2 years ago

About

I'm unsure why we chose to make transaction events an optional economy API feature in the first place. They are so easy to implement.

Let's wait until 2.0.0 to remove it, but deprecate this next update.

Additional context: this issue evolved from the conversation in #162.

Current Votes

(Let me know if I'm wrong!)

lokka30 commented 2 years ago

@Jikoo do you think this is a good idea?

MrNemo64 commented 2 years ago

I, Ivan and Nemo support this. (Let me know if I'm wrong!)

I am not against nor do I support this. Since it's already an optional feature I wouldn't touch it but I don't mind if we change it

Jikoo commented 2 years ago

I feel like it's not unreasonable to ask that people call events in their provider, yeah.

In general not a huge fan of optional features included in the main interface, either the methods should be in separate interfaces or they should not exist imo. I know that doesn't apply in the case of calling events, but feels relevant to state.

yannicklamprecht commented 2 years ago

Cancellable events should be called before calling the economy provider impl. Non-Cancellable should use the providers response to call the event with it. I don't see a must in forcing the people implementing the providers to explicitly calling events.

lokka30 commented 2 years ago

Cancellable events should be called before calling the economy provider impl. Non-Cancellable should use the providers response to call the event with it. I don't see a must in forcing the people implementing the providers to explicitly calling events.

This would be ideal, but I don't know of any solid way of achieving it.

MrIvanPlays commented 2 years ago

The economy provider shall call them. Cancellable shall be called before anything else the provider would do and non cancellable are informative and can be called later. If we see a economy provider who doesn't call them we nag. That's it and its not that complicated

lokka30 commented 2 years ago

The economy provider shall call them. Cancellable shall be called before anything else the provider would do and non cancellable are informative and can be called later. If we see a economy provider who doesn't call them we nag. That's it and its not that complicated

This is the most feasible approach I am aware of. +1

yannicklamprecht commented 2 years ago

The economy provider shall call them. Cancellable shall be called before anything else the provider would do and non cancellable are informative and can be called later. If we see a economy provider who doesn't call them we nag. That's it and its not that complicated

So you think there are events that should be called in between?

MrIvanPlays commented 2 years ago

Either before or after handling of the method is what I meant for non cancellable events.

yannicklamprecht commented 2 years ago

Either before or after handling of the method is what I meant for non cancellable events.

Then I see no reason that the provider themselves calling these events.

MrIvanPlays commented 2 years ago

Then how shall we call these events mr big brain?

yannicklamprecht commented 2 years ago

Then how shall we call these events mr big brain?

https://github.com/lokka30/Treasury/pull/164

https://www.tutorialspoint.com/design_pattern/decorator_pattern.htm

lokka30 commented 2 years ago

Will be solved by #199