Skip to content

Fix jail-online-time with offline players#5701

Merged
JRoy merged 6 commits intoEssentialsX:2.xfrom
pop4959:fix-5658
Feb 26, 2024
Merged

Fix jail-online-time with offline players#5701
JRoy merged 6 commits intoEssentialsX:2.xfrom
pop4959:fix-5658

Conversation

@pop4959
Copy link
Member

@pop4959 pop4959 commented Feb 24, 2024

Fixes #5658

Apparently you cannot get statistics for players returned by getPlayer when they are offline. This may be an upstream bug, but regardless, an easy fix here is to simply allow getting an OfflinePlayer reference from the User base for this purpose.

Below is a demonstration of the issue.

sender.sendMessage(String.valueOf(player.getBase().getStatistic(PLAY_ONE_TICK)));
sender.sendMessage(String.valueOf(ess.getServer().getOfflinePlayer(player.getBase().getUniqueId()).getStatistic(PLAY_ONE_TICK)));
sender.sendMessage(String.valueOf(timeDiff / 50));

90fd0a3c-20fb-42e3-badf-372405fc50b6

As you can see, the offline player returns the correct result, but the player base does not.

@pop4959 pop4959 added type: bugfix PRs that fix bugs in EssentialsX. module: main Issues or PRs for the main Essentials module labels Feb 24, 2024
mdcfe
mdcfe reviewed Feb 24, 2024
View reviewed changes
@pop4959
Copy link
Member Author

pop4959 commented Feb 25, 2024

Not an upstream bug as initially thought, this is just some Essentials goofiness.

JRoy
JRoy previously requested changes Feb 26, 2024
View reviewed changes
Copy link
Member

@JRoy JRoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a fan of this implementation, would rather a logoff-time entry be added to essentials userdata so we can do the calculation on our end. loading offline playerdata is unpredictable and can lead to even worse I/O issues that loading Essentials userdata.

@pop4959
Copy link
Member Author

pop4959 commented Feb 26, 2024

not a fan of this implementation, would rather a logoff-time entry be added to essentials userdata so we can do the calculation on our end. loading offline playerdata is unpredictable and can lead to even worse I/O issues that loading Essentials userdata.

Yeah that works too, alternative would be to get the offline player from the offline player stub which has already done the lookup at that point (so it would incur no additional cost). Or actually implementing the method on the stub itself (which we may want to update anyway - it's horribly outdated at the moment).

@pop4959 pop4959 requested review from JRoy and mdcfe February 26, 2024 11:52
@pop4959 pop4959 dismissed JRoy’s stale review February 26, 2024 13:16

discussed on discord

@JRoy JRoy changed the title Fix Jailing offline players doesn't work with jail-online-time set to true Fix jail-online-time with offline players Feb 26, 2024
@JRoy JRoy merged commit 6b521e7 into EssentialsX:2.x Feb 26, 2024
@pop4959 pop4959 deleted the fix-5658 branch February 26, 2024 19:38
HarvelsX pushed a commit to HarvelsX/Essentials that referenced this pull request Jun 2, 2024
@pop4959 @HarvelsX
Fix jail-online-time with offline players (EssentialsX#5701)
63fe356
Starmism pushed a commit to valence-smp/Essentials that referenced this pull request Jan 8, 2025
@pop4959 @Starmism
Fix jail-online-time with offline players (EssentialsX#5701)
f855636
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JRoy JRoy JRoy approved these changes

@mdcfe mdcfe Awaiting requested review from mdcfe

Assignees

No one assigned

Labels

module: main Issues or PRs for the main Essentials module type: bugfix PRs that fix bugs in EssentialsX.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Jailing offline players doesn't work with jail-online-time set to true

3 participants

@pop4959 @mdcfe @JRoy