Vault

Vault

7M Downloads

Implement new interface method - transfer(from, to, amount, description)

jkobus opened this issue ยท 7 comments

commented

Right now its not possible to monitor where the money came from.
For example chestshop transactions - there is no way to bind withdraw with deposit that occurs during one transaction. I dont even know what the user is paying or paid for.
I would have to monitor chestshop events and somehow get this information in my economy provider class during transaction.

Tell me if you're positive about this proposal.

commented

This is outside the scope of Vault.

commented

How such method can be outside the scope of a thing called "economy provider" ?
Money usually flows between two parties not in/out the air so its very wise to create a method that will allow this. The scope should be: withdraw, deposit, transfer. A real life example of such provider is a bank. You can pay someone by withdrawing funds and then paying him in cash, (and he can deposit the money on his bank account) but who does that ?

After few transactions you wouldn't even know what you've paid for (that's the problem I have with vault). Provider would still do what it does the best (SRP) - control where and how the money goes. That should be his responsibility.

commented

this is something that would need to be implemented directly at the economy level, please see Craftconomy for an example of how to monitor transactions. Vault only provides methods that are universally available in economies, because transfer method is not in them it can't provide the method.

Additionally, Vault can't provide new methods on a whim like this as it would be a breaking change. Either no plugins would work as they wouldn't use the new transfer method, or I'd have to remove/deprecate a bunch of methods to force plugins to update. This is also a bad solution as it means servers can't just update to the latest version for compatibility/fixes.

I thought I'd mention that your anaology is very inaccurate. Vault is not the 'bank.' It's a contract, you can't change the contract unless all parties agree to it. In this case changing Vault (contract) would require getting all Plugins that implement it to change how they work with the contract (Vault) and most likely getting additional changes into economies, or simulating them.

commented

Vault is a virtual bank at its bare minimum. About Craftconomy - In my plugin I don't have problem with binding transactions made through my plugin together. I have problem with plugins sending money using _your interface_ as they wont tell me where the money came from. Let me explain it more clearly.

I'm using database as a transaction storage. With chestshop, that uses Vault, it works like that:

Theory
A -> sends money to-> B - I should have 2 rows in database for each player made in one transaction

The problem
A -> sends money to -> BANK - I have 2 rows - one for player one for bank
BANK -> sends money to -> B - I have 2 rows, one for bank and one for player

Total: 4 rows of data, 2 transactions instead of 2 rows of data and 1 transaction.
Also, there is no way to tell A that B bought something. I cannot connect the two.

The "contract" aspect, or backward compatibility as I call it, is a different issue. You still did not answer me why such method is out of scope of your plugin. "Method is not universally available in economies" ? Every single economy plugin has a feature called "pay money to other player". There is absolutely no logical explanation for the lack of this method.

Yes, adding it would cause problems as it would break the api. Yet still It is possible to implement this the way old plugins will work and new plugins will be able to _evolve_.

commented

@jkobus - actually economies generally just implement deposit and withdraw. Secondarily Vault is in no way a Bank, in fact a good number of the Economies have Bank support. Vault does not handle/store transactions ever, it simply provides an API contract for other plugins to hook into. If you see ChestShop shoving transactions into a bank that's not Vault doing it. That's ChestShop.

As I already stated it's outside the scope due to the only 2 ways of implementation causing discrepancies no matter what with legacy hooks. It will continue to cause problems if a change is attempted at this point. If you want something like this you're going to have to log it at the plugin level not at Vault's level.

commented

You misunderstood what I said; Let me clarify, because we miss the point here.

I know vault wont do anything on its own and its obvious.
It works in connection with economy backend - lets call it "provider"
Vault contains methods that mimics real life "bank" thats why I call it like that. It has actions like deposit, withdraw, has account, etc.

Chestshop was given as a general, abstract example. That was a theory of operation showing what would need to be done in two different use cases where one used my approach.

Now:
I wrote a "provider" but there is no way to get information about who paid what because your interface does not allow that. Every plugin that transfers money from John to Jack has to withdraw money and then deposit it. It calls economy twice instead of once. This logic is wrong as the functionality is very limited.

"actually economies generally just implement deposit and withdraw" - thats not an argument; yet still you can pay money to someone else, and they are using both methods because of your interface. You force them to do it like that . Or even better - they did it wrong and you copied it. I really don't care where this idea came from, its still wrong. "deposit" and "withdraw" is a player wallet. Not a full scaled economy.

Bukkit plugins are mostly in opposite of how real software looks like. But we are to far to go back and straighten things up, so lets stay where we are now and don't fix it.

Thanks.

commented

@jkobus - Let me start by saying the interface is not 'wrong.' It's simply how economies grew over the last 4 years. Just because it's not how you envision it to work doesn't invalidate that's how everything has been done in the past.
Perhaps it's not the best way to handle it, or it doesn't allow you to do what you want. At this point it's moot. Vault was developed as a way to interact indirectly with any economy, if you want to handle money in a completely different way than 99% of plugins then you should probably be developing an edge-solution for your own server instead of coming on here and saying how Vault is doing everything wrong.

Or if this is a priority for you, you could provide pseudocode/example code/ simple implementation of what you're expecting that doesn't break current functionality, and isn't going to be completely inconsistent.