GPAddons / ClaimClassifier

Addon for GriefPrevention that provides classification for claims, such as sorting and naming
http://r.robomwm.com/patreon
MIT License
4 stars 3 forks source link

Command request: /claimtop to display claimblock balances like /baltop #15

Closed NullCase closed 3 years ago

NullCase commented 4 years ago

The command /baltop or /balancetop (essentials) will display all $ balances for players. I want to build /claimtop (alternatives /claimblocktop, /claimlisttop, other suggestions welcome).

/claimtop will display player balances using the same format as essentials /baltop but instead of $ it shows claim block balances.

Questions: Is there anything about this update which causes unexpected problems? Do you already have plans to build this into GriefPrevention? What does essentials do well/poorly when getting /baltop data? (because it takes a while when you run the command, even for just 1000 players).

Funding is available

RoboMWM commented 4 years ago

Since you need to access the data of every single player, yes, this will take a while, similar to how Essentials does (it is also loading data for every single player in its folder).

I am not building this in GP, there is no need for this statistic.

I don't expect any issues if I manually (i.e., open files not through GP) open the files. I don't plan to change the structure of the playerdata files anytime soon.

NullCase commented 4 years ago

Following up from the above:

OK, similar to essentials. It seems like there will be ways to optimize here, especially if Essentials is calling data for every player each time the command is used.

For example, I don't know if Essentials updates a list once in a while, stores it for when it's requested.

No need for it, naturally :)

And it sounds like if the data files change that'd muck things up.

NullCase commented 4 years ago

It sounds like there's three or four steps here.

RoboMWM commented 4 years ago

To be honest, I wouldn't worry about attempting to optimize it. EssentialsX performs the lookup asynchronously, so it's of no impact to the server thread (i.e., it shouldn't ever show up in timings). I also don't know how often players on your server would be interested in such stats, especially relative to other commands.

The other two look fine.

NullCase commented 4 years ago

Two are fine. OK Great.

Is this week a bad time for Dev work?

RoboMWM commented 4 years ago

Should be fine to get it to it shortly.

RoboMWM commented 4 years ago

So, is this the request?

As a player When I execute /claimtop Then I receive a list of players sorted by total amount of claimblocks in descending order And is paginated with 10 players per page

NullCase commented 4 years ago

That's it, and one thing (italicized)

As a player When I execute /claimtop Display total claimblocks at the top and a list of players sorted by total amount of claimblocks in descending order. And it's 10 players per page.

For reference: 2020-05-28_01 47 40

NullCase commented 4 years ago

Regarding formats, match everything from 'Ordering balances' down to 'Type /claimtop 2 to read the next page'

did I forget anything here?

NullCase commented 4 years ago

How do I find the jar to test it out? I don't have a link.

edit: In case it's not ready yet, just ignore this :) Because i do have this link and despite the name change, perhaps it'll show up here.

RoboMWM commented 4 years ago

Yea it's not ready, just a very very basic implementation of the featurem but feel free to test it out. By default it's set to disabled because I'm using that new command/config registration thingy and I believe I have that default to false for new features. And it only prints out the UUIDs and claimblock balances with no pagination. It should be sorted though.

RoboMWM commented 4 years ago

as for the jar, looks like appveyor doesn't recognize it if I rename the repo. So I already went ahead and made a new one in appveyor. Looks like it can't build rn, will fix that shortly. https://ci.appveyor.com/project/RoboMWM39862/claimclassifier/build/artifacts

NullCase commented 4 years ago

With ClaimTopCommand: true With [ClaimslistClassifier] Enabled ClaimTopCommand As server operator, typing /claimtop produces: Unknown command.

2020-05-30_02 52 19

RoboMWM commented 4 years ago

Hmm, forgot I need to add it to plugin.yml to register the command.

NullCase commented 4 years ago

will test tmr

NullCase commented 4 years ago

Generated exception when typing claimtop in console:

[05:25:29] [Craft Scheduler Thread - 6/INFO]: Claimblock totals
--
[05:25:29] [Craft Scheduler Thread - 6/WARN]: [ClaimslistClassifier] Plugin ClaimslistClassifier v1.7.0 generated an exception while executing task 42
java.util.NoSuchElementException: null
at java.util.ArrayList$Itr.next(Unknown Source) ~[?:1.8.0_251]
at com.robomwm.claimslistclassifier.command.ClaimTopCommand$1.run(ClaimTopCommand.java:65) ~[?:?]
at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftTask.run(CraftTask.java:84) ~[patched_1.15.2.jar:git-Paper-318]
at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:54) ~[patched_1.15.2.jar:git-Paper-318]
at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22) ~[patched_1.15.2.jar:git-Paper-318]
at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) [?:1.8.0_251]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) [?:1.8.0_251]
at java.lang.Thread.run(Unknown Source) [?:1.8.0_251]

Generated exception when typing /claimtop in game.

[05:25:47] [Server thread/INFO]: NullCase issued server command: /claimtop
[05:25:47] [Craft Scheduler Thread - 6/WARN]: [ClaimslistClassifier] Plugin ClaimslistClassifier v1.7.0 generated an exception while executing task 47
java.util.NoSuchElementException: null
at java.util.ArrayList$Itr.next(Unknown Source) ~[?:1.8.0_251]
at com.robomwm.claimslistclassifier.command.ClaimTopCommand$1.run(ClaimTopCommand.java:65) ~[?:?]
at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftTask.run(CraftTask.java:84) ~[patched_1.15.2.jar:git-Paper-318]
at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:54) ~[patched_1.15.2.jar:git-Paper-318]
at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22) ~[patched_1.15.2.jar:git-Paper-318]
at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) [?:1.8.0_251]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) [?:1.8.0_251]
at java.lang.Thread.run(Unknown Source) [?:1.8.0_251]
NullCase commented 4 years ago

@RoboMWM thanks for all your hard work here so far. I'm a bit run down so I'll take a couple days to rest now.

I'll look here again by Thursday to continue.

NullCase commented 4 years ago

Hi @RoboMWM I was able to test yesterdays' update. here's the latest error logs. It loosks like this is just displaying for player ignore files.

[08:50:31] [Server thread/INFO]: NullCase issued server command: /claimtop
--
[08:50:32] [Craft Scheduler Thread - 3/WARN]: [ClaimslistClassifier] Skipping file 08493b4a-b0e0-4801-974f-83b36cc1e4be.ignore due to this ''Java-termed reason:''
[08:50:32] [Craft Scheduler Thread - 3/WARN]: java.util.NoSuchElementException
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.util.ArrayList$Itr.next(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at com.robomwm.claimslistclassifier.command.ClaimTopCommand$1.run(ClaimTopCommand.java:66)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftTask.run(CraftTask.java:84)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:54)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.lang.Thread.run(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]: [ClaimslistClassifier] Skipping file 4a391bc8-91f1-46eb-8c55-45b50030c870.ignore due to this ''Java-termed reason:''
[08:50:32] [Craft Scheduler Thread - 3/WARN]: java.util.NoSuchElementException
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.util.ArrayList$Itr.next(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at com.robomwm.claimslistclassifier.command.ClaimTopCommand$1.run(ClaimTopCommand.java:65)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftTask.run(CraftTask.java:84)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:54)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.lang.Thread.run(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]: [ClaimslistClassifier] Skipping file 6ac4761f-f9fe-417f-b7d4-90bb95ede34c.ignore due to this ''Java-termed reason:''
[08:50:32] [Craft Scheduler Thread - 3/WARN]: java.lang.NumberFormatException: For input string: "4dfe4d44-4db1-4874-92ac-bd761f297b84"
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.lang.NumberFormatException.forInputString(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.lang.Integer.parseInt(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.lang.Integer.parseInt(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at com.robomwm.claimslistclassifier.command.ClaimTopCommand$1.run(ClaimTopCommand.java:66)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftTask.run(CraftTask.java:84)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at org.bukkit.craftbukkit.v1_15_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:54)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
[08:50:32] [Craft Scheduler Thread - 3/WARN]:   at java.lang.Thread.run(Unknown Source)
[08:50:32] [Server thread/WARN]: Can't keep up! Is the server overloaded? Running 5442ms or 108 ticks behind
NullCase commented 4 years ago

After further testing:

And just to remind myself later, after that is formatting. Basically matching everything Essentials does. It seems like they have everything that I want.

RoboMWM commented 4 years ago

Interesting, idk why some of the playerdata files aren't formatted as expected, but if it appears everyone's there then I guess it's good. I'll remove the debug messages there next build

NullCase commented 4 years ago

Hi @RoboMWM

I have to delay testing possibly for two weeks or more. My computer has died following a lightning storm here. It'll take a bit of time to replace. After that I'll be back in business, as usual.

Is this going to cause any problems on your end?

NullCase commented 4 years ago

regarding formating, here's a screenshot i took at the time. just disregard the auctions :)

2020-06-04_09 05 23

RoboMWM commented 4 years ago

ya that's how I expected it to look thanks. Will be a little while to work on pagination and I gotta find a good way to convert UUIDs to names, cuz I think if I use the Bukkit way, it's likely gonna do a web request to lookup the name if it isn't cached

RoboMWM commented 4 years ago

I guess if it matters, which would you like to see first, pagination or names first?

NullCase commented 4 years ago

What one first... i don't know, whatever order makes sense on your end is great.

Also, in case you missed that note I'll be unavailable due to computer issues. could be a bit.

NullCase commented 4 years ago

If you haven't already, pagination then names. So I can see in one glance what the formatting up top looks like.

I still can't load up MC to test anything yet. New PC is ordered.

RoboMWM commented 4 years ago

Yea, both are separate tasks with about equal estimated complexity. Will work on that first, thanks.

RoboMWM commented 4 years ago

I switched the algorithm to use taskchain (a library for chaining asynchronous/synchronous tasks) so I can safely save the result for pagination. I haven't worked on the pagination command yet, but I have implemented the "first page" so idk if you're setup to test yet or not.

NullCase commented 4 years ago

Plan is to be set up and testing by late next week, provided the replacement arrives on schedule.

NullCase commented 4 years ago

The command /claimtop now outputs only:

/claimtop <page>

2020-06-26_04 28 33

RoboMWM commented 4 years ago

It should never return false (which causes this) if you don't input any extra arguments. I.e., don't do /claimtop 1 just do /claimtop

RoboMWM commented 4 years ago

May need to also check if it's enabled in the config. I have it setup to automatically add new commands to the config but they default to false. I'll make a change so your features default to true.

NullCase commented 4 years ago

When running /claimtop

[06:56:12 ERROR]: null
org.bukkit.command.CommandException: Unhandled exception executing command 'claimtop' in plugin ClaimslistClassifier v1.7.0
        at org.bukkit.command.PluginCommand.execute(PluginCommand.java:47) ~[patched_1.16.1.jar:git-Paper-1]
        at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:159) ~[patched_1.16.1.jar:git-Paper-1]
        at org.bukkit.craftbukkit.v1_16_R1.CraftServer.dispatchCommand(CraftServer.java:792) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.PlayerConnection.handleCommand(PlayerConnection.java:1908) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.PlayerConnection.a(PlayerConnection.java:1719) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.PacketPlayInChat.a(PacketPlayInChat.java:47) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.PacketPlayInChat.a(PacketPlayInChat.java:5) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.PlayerConnectionUtils.lambda$ensureMainThread$0(PlayerConnectionUtils.java:23) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.TickTask.run(SourceFile:18) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.IAsyncTaskHandler.executeTask(IAsyncTaskHandler.java:136) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.IAsyncTaskHandlerReentrant.executeTask(SourceFile:23) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.IAsyncTaskHandler.executeNext(IAsyncTaskHandler.java:109) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.MinecraftServer.aZ(MinecraftServer.java:1136) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.MinecraftServer.executeNext(MinecraftServer.java:1129) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.IAsyncTaskHandler.awaitTasks(IAsyncTaskHandler.java:119) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.MinecraftServer.sleepForTick(MinecraftServer.java:1090) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.MinecraftServer.v(MinecraftServer.java:1004) ~[patched_1.16.1.jar:git-Paper-1]
        at net.minecraft.server.v1_16_R1.MinecraftServer.lambda$a$0(MinecraftServer.java:177) ~[patched_1.16.1.jar:git-Paper-1]
        at java.lang.Thread.run(Unknown Source) [?:1.8.0_251]
Caused by: java.lang.NullPointerException
        at com.robomwm.claimslistclassifier.command.ClaimTopCommand.onCommand(ClaimTopCommand.java:61) ~[?:?]
        at org.bukkit.command.PluginCommand.execute(PluginCommand.java:45) ~[patched_1.16.1.jar:git-Paper-1]
        ... 18 more
NullCase commented 4 years ago

OK, it looks like things are working.

What i notice is there's just 9 players listed. Everything else looks like what you meant at this point (still UUIDs, but it's printing the first page).

2020-07-03_05 44 31

RoboMWM commented 4 years ago

Alright cool! Yes, I started implementing pagination but haven't added the argument to specify page number just yet.

RoboMWM commented 4 years ago

Ok, I updated the algorithm to support page numbers properly, hopefully my math/logic is correct and it works as expected. I think it will error if negative numbers are put for page (which I can either perform an absolute value, or just send an error message? Whichever is your preference), but it should print nothing if a greater page number is entered.

NullCase commented 4 years ago

hi @RoboMWM I messed up and have taken a week off without saying so here.

I'm back to business on Saturday. Thanks for all of your hard work.

RoboMWM commented 4 years ago

No problem!

NullCase commented 4 years ago

It looks pretty good! Pages are working.

Using the absolute value to convert negative page values sounds smart, because an internal error message isn't info players can use.

Is there anything else that has to happen before converting UUID's to usernames?

2020-07-11_07 02 20

RoboMWM commented 4 years ago

Other than colors, if the pagination looks correct (e.g. page 137 should exist), then I think that's it and I'll start figuring out the best way to convert these UUIDs to names.

IIRC, Essentials has a head start on this because they store the name with their userdata. GP does not - and caches the OfflinePlayers returned by Bukkit (which is from usercache.json). If Server#getOfflinePlayer is safe to call async, then there's no issue - but if that's not the case, some more engineering will have to be thought of.

NullCase commented 4 years ago

Colors yep. And I just remembered one thing

/baltop has the Server Total: a sum total of all balances which is a feature I'd like to replicate for claim blocks.

It sounds like converting UUIDs will be tricky.

Keeping in mind I don't know how to code... so this question might not make sense. What kind of risks do you expect by calling Server#getOfflinePlayer asynchronously?

RoboMWM commented 4 years ago

Mk, added colors and moved the sorting method to the async portion of the code. Please check to see if everything is as you like it to look. Also - what happens when you set the page number as 0 (and what behavior do you expect)?

What kind of risks do you expect by calling Server#getOfflinePlayer asynchronously?

Calling API methods asynchronously, if not supported or documented, is undefined - as in, who knows what may happen. Most of the Bukkit API is not safe to call asynchronously. There are some safe cases, though idk if this is one of them.

NullCase commented 4 years ago

Everything looks good. It's fast. UUIDs still show (but it seems like you expect that).

2020-07-12_13 51 29

what happens when you set the page number as 0 (and what behavior do you expect)?

It seems like showing page 1 when 0 is requested will be fine. Here's what happens rn.

[13:53:04 WARN]: Unexpected exception while parsing console command "claimtop 0"
org.bukkit.command.CommandException: Unhandled exception executing command 'claimtop' in plugin ClaimslistClassifier v1.7.0-2dc990e
        at org.bukkit.command.PluginCommand.execute(PluginCommand.java:47) ~[patched_1.16.1.jar:git-Paper-75]
        at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:159) ~[patched_1.16.1.jar:git-Paper-75]
        at org.bukkit.craftbukkit.v1_16_R1.CraftServer.dispatchCommand(CraftServer.java:794) ~[patched_1.16.1.jar:git-Paper-75]
        at org.bukkit.craftbukkit.v1_16_R1.CraftServer.dispatchServerCommand(CraftServer.java:756) ~[patched_1.16.1.jar:git-Paper-75]
        at net.minecraft.server.v1_16_R1.DedicatedServer.handleCommandQueue(DedicatedServer.java:411) ~[patched_1.16.1.jar:git-Paper-75]
        at net.minecraft.server.v1_16_R1.DedicatedServer.b(DedicatedServer.java:378) ~[patched_1.16.1.jar:git-Paper-75]
        at net.minecraft.server.v1_16_R1.MinecraftServer.a(MinecraftServer.java:1212) ~[patched_1.16.1.jar:git-Paper-75]
        at net.minecraft.server.v1_16_R1.MinecraftServer.v(MinecraftServer.java:1000) ~[patched_1.16.1.jar:git-Paper-75]
        at net.minecraft.server.v1_16_R1.MinecraftServer.lambda$a$0(MinecraftServer.java:177) ~[patched_1.16.1.jar:git-Paper-75]
        at java.lang.Thread.run(Unknown Source) [?:1.8.0_251]
Caused by: java.lang.ArrayIndexOutOfBoundsException: -10
        at java.util.ArrayList.elementData(Unknown Source) ~[?:1.8.0_251]
        at java.util.ArrayList.get(Unknown Source) ~[?:1.8.0_251]
        at com.robomwm.claimslistclassifier.command.ClaimTopCommand.print(ClaimTopCommand.java:129) ~[?:?]
        at com.robomwm.claimslistclassifier.command.ClaimTopCommand.parseAndPrintPage(ClaimTopCommand.java:118) ~[?:?]
        at com.robomwm.claimslistclassifier.command.ClaimTopCommand.onCommand(ClaimTopCommand.java:52) ~[?:?]
        at org.bukkit.command.PluginCommand.execute(PluginCommand.java:45) ~[patched_1.16.1.jar:git-Paper-75]
        ... 9 more
NullCase commented 4 years ago

Is it a problem if the .ignore files aren't printed to the console? I don't know, you might already have a plan for that. Just because it seems like they could really add up after a while. console ignore file spam

RoboMWM commented 4 years ago

Interesting, didn't know you had some .ignore files in there. Ya, I can remove that message. Just didn't expect there to be any invalid files in there so I am kinda curious what the contents of those files are.

NullCase commented 4 years ago

those files just have a players UUID and then list UUIDs of players they ignore.

RoboMWM commented 4 years ago

Oh... interesting. I thought GP used a dedicated ignore file. Guess he changed that or maybe it was always that way and I didn't realize that.

NullCase commented 4 years ago

I first realized it at least as early as 2017. Prior to that i don't know.

NullCase commented 4 years ago

Tested the latest build:

test 7 14 2020

RoboMWM commented 4 years ago

Ok, thanks. I guess my quick file extension check didn't quite work. Will fix via a better known/supported library for that.