search

The-Fireplace-Minecraft-Mods / Grand-Economy

A Minecraft server-side economy mod and api for hooking into various economy mods/plugins.
https://www.curseforge.com/minecraft/mc-mods/grand-economy
Mozilla Public License 2.0
5 stars 8 forks source link

[1.12.2] GE arbitrary enforceNonNegativeBalance config option #23

Closed Caltinor closed 4 years ago

Caltinor commented 4 years ago

Describe the bug Changing the config value to false does not permit negative values. This is behavior was noticed when using the API for a mod compat implementation

To Reproduce Steps to reproduce the behavior:

  1. set config option to false
  2. set GNC config tax rate to very short (eg. 100)
  3. set player balance to 5000
  4. create guild (survival mode for all)
  5. put remaining balance into guild account (E -> 3 -> enter value in center of screen and use "+" to transfer)
  6. claim one chunk
  7. claim outpost in non-adjacent chunk.
  8. claim land connected to the outpost but do not connect to first until only $100 remains in guild account.
  9. refresh guild menu to see account reduce by $30 each time taxes are applied.
  10. observe no more reduction in guild funds once it falls below $30. Guild&Commerce-1.2.4d.zip

I think I found out why. The economyWrapper does a check for the config value: https://github.com/The-Fireplace-Minecraft-Mods/Grand-Economy/blob/93687402f39d7ccc20b32b7ed2caba225f5dab08/src/main/java/the_fireplace/grandeconomy/GrandEconomy.java#L65-L72

But when the takeFromBalance method is invoked it still returns false if the balance outcome would be negative https://github.com/The-Fireplace-Minecraft-Mods/Grand-Economy/blob/93687402f39d7ccc20b32b7ed2caba225f5dab08/src/main/java/the_fireplace/grandeconomy/econhandlers/ge/GrandEconomyEconHandler.java#L42-L43

Since you are checking the whether the balance is greater than the amount when you confirm the config is set to true, you could remove the two lines from takeFromBalance entirely.

Logs/Screenshots/Videos

Versions (please complete the following information, do NOT say "latest"):

The-Fireplace commented 4 years ago

This is intentional, grand economy's native currency is not allowed to be negative either way. the enforce non-negative values option is to deal with other plugins/mods that aren't designed to go below 0 but do so anyways.