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

when player#getLastPlayed is 0 then /claimexpire check calculates incorrectly #10

Closed NullCase closed 4 years ago

NullCase commented 5 years ago
07.04 07:38:24 [Server] INFO [ClaimslistClassifier] Prevented a claim owned by player UUID 07cdc8c0-66ae-48cb-a5df-11a515898307 from expiring.
07.04 07:38:24 [Server] INFO [ClaimslistClassifier] Planned expiration time: 1594816074001 is greater than current time 1554637104420

OK, looks normal. UUID: 07cdc8c0-66ae-48cb-a5df-11a515898307 is player spfoia

But what about using /claimexpire check playername

07.04 07:41:07 [Server] INFO [ClaimslistClassifier] Server reports spfoia played 17993 day(s) ago
07.04 07:41:07 [Server] INFO spfoia's claims have expired due to inactivity.

17993 days is nearly 50 years. Player last played about 3 months ago.

Player has no claimed land so /claimlist is empty. but the claims were sold off 4 months ago. And they weren't mailed so that all seems good.

Player had days remaining but /claimexpire check says expired.

I don't know what problem this can cause, but it seems odd and potentially an important issue.

RoboMWM commented 5 years ago

Was typing up a response and then noticed - how can it prevent expiration of a claim, yet you tell me the player's claimlist is empty since all their claims have been sold?

RoboMWM commented 5 years ago

(Here's what I was typing up)

Bukkit deprecated Player#getLastPlayed, but it shouldn't affect the current behavior. However, in the claim expiration check I don't check player#getLastPlayed, rather I only check if a /claimexpire value has been set. So I think the claim expiration feature is working fine, the issues are with the /claimexpire check command.

For background - the reason why the claimexpiration check doesn't need to do player#getLastPlayed or similar is because GP handles all of that already before firing ClaimExpireEvent. So if the event fires, I already know the player is past the expiration, and only need to check if they have an extension.

With /claimexpire check you want to have this info + any extension. I can't just return the extended time, since if the player is active they likely will have an expiration time that supercedes the stored one (i.e. config is set for 60 days, player extended for 30 days (90 days total), but logged in again on day 70. /claimexpire check in this case should return day 130 as the expiration (70 + 60), not day 90.)

So if player#getLastPlayed reports incorrectly (it's reporting 0 in this case), that'll mess up the calculations.

NullCase commented 5 years ago

So I think the claim expiration feature is working fine, the issues are with the /claimexpire check command.

That sounds like a good guess. The main feature is working as intended. Also, only when player's are online can they check their own delay. So it's really just confusing for the server operator :D

how can it prevent expiration of a claim, yet you tell me the player's claimlist is empty since all their claims have been sold?

I don't know, it looks like GP is searching by player to see if they've been gone a while and ClaimClassifier butts in once a player with extensions is found, even before GP knows if they have claims.

Idk if that's right, but in that case it's just that CC is claiming to prevent expiration when in fact it prevents GP from something a bit more general. Like... preventing GP from checking the list to try and expire.

RoboMWM commented 5 years ago

I did change the expiration task to go through players instead of claims (yet server admins still complain about unexpired claims while not providing any debug logs) so that's probably why.

NullCase commented 5 years ago

:D

Btw, going through players instead of claims has been really great because absentee land owners are found way faster.

Off topic, it sounds like some of the admins haven't updated or something, but idk.

NullCase commented 5 years ago

idk if that title is more accurate.

RoboMWM commented 4 years ago

Is this still an issue? From what I recall this was a Bukkit bug?

NullCase commented 4 years ago

Not an issue (tested now).

It sounds like you recall right, but i don't know what was going on with Bukkit/if it was the issue.

I'll close this. Can always reopen if it reappears.