Treasury Economy Integration
lokka30 opened this issue · 9 comments
Hi there! I totally recommend you check out the Treasury library, it has an economy API which will make it easier for other plugins to use your multi-currency system. 😃
Added as a TODO for the next version. Thanks for the recommendation
There API you are suggesting seems a bit... complicated. There's a few things that I would recommend changing (e.g. making symbol a char, removing unnecessary methods in Accounts, Currencies, etc.)
I may add it in the future, but for the sake of time I can't add it in v1.1.0.
There API you are suggesting seems a bit... complicated. There's a few things that I would recommend changing (e.g. making symbol a char, removing unnecessary methods in Accounts, Currencies, etc.)
I may add it in the future, but for the sake of time I can't add it in v1.1.0.
If you have time, I would greatly appreciate if you could discuss your concerns with us. We have made the symbol a string as it is possible that a language requires a few characters joined together to make it appear as a single system (lenny face, for example 😄). I guess we could add an overload for accepting a char but it should (in my current opinion) remain a string.
I've thought it over, and have found a workaround:
I should be able to create a wrapper class for the Economies and convert it that way, so I'll probably add support for the next version.
I've thought it over, and have found a workaround:
I should be able to create a wrapper class for the Economies and convert it that way, so I'll probably add support for the next version.
Spot on, a great way to go about this. I assume most developers are going the wrapper route too.
I would have developed a shaded wrapper for economy provider developers to use, but I figured that everyone does things differently, each creating their own wrapper is probably a better choice.
Pushed & Tested, if you have any suggestions take a look at the hook directory
Your Wiki was a bit vague on what to specifically register for the economy API, so I registered the EconomyProvider and checked for an existing one, if I'm supposed to register something else let me know.
Awesome! Here are some issues I found upon a (not so detailed) scroll over the implementation:
- Implementation does not support non-player accounts and treats such as player accounts
- This must not be null
- Checking for existing registrations is unnecessary as Treasury handles this using service priorities. It also shouldn't unregister any other economy providers
- Another null value returned for a non-null method
- Subscription is never completed
- States that transaction events are supported, but they are never fired
The Wiki definitely has a long way to go. Unfortunately there has been a standstill in user contributions since I have halted any activity on the Spigot forum about the library.
We will be improving the EconomyProvider interface in ArcanePlugins/Treasury#167 and ArcanePlugins/Treasury#180 respectively.
Awesome! Here are some issues I found upon a (not so detailed) scroll over the implementation:
- Implementation does not support non-player accounts and treats such as player accounts
I don't see a reason why this should be a feature, and I don't plan on making it one, but if you would rather create a dummy account for non-players then that should be fine.
- This must not be null
Fixed
- Checking for existing registrations is unnecessary as Treasury handles this using service priorities. It also shouldn't unregister any other economy providers
If I already have my EconomyProvider, will it automatically remove that registration? I use that method for when the plugin reloads/restarts just to update the method.
Also Fixed
Isn't it? The subscription is being passed to another method where it does complete.
I guess I misunderstood what you meant, the API calls that happen do fire my own events, but if that's not what you actually mean then I can remove it.
The Wiki definitely has a long way to go. Unfortunately there has been a standstill in user contributions since I have halted any activity on the Spigot forum about the library.
We will be improving the EconomyProvider interface in lokka30/Treasury#167 and lokka30/Treasury#180 respectively.
Glad to hear it! Thanks!
I don't see a reason why this should be a feature, and I don't plan on making it one, but if you would rather create a dummy account for non-players then that should be fine.
It's one of the keystone features in the API, allowing some plugins such as Towny and GriefDefender to operate properly. If you choose to ignore it, I advise that you state that feature(s) XYZ of Treasury are not available.
If I already have my EconomyProvider, will it automatically remove that registration? I use that method for when the plugin reloads/restarts just to update the method.
I recommend only registering the service once - I don't see a point in reloading it :)
Isn't it? The subscription is being passed to another method where it does complete.
Apologies!... brain fart.
I guess I misunderstood what you meant, the API calls that happen do fire my own events, but if that's not what you actually mean then I can remove it.
They'll need to fire Treasury's economy transaction events as well, if you wish to support transaction events (which we will be making "mandatory" (expected for all implementations) in v2).
Feel free to make any changes if wanted, let me know when you want me to take another look :)