Vault

Vault

7M Downloads

ItemRepair Balance Deduction

krisdestruction opened this issue ยท 13 comments

commented

BOSEconomy - Not Working
EssentialsEco - Working
iConomy - Not Tested

The developer says he's calling the correct functions. It's a twist but when you repair something using this plugin, it works in EssEco but not BOSE. I have not tested iCo as my test server doesn't have a SQL DB.

Test Case:

  1. Check balance
  2. Create repair block
  3. Wear out a pick
  4. Repair a pick
  5. Check balance

Expected Results
The balance is correctly deducted.

Actual Results
The balance is not correctly deducted for BOSEconomy.

Plugin Dev: QuickWango
Plugin Page: http://dev.bukkit.org/server-mods/itemrepair/

commented

If possible, some output information or test debug data would be great in isolating the issue. I see the current data workflow/function call:
ItemRepair -> Vault -> Econ Plugin

commented

If they cache the value, shouldn't there be a way to invalidate this cache?

commented

@Sleaker I'm a Server Admin not the plugin dev XD

commented

I'll try to use my programming knowledge here and I'll pull out some of my QA experience. Okay is it possible to call something like subBalance() and getBalance() in the implementation? If it's the incorrect balance, then we know it's a vault/BOSEconomy bridging issue.

commented

@krisdestruction - you can add the debugging yourself with the getBalance methods. I have noticed that some economy plugins cache their values resulting in odd balance behaviours sometimes. I'll check Vault's methods, looks like something may have gotten deprecated recently and I missed it.

commented

can be closed, found the problem in the source of Vault.

I used depositPlayer with a negative value what the BOSE7 implementation does not allow.
I'll switch to withdrawPlayer()

commented

Awesome =)
If it works tonight, I'll donate to you for your awesome plugin!

commented

@quickwango - that's actually vault that does that, I could probably reroute all negative values like this so that it doesn't fail.

Also I think it's mostly just iCo via SQL that does the caching. and the reason why is because it queues the write to the db, but if you re-grab the value right after the set it doesn't show the queued data apparently (limited cases)

commented

I know that Vault is the one that "fails" here.

rerouting might be a good idea

commented

@krisdestruction well the thing is Vault never supported withdrawing/depositing negative values. So there's no backwards compatibility being checked for here.

commented

@Sleaker Gotcha. Well to maintain maximum compatibility, it'd be beneficial to include that. I know as a programmer, I'd rather call 1 function (say addBalance) and specify a signed int parameter. I guess it'd be easier for the devs? :/

commented

If I understand it correctly, you can maintain backwards compatibility by rerouting to ensure a non errored case. Unless the function is depreciated, I think it'd be better for other devs to maintain this compatibility.
backs out of dev code fight

commented

That has actually nothing to do with compatibilty. It's fine to have these 2 methods, but the documentation could be clearer about that and the behaviour should be consistent across all eco systems