EngineHub / CraftBook

🔧 Machines, ICs, PLCs, and more!
https://enginehub.org/craftbook/
GNU General Public License v3.0
301 stars 160 forks source link

Payment messages are improperly formatted. #1251

Closed LlmDl closed 3 years ago

LlmDl commented 3 years ago

Versions

CraftBook version: Affects versions 3 and 5. Bukkit version: n/a

Describe the bug

On displaying the transaction results in order to show payments:

https://github.com/EngineHub/CraftBook/blob/df13f63435605cbdc54d6cca46788d7861759c36/craftbook-bukkit/src/main/java/org/enginehub/craftbook/mechanics/Payment.java#L86

Which is using getName() which is meant to return the name of the Economy Plugin. Although the javadoc is slightly ambiguous about what Name of Economy Method means I believe it is clear when you look at Vault's own implementation of EssentialsEconomy.

I believe what Craftbook is trying to show is the amount the player has paid, formatted to the Economy plugin's desired look, which would be done using .format()

player.print(player.translate("mech.pay.success") + CraftBookPlugin.plugins.getEconomy().format(money));
me4502 commented 3 years ago

Firstly this doesn't affect CraftBook 5 as the Payment mechanic doesn't exist for CB5 yet (it's awaiting a Vault replacement, due to economy plugins being generally bad at implementing Vault consistently due to its loose API contracts)

When this was written - a vast majority of economy plugins returned their currency name via getName, and most returned garbage data or just straight up exceptions for the format method.

I'd have to look at the common economy plugins to determine if it's safe to switch to that - but the way it's currently done is intentional and not a bug

LlmDl commented 3 years ago

I hadn't realized this wasn't actively used in CB5. I await Vault's replacement with an open heart. EssEco is pretty active and is returning the economy name, but I have heard they are redoing their Vault implementation.

I'm not sure if you've looked into Reserve but it is quite nice and in many ways superior to Vault. When I can drop Vault support will be a good day.