search

ArcanePlugins / Treasury

🏦 A powerful multi-platform library for next-level plugin integrations.
https://hangar.papermc.io/ArcanePlugins/Treasury
Other
56 stars 13 forks source link

Make migration working #220

Closed MrIvanPlays closed 1 year ago

MrIvanPlays commented 1 year ago

Closes #217

MrIvanPlays commented 1 year ago

@Jikoo can you review please so we can continue working on v2?

MrIvanPlays commented 1 year ago

I've thought of migrating the logic from that long-ass chained countdown latch spaghetti to my new Process library. Opinions wanted.

MrIvanPlays commented 1 year ago

I did a explanation in ArcanePlugins discord when I made the Process library and I'm gonna do a quick rundown here and what is the potential advantage:

Current code relies on at least 1 countdown latch per account (2 latches if the account is a non player one), and a countdown latch for all the account types migrated. Combined with the .thenApply/.thenCompose calls on all the futures and stuff is currently a literal spaghetti. Another disadvantage is that the code waits for 1 account to finish migration before proceeding to another account.

With Process we can implement a simple Process class for player accounts and non player accounts, make x amount of instances of that implementation and run them in parallel and have it wait (either sync or async) for all of them to finish. Process uses only 1 (!) countdown latch per runProcesses call, reducing the spaghetti down to nonexistent.

To add to this, since all the migration code is running async I'm pretty sure we're good to replace all the .thenApply/.thenCompose spaghetti with simple .get/.join calls. My tests conclude they do not swallow exceptions.

Jikoo commented 1 year ago

That sounds good to me. I think the old Phaser code did run pretty parallel, but it was also an absolute nightmare of back and forth. Honestly, anything to make migration cleaner.

MrIvanPlays commented 1 year ago

Alright. I think in about 2 hours ill be done implementing it.

MrIvanPlays commented 1 year ago

taking longer than expected, i have to do a modification to my library

MrIvanPlays commented 1 year ago

img 😁 Special thanks to: BetterEconomy v2.6-ivanized running the version of treasury on the latest commit on this branch. img1

MrIvanPlays commented 1 year ago

Also a quick clarification: I'm not merging this until we test it.

MrIvanPlays commented 1 year ago

Waiting for the day When @Jikoo will review this

MrIvanPlays commented 1 year ago

Tested and works. I'm still waiting @Jikoo

MrIvanPlays commented 1 year ago

@Jikoo : image