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
Claim still present with very inactive owner #496
Comments
Claim Expiration check rate is slow and scans one claim at a time every 60 seconds (though I believe it was recently refactored to check owners, will have to doublecheck what I did). If you wish to speed this up modify the value in Advanced.ClaimExpirationCheckRate |
There shouldn't be so many claims on my server for me to reach a point of needing to accelerate this check, + most claims dissappear in their normal time. |
The claim check is random, and it will skip players that meet any of the claimblock requirements... |
It starts at a random position but iterates through all claims. Big_Scary's implementation. Definitely done for efficiency, since the other group complains that expiration makes a big scary number in timings. I don't think creating a timer for each claim that'd have to be updated on player joins/quits would be any better. Enable debug logging in the config to see info about claim expiration checks. Then you could search for the uuid of the player who's claim should expire. |
One of my players suggested the following: It's actually even better than auto unclaiming. We could even disable the autounclaim and allow players to declaim themselves if they're interested in the terrain, and the terrain is expired. Unless you consider the benefit of inactive claim removal to be the desaturation fo inactive claims (In which case having both systems would be the best outcome) |
I actually quite like the idea of a "request expired claim drop" command, I'd find that useful. Personally, I'd probably want it to mark the claim as due to be dropped in a random time (between hours and days), so that it's less useful for people simply looking to drop a claim solely for the purposes of looting builds where that player isn't around any more, but is still useful for people who legitimately want that bit of land. |
Also, if you could give out a permission for claim preserving, that would be nice. |
My claim expire has never worked, ever. I find claims that are 4 years old from time to time still. |
I can see this causing issues - a lot of permissions plugins don't support offline permissions well. A command to do an expiration check manually would be simple, and could easily be done with a small addon. |
I don't see the issue. However that random iteration check works, it just has to check if the owner has a certain permission. If it can delete the claim, it should be capable of doing that first and preventing the deletion. The way I see it, it would be a great permission to offer to donnors, since you give them a reason to contribute for preserving their claims, if they just don't feel like contributing by being "somewhat active". And you can defend better against people who complain about losing claims for inactivity by just telling them to buy a vip rank x'D |
Yes, and then someone's permission plugin doesn't load permissions for offline players and the permission always evaluates false, leading to their donor who paid for exemption complaining, yielding another unhappy server owner here. |
If the permission system fails (Mine doesn't, just switch over to LuckPerms if yours does, because that sounds like a serious issue, and it's not too hard to migrate), I'm assuming the most intelligent thing to do by GP would be assume the user HAS the permission, or stop iterating for inactive claims all-together if the permission system fails (These details may even be configurable). |
Wait, you mean offline, as in "cracked" users? |
Last I was aware, while LP does allow offline access, it will not initiate loads for offline players through standard requests. This is (or at least was) to prevent people from causing lag by making load requests on the main thread. Back when I initially swapped to LP, I had to use its API over Vault in order to actually use offline perms. There is no differentiation provided by perms systems beyond set, true or false, or unset. You cannot determine if a provider has not loaded perms or the server owner has not set them using standard APIs. GP cannot fall through like that.
No, offline as in currently not online. |
Offline permissions do not exist in Bukkit. Permissions are attached to the Player object; there is no such interface for OfflinePlayers. Please enable debug logging in GP config to determine what's happening with the expiration check. This data is needed to start determining where to look for issues in this feature. Although I skimmed through some, a lot of suggestions here would be perfect for addons (adding a command, etc.) |
FYI, the new expiration task has been working for others. GPAddons/ClaimClassifier#10 (comment)
So yea, will need GP debug logging to see the expiration task in action. |
Sorry I answered soo late. But I just updated the plugin to the latest release, for my 1.13.2 server. Still don't have any results, but maybe unclaiming starts working more precisely to the expiry date as you commented. |
It looks like this issue is still happening with GP 16.11.6, Spigot 1.13.2
I guess I'll do this then.
Please consider this. |
Hey. I know this ticket is substantially old. I actually missinterpreted the instructions (probably). I'm not sure because I haven't checked, and none has been reported. But I would bet there should be some claim that's well beyond the 30 day inactivity unclaim threshold on my survival world. Maybe these logs will be helfull, allthough at first glance they don't seem to contain any usefull information. On my side, all I can say is that claims don't unclaim after 30 days. Last claim we found was 50 days old (yesterday) in owner inactivity, and we had to remove it manually (case for which a player command would be really usefull). I'm always using GP's latest version, allthough new releases don't seem to come out that often. |
Idk if you have debug mode enabled (I think you do, just thought I put more in for the expired claim check but I guess not). But ya, would need to know the UUID of the player. |
Will do. |
Here's a terrain, which by the way, presents a bug where it doesn't show the owner's name, but instead the UUID. Which in this case seems rather convenient: |
You can modify the rate and player cache in the config. Looks like it didn't get around to checking that player in that log. |
ClaimExpirationCheckRate: 60 This is how I have them. I found no information on these options, what do they do, and what units do they use? |
1 player checked per 60 seconds
Players' names will be cached for 90 days |
Is there any point in modifying the player cache days (whatever that does)? As for ClaimExpirationCheckRate, I'll try making it lower, see if that helps. |
It's a pretty minimal amount of data, but there's no real point beyond the deletion threshold. Could you post a new GP log (or, ideally, two) of a run of claim checks and a UUID of a player whose claim was missed? GP theoretically checks 1440 players per 24 hours with default settings. I'm wondering if the new system starts at the same position, so if you have too many and too short uptime, it never reaches the final few. |
From EntityHuman: public static UUID getOfflineUUID(String s) {
return UUID.nameUUIDFromBytes(("OfflinePlayer:" + s).getBytes(StandardCharsets.UTF_8));
} |
I'm back after a long time reflecting on this (and the fact that I'm too lazy to make a plugin just for that uuid checking code). I just realized the UUID's for Hub and Survival are the same, and I never forced the user to login to survival, only Hub. I don't know if the example claim is still around though. Anything else you need? |
Can you please reopen this? |
Found out the inactivity time resets once the player disconnects... |
The server saves player information only upon disconnect. The cleanup task ignores online players so this is already known and accounted for. |
Yeah... about opening this ticket (?)
By "known and accounted for" do you mean you will fix it?. It seems like someone online could basically lose their claim while playing right now, does that seem right? |
Are you still having issues? Do you have logs demonstrating the issues?
No, he means that that is already accounted for. The owner must be offline for their claims to expire. |
Oh, turns out GP actually only deletes 1 claim at a time even doing owner-based expiration. That particular user probably had 20+ claims originally, so it took a long time to expire them all. This whole expiration system is a mess. I'd bet that the shorter duration new player claim expiration feature is entirely broken (though frankly, it was a hack in the first place). |
GriefPrevention is a great idea, and is awesome in many ways, simple in some aspects, but also effective at doing what it must do (most of the time). However, I haven't seen the code, but sometimes it feels like something can't be right. It's like some of it's fundamental designs have to be broken even though somewhat functional. (PvP, Claim Inactivity...) If I had to guess I'd say it all probably just derives from the original developer making this plugin in concordance with his own server's needs. |
Yes. Same issues, claims persisting after expiry days timer has long surpassed it's threshold. I get reports on this by players and staff very regularly, aparently people are eager to know when claims would be unclaimed exactly, but they find that there's no guarantee. This seems to be frustrating to everyone, if the time isn't exact one cannot claim nuisance abandoned lands, or even steal from abandoned houses (which seems to be something most of my players find amusing).
I've sent a ton of logs. Just tell me what's missing in them and I'll gather the data. |
Unfortunately, most of the logs didn't really demonstrate anything more than the usual run through, possibly due to a lack of necessary debug messages in certain areas. That said, at this point I'm relatively confident that the actual run is fine, just that some of your users create so many claims that it takes a couple days to entirely clean them out.
Unfortunately, it's more that GP is a long-standing plugin with many maintainers and no defined style guide or system. Trying to work with or around existing features is kind of a nightmare because so many things were crammed in where they'd work, not where they'd make sense. This sort of issue is exactly why Robo is making v20. I believe that the best short-term compromise (without rewriting the entirety of claim expiration and cleanup) would be to move more of the claim checking process into the pre-task. |
For now we can keep waiting and see if eventually they die out. I'll mention this again eventually. |
Thanks for looking at this Jikoo. Haven't had much time to get to v20 especially given the hectic situation - but yes the discussion for design and other things currently live in #63 |
With this level of debugging I doubt I'll be capable of providing any info. What I would do now would be listing persistent claims and their owner's UUID's, and then checking relevant logs, but I just doubt you'll be getting much info from them. Using GP: 16.13.0-c19e3b7. Either the logs are improved, or I just give up and wait for the GriefPrevention rework that you guys are doing, which I guess will take a long time u_u |
All you need to see if the issue is occurring is the UUID of the player who's claims haven't expired but should've. Of course, this would be easier if you used online mode on bungee... |
@RoboMWM No problem. Here's one: https://cdn.discordapp.com/attachments/711609636742234132/711611420524937258/unknown.png Player UUID: Full log: https://gist.github.com/MithrandirCraft/1513811f6b8e055ef7bb1bfa5bc52035 Relevant logs: |
As you can see, the information on my side isn't exactly lacking. |
What's up? Could you at least add a provisional command that forces stuck claims to unclaim if above expiry? So I can give it to constantly complaining players. |
Just noticed this possibly only affects automatic chest claims Can anyone else experiencing this issue confirm? |
I think I can pretty much confirm at this point that the issue only affects claims that have been generated with the automatic chest claim. Shouldn't this help in determining why the claims aren't disappearing? Can I get an answer? I feel like this issue has been completely abandoned. It's been more than 1 year since I first reported this. It really isn't fun to go around unclaiming these stuck areas. Edit: Actually, I think I'll just turn auto chest claiming. It should "get rid" of the issue. Edit2: This configuration puts an end to the saddest chapter in human history. |
@RoboMWM Is there some SQL we could run to purge old claims? I tried to write my own but the last online is not being updated properly for my GriefPrevention as I'm using an ancient build on Paper 1.12.2, so Im trying to join to another table which I can get last seen of a player from, but its inconsistent. |
@TomLewis do you have claims which haven't expired that aren't the 9x9 starter autochestclaim? Disabling auto chest claiming seems to be working like a charm on my side. It even auto deleted all existing claims which didn't expire. Just set chest protection after certain inactivity it to -1 to disable it. |
@TomLewis If you don't have access to a database including up-to-date UUID <-> last play timestamp then no, there is no pure SQL solution. /e slapping this in front so it gets seen - I'm sure you're aware, but THE SERVER MUST BE OFF OR GP WILL JUST SAVE THE STUFF FROM MEMORY. Note that using data from another database (not table!) is somewhat dependent on your SQL server implementation. I have not tested these calls myself - you can always see what the results will be by replacing Abandon subclaims (technically unnecessary, but prevents nasty logs when GP abandons them on load): DELETE FROM griefprevention_claimdata WHERE parentid IN
(
SELECT id FROM griefprevention_claimdata WHERE owner IN
(
/* Replace this call with whatever uuid <-> timestamp data you have. */
SELECT name FROM griefprevention_playerdata WHERE lastlogin < '2020-9-25 20:30:00'
)
); You could also (if this is going to be a repeated thing) write a trigger that automatically deletes claims with Abandon parent claims: DELETE FROM griefprevention_claimdata WHERE owner IN
(
/* Again, any uuid <-> timestamp will do. */
SELECT name FROM griefprevention_playerdata WHERE lastlogin < '2020-9-25 20:30:00'
); @MithrandirCraft While I haven't forgotten this issue, it's a relatively low priority for me due to the high complexity, low impact, and my own inability to find any logical flaws beyond the ones I already outlined. A rewritten system would likely be an addon, but due to the other feature requests for expiration the addon would be relatively time-consuming to write. I am personally way more concerned with trying to improve addon flexibility and general code health. |
@Jikoo awesome! Thanks a bunch I will make a copy of my database locally and have a play! Brilliant! I completely forgot about sub claims, would have caused a mess if I did a clean-up without doing that first! |
Given that the server is in offlinemode I haven't really bothered (i.e., other priorities) to try to figure out how offline player caching works there, if at all, which I assume may have something to do with it. I've added more debug messages to expiration, and testing with that is honestly the best way to figure out what's going on here; test server with 1 day expiration should work; when I was making an addon for a guy to extend claim expiration due dates (and another which causes an auction on expiration), that appeared to be sufficient to test the cases. |
Those answers give a needed insight on how you feel about this issue. About time. Edit: Also no answers on this?: I don't think I'll comment on this issue anymore unless you feel like dragging me out a bit more by asking for pointless information. |
Would appreciate someone running GPClaimExpiration (latest build from CI here) with GP's debug logging enabled on on a test server. It does not support the restorenature option at all yet, that will likely come as a separate addon or a PR changing how GP core considers when to do nature restoration. |
What happened:
An inactive player still has their claim.
What you expected:
Claim should no longer be there. The player has exceeded the configured ammount of time (30 days) offline. No idea how many claimblocks the user has, but I highly doubt the threshold was reached (I'm pretty sure it's impossible). In any case, I wouldn't even know how to check this (DB maybe?)
Steps to reproduce:
No idea man, it just happens. I also receive usual complaints about claims not unclaiming after the configured ammounts of time (like after 31, 32 days), allthough this is usually undone with some extra patience, and the claims dissapear.
Server and GriefPrevention version:
This can be found by running
/version
and/version GriefPrevention
on your server.Paste of
/version
: git-Spigot-3cb9dcb-40cbae4 (MC: 1.13.2) (Implementing API version 1.13.2-R0.1-SNAPSHOT)Paste of
/version GriefPrevention
: 16.11.5Stack trace/error or server log
No errors
Or, paste the server.log at gist.github.com and paste the link here:
GriefPrevention config.yml
Plugin list (if applicable):
Probably not, just ask.
The text was updated successfully, but these errors were encountered: