-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Command request: /claimtop to display claimblock balances like /baltop #15
Comments
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. |
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. |
It sounds like there's three or four steps here.
|
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. |
Two are fine. OK Great. Is this week a bad time for Dev work? |
Should be fine to get it to it shortly. |
So, is this the request? As a player |
Regarding formats, match everything from 'Ordering balances' down to 'Type /claimtop 2 to read the next page' did I forget anything here? |
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. |
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. |
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 |
Hmm, forgot I need to add it to plugin.yml to register the command. |
will test tmr |
Generated exception when typing claimtop in console:
Generated exception when typing /claimtop in game.
|
@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. |
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.
|
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. |
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 |
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? |
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 |
I guess if it matters, which would you like to see first, pagination or names first? |
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. |
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. |
Yea, both are separate tasks with about equal estimated complexity. Will work on that first, thanks. |
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. |
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)?
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. |
Everything looks good. It's fast. UUIDs still show (but it seems like you expect that).
It seems like showing page 1 when 0 is requested will be fine. Here's what happens rn.
|
Interesting, didn't know you had some |
those files just have a players UUID and then list UUIDs of players they ignore. |
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. |
I first realized it at least as early as 2017. Prior to that i don't know. |
Ok, thanks. I guess my quick file extension check didn't quite work. Will fix via a better known/supported library for that. |
As for names to UUIDs: it is not safe to run Bukkit#getOfflinePlayer asynchronously. From what I've read, it was (originally?) intended to be thread safe, but it is not. So here are some options:
|
I don't know how the local server cache works. Do all MC servers have this? How do names get into the cache? How long are they held for? What's the downside of using the easy route (beside possibly incomplete name lists) |
Downside of easy route is just that, may be incomplete. The local user cache exists in all servers, and its file can be found in your server's root directory as |
ok awesome. The easy route sounds best. Let's try that. (edit: it looks like Usernames survive for one month in For the 'someone' part let's have that specified in the configuration. Because then it doesn't have to be decided in advance. Also, this might be annoying but I want to add one thing to this feature. What if we set up /claimtop as a permissioned command? 'claimclassifier.claimtop' or something. I don't know how the format works. Because i'd like to specify in GroupManager what groups have /claimtop |
It seems like this claimtop feature was more complicated than it appeared at first glance. Is it a problem on your end? Like, if this kind of thing happens where you want to do one thing and end up having to do ten things.. I don't know if that's normal for coding work. |
That's how things usually happen, but overall we're on track with what you posted here #15 (comment). Your steps don't fully account for technical "hurdles" e.g. converting offline players UUIDs to names (it does for some, if you consider the calculations happening asynchronously to avoid performance impact), but that's fine and not really something you need to know 100% about as the client/customer - just the complexity, as you state. (And I kinda did review the options and did an informal grading of complexity in that comment earlier, so no problems there.) Idk the official agile methodologies, but I prefer doing little releases and seeing if the stuff is progressing to your liking instead of doing something all at once and then going back to change all sorts of things afterwards. I don't feel overwhelmed with this feature at all. As for the 3 day delay that's just because I get busy and forget sometimes, especially after a weekend/beginning of the week. So it's ok to ping me on this from time to time if there's no activity, especially since you're a reliable client and nowhere near impatient. I do wish there was a better way for me to track stuff across all of my projects on GitHub - the new Notifications stuff is a bit closer - but if there was a way to have me view/sort all issues across all my projects would help me remember/attend to the stuff I'd like to finish instead of just visiting the various repos issues or going through my past notifications. |
will test tmr. Thanks @RoboMWM it sounds like little releases are best. Because I too don't want to go back and redo all sorts. Enjoy your time. There's no rush on my end. As you say, no where near impatient. You were very clear at the outset 3-odd years ago that flexible time was critical. Tracking on github... yeah it seems like a common problem. I sometimes also miss issue updates/posts and have to manually visit. Especially for older issues. Or closed issues.. |
when executing /claimtop it threw an exception.
|
It's actually easy to see issues you opened - the topmost bar menu link Issues will provide you that view. I just wish there was an option in that view to view all issues on all repos I'm the owner of/a member of. For the error - I guess I do have to shade in the dependency... the other plugin I use this in I don't shade it, or at least I thought I didn't, so I guess I'll doublecheck that. |
@RoboMWM it looks like ClaimTop is done! $ sent. Is there anything I'm forgetting here? |
Not aware of anything else to do for this. Thanks! |
Alright! It might be a little bit before I have more dev work, or the next job might be a smaller one. It's just because I had to replace my computer and will have to be more careful about future projects. Just a heads up. |
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
The text was updated successfully, but these errors were encountered: