nikosgram / gringotts

Gringotts is an item-based economy plugin for the Bukkit Minecraft server platform. Unlike earlier economy plugins, all currency value and money transactions are based on actual items in Minecraft, per default emeralds.
https://www.spigotmc.org/resources/gringotts.42071/
BSD 2-Clause "Simplified" License
43 stars 36 forks source link

AccountChest logic changes #174

Closed GroobleDierne closed 1 month ago

GroobleDierne commented 6 months ago

The goal of this PR is to change the way transactions from and towards account chests works in order to avoid loading chunks on every transaction.

The idea behind this is to store in the database the balance of every AccountChest, when a transaction is made, the system will check if the account has enough money based on the sum of its vaults' balances stocked in DB or on the account's balance (not sure if this information is calculated on every call currently). If the account has enough money and the transaction can be made, a PendingTransaction will be emitted containing the targeted chest and the amount to credit/withdraw. Once a new chunk is naturally loaded, the pending transaction system will check if there is a transaction for a vault in that chunk and apply it if so. If the chunk is already loaded when the transaction occurs, no need to go through the pending system. Of course, the pending transactions need to be stocked in the database in case of a crash or server restart.

Additional features that I'm not sure are worth implementing at this point:

I'm opening this pull request very early in the process to get reviews on the concept mainly from @nikosgram & @painOchoco .

PainOchoco commented 6 months ago

Nice! this feature would enhance performance greatly. Some tests would be great because this feature touches a core part of the plugin and we want to make sure it works consistently. I don't know if the test suite is able to test that but it would be nice.

nikosgram commented 6 months ago

I highly recommend not changing the existing model, but instead creating a new model that stores the location, including the world id, of the vault and the a cache of the total value from all different denominations might exists.

For example: [cached_vault_values_table]

  • <x>,<y>,<z>,<world_id>,<total_value>

Let's say, there is a vault in location 10,65,0 and in the world with UUID ebeb6a25-fbea-4004-8699-811d8a193f18. The vault has 10 emeralds, which their total value is 10, and 1 emerald_block, which it's value is 9, the total_value of the vault would be 19 - 19 = (10 [total_emeralds] * 1 [emerald_value]) + (1 [total_emerald_blocks] * 9 [emerald_block_value]). The entry is the table would look something like that:

  • 10,65,0,ebeb6a25-fbea-4004-8699-811d8a193f18,19

That will allow removing complexity from the original modal and allow for a bit more flexibility when it comes down to testing and backwards compatibility.

@PainOchoco what are your thoughts on this one?

Or we don't need to split the location into multiple fields/cols but instead have them combined. That will allow us to search faster by creating a hash index of it. For example:

GroobleDierne commented 6 months ago

I don't really see the interest of using a hash as this is meant to be stored in the DB? Also I spent maybe 1 hour trying to figure out how EBEAN is working but it led nowhere :sweat_smile: I cannot find a way to update the tables definition or at least delete and recreate them.

GroobleDierne commented 3 months ago

Upgrading from a 2012 to a 2024 version of a library is most painful thing I have endured

GroobleDierne commented 3 months ago

It seems to be functional we will start to test it

GroobleDierne commented 1 month ago

@nikosgram I'll probably make a follow-up on these changes in a couple days, I need to add more documentation and maybe make some changes. Also can you publish a snapshot of the maven artifact? I need to update gringotts-towny as it broke because of my changes. Thx

nikosgram commented 1 month ago

Sure :) I'm also making few changes regarding supporting 1.20 and 1.21.

GroobleDierne commented 1 month ago

Oh yeah I should have committed my work on 1.21 too 😅

GroobleDierne commented 1 month ago

Also i need to document how to upgrade the database for the new version

nikosgram commented 1 month ago
<dependency>
  <groupId>minecraftwars</groupId>
  <artifactId>gringotts</artifactId>
  <version>3.0.1-SNAPSHOT</version>
</dependency>
nikosgram commented 1 month ago

Still a snapshot, for testing purposes. Not gonna make a stable release yet.