[Dev] Is there a need for an OfflinePlayer

.awardAchievement()
.chat()
.damage()
.getEntityId()
.kickPlayer()
.playNote()
.sendMessage()

Just some more.

1 Like

I don’t like the idea of throwing exceptions though. Mainly want we can workaround that. Exceptions should be used when you have input or output (user input data, file interaction, network calls). I could be wrong but I’ve always seen Try{} catch() a way to slow down your application.

Not sure if it slows down apps. Possibly. If so, it might not be the best idea and another solution might be better. But I dislike the idea of an OfflinePlayer.

Well if you don’t want exceptions to occur just check if the player .isOnline() and if he’s not then just don’t use the methods.

I don’t think it’s good practice to put a try catch every time we want to use some of the functions from player objects.
Yes you can check if the player is online first but then if he’s online you still have to put a try catch.

I personally used OfflinePlayers mostly for one of two purposes:

  1. A convenient way to store and hand UUID + Name pairs around, being able to use UUID’s internally but Names for User in/output.

  2. To access/modify basic Information about the player like last/first online, check if he is banned, when he was las seen, and sometimes taking actions like to ban.

The Player implementing OfflinePlayer was - at times - usefull, since I didn’t have to worry what type of player I got. But appart from that, it didn’t really make sense and posed a pittfall for memory leaks if you intended to store an OfflinePlayer and istead got an online one.

For me, after reading the above posts and discussions, the most natural and clean solution would be to have a Player representing a player entity currently on the server. Get rid of the OfflinePlayer completely. Add some kind of User / Profile / etc. for the UUID + Name paring and lookup. Some kind of Access to player stats like last seen, banned or not, logout location, etc. for players currently not online.
Edit: Having the same Player object for online and offline players is in my oppinion not ideal. It would be confusing and might lead developers to take actions, although the player is not on the server at all.

  • Opening inventories for them on the spot.
  • Applying potion effects
  • Setting a temporary vectors
  • (Bukkit wise) player.hasPermission doesn’t work for an OfflinePlayer
  • Probably more but I can’t think of any

Forgot who it was but the suggestion of having a User interface to store all the data and then have Player if we need to do things with it, forgot the details was mentioned on IRC few weeks ago

Maby if you want to edit a player location
before he comes back

With a Bukkit-like OfflinePlayer/Player system it gets much worse, because if you store a reference to a Player object and the player goes offline, the methods will throw exceptions, just as they would with only one Player class, except they’re undeclared exceptions and so they’re unlikely to be caught before breaking your plugin.

I think @MrMysteri0us said most there is to say.

Probably some methods should be overloaded to take an optional “ignoreOffline” argument, e.g.

public void heal(int strength, boolean ignoreOffline)

for things that can technically be done without the player being online, but are not usually intended.
For example, it could be convenient for admins to be able to heal a player who is offline (ignoreOffline = true), but some other plugin, e.g. dealing with radioactivity, has ignoreOffline = false so it doesn’t just kill the player while he’s offline.
The same would go for teleportation, applying potion effects and basically anything that modifies the player in any way. The only method that really has to throw an exception when the player is offline would be sendPacket (if we even get access to that).
Even writing something in the chat or running a command could technically be done without the player being online, right?

Seems to me that there should be Player and an OnlinePlayer class that inherits from Player. OnlinePlayer would have the methods/behaviors that can only be performed when the player is online. In general I dislike having methods/behaviors on a class that throw exceptions if the object is in an incorrect state.

In general I dislike having methods/behaviors on a class that throw exceptions if the object is in an incorrect state.

So explain to me what else happens when:

  • you get an OnlinePlayer object
  • the player goes offline
  • you call a method on your OnlinePlayer object

If you have only one Player class, the exceptions are at least declared.

I am not sure 100% of the functionality of how the internal player list is managed, but I suspect that a player will not go offline out from under the code in the plugin that is executing. Regardless, a plugin should not maintain a strong reference to entities managed by the framework across “ticks”, IMO.

But related to the general concept of exceptions, exceptions should only be thrown if a given situation is exceptional. Normal expected errors should report themselves through other mechanisms, such as a return code. IMO, if there is a single player class that merges the both online and offline concepts then a method that only works when the player is online should return an error, not an exception, because the design expect that these methods would fail at some point. This particular point can be somewhat religious and I am sure there are those that have differing opinions.

1 Like

But related to the general concept of exceptions, exceptions should only be thrown if a given situation is exceptional.

So if we look at java.lang.Object.wait():

public final void wait() throws InterruptedException

Doesn’t look that exceptional to me. Sure, solving this by using exceptions is everything but pretty, but at least it’s safer than having objects to which no strong references should be stored, but developers don’t even get a notice, if they do.
And as I mentioned before, as far as I see this would only be absolutely required for “sendPacket”.
Another idea that just came to my mind would be:

public interface Player
{
    public boolean isOnline();
    public void sendPacket(Packet packet) throws PlayerOfflineException;
    public void sendPacketUnsafe(Packet packet); /* throws a RuntimeException,
                                                    so can be used without
                                                    a try/catch block, but its
                                                    name  is likely to make
                                                    devs check the javadoc to
                                                    find out why exactly
                                                    this is unsafe. */
}

That is exactly what it has been used for.
A server I was on had a custom plugin you could send them back to spawn whilst offline if they were in a corrupt chunk and crashing the server when they logged on, also an admin could look at a double chest and move all that persons belongings into the chest in case they picked up a corrupt item (usually it was putting full me drives into electric chests in their inventory.)

1 Like

@theresajayne why not just remove the corrupt item :open_mouth:

I like the idea of having the Player’s data be separate from the Player class, and the Player class just includes convenience methods to retrieve and set frequently accessed data.

However, I don’t understand why exceptions have to be thrown for calling methods that require the player to be online while the player is offline…why can’t it just do…nothing?

Nothing fatal happened that constituted an exception to be thrown and nothing fatal would happen that requires a try-catch block. If the API can avoid the fatal thing from happening, then why doesn’t it? Why must we force the end-developer to do the checks?

I can see why exceptions would be used, you want to inform the end-developer that the player is offline, but that’s not what exceptions are for. You can have a return value that indicates whether the message was sent or not, or have a method that tells you whether the player is online or not.

Nothing fatal happened when I tried to send a message to an offline player, so a fatal exception shouldn’t be thrown.

2 Likes

OfflinePlayer is essentially just that. It’s an easy way of getting stuff from the data file.

Besides the need for OfflinePlayer for getting to persistent vanilla player state while they are offline, it is also quite critical to security administration - we need to be able to talk about granting/revoking privileges in plugins for users that are not currently active. This was a big problem for plugins that needed to deal with privileges with the Bukkit security model - there was no way, for example, to check privileges on an offline player: a big problem for delayed/scheduled actions, or out-of-band access (such as player-privilege-based presentation through apps like Dynmap). Dynmap, specifically, wound up having to implement plugin-specific support for accessing privileges for offline players (which, in a word, sucked) - which largely defeats the point of a framework-defined security model.
Beyond that, the option to access player player persistence has lots of good uses - managing player inventory, whether or not they are online, has lots of value. Ditto for spawn locations and the like.
Now - whether or not this means an ‘OffinePlayer’ class, or just the option of accessing a Player by name or GUID (whether or not they are online at the time) - isn’t particularly material to this: I personally prefer NOT having a different class, since being online is a matter of state versus a matter of definition, but that’s just my $0.02.

2 Likes

Well another use is when a user has been bannred for potentially duping, an admin can drop the users inventory to a chest and confirm / deny whether that has happened

Wouldn’t UUID’s take this role? I don’t personally see why the player objects should be responsible for permissions, Wouldn’t the permissions system (or proxy of) be responsible?

SpongeGame.getPermissionsSystem().checkPermission(UUID, permissionNode);

It would work regardless of players being online, offline, having a reference to the player, or even being players at all. An entity granted permission to modify their land by the player could have permission to do something.

A system that assigns groups or teams to UUID’s could be used to check if a team or group has permission for an item, and resolve whether a team has permission to do something.