Just wondering what the community thought about this.
Please give reasons to why you think we need an OfflinePlayer
To be able to answer the questions like the following after the player has logged off.
Where was the player last seen?
When did the player last play?
When did the player join the server?
etc,
I had the idea of a PlayerOfflineStorage for all this things …
IRC log:
[11:21] <sk89q> jacklin213: why are you duplicating offlineplayer's methods in player
[11:21] <sk89q> isn't offline player a subset of player
[11:21] <IDragonfire> maybe we need no clea specification ... mabye more a feeling of the direction for things ... e.g. config system, path based, maybe typesafe ... persmmission ... basic stuff with context ...
[11:21] <sk89q> and thus player can extend offline player
[11:21] <jacklin213> i didn't want to do it like that
[11:21] <jacklin213> but how would i have done it?
[11:21] <sk89q> uh
[11:21] <sk89q> eh
[11:21] <jacklin213> have a Player object in OfflinePlayer
[11:21] <jacklin213> like bukkit did
[11:21] <jacklin213> ewww
[11:22] <sk89q> how did that work
[11:22] <sk89q> I forget
[11:22] <jacklin213> Give Player and OfflinePlayer an interface to extends
[11:22] <jacklin213> offlinePlayer.getPlayer().hasPermission("test")
[11:22] <jacklin213> which broke everything
[11:22] <sk89q> oh
[11:22] <sk89q> that is lame
[11:22] <sk89q> can do
[11:22] <sk89q> server.getPlayer(offlinePlayer)?
[11:23] <IDragonfire> http://jd.bukkit.org/rb/apidocs/org/bukkit/OfflinePlayer.html
[11:23] <IDragonfire> http://jd.bukkit.org/rb/apidocs/org/bukkit/entity/Player.html
[11:23] <sk89q> I also think
[11:23] <sk89q> naming something offlineplayer should be avoided
[11:23] <sk89q> it seems like a hack
[11:23] <sk89q> and if player extends offlineplayer, it does not really make sense
[11:23] <jacklin213> ^
[11:23] <jacklin213> but people have been complaining about not having an offline player
[11:23] <IDragonfire> you mean more like a PlayerStorage?
[11:23] <jacklin213> the other way to go about it
[11:24] <jacklin213> would be have everything as Player
[11:24] <TheYeti> Yea, it always seemed strange to me that Player extended OfflinePlayer. Seemed backwards for starters. Seemed like a Player object should be just that, whether they are online or offline.
[11:24] <jacklin213> and provide isOnline or isOffline
[11:24] <TheYeti> ^
[11:24] <IDragonfire> with the components system a player can me a gameobject ... then we have not the problem
[11:24] <jacklin213> though through debate i believed that the components system is now going to be a submodulal if implemented?
[11:25] <IDragonfire> maybe we should refactor it ... OnlinePlayer extends Player
[11:25] <jacklin213> @sk89q u want me to just scrap the OfflinePlayer class then?
[11:25] <jacklin213> ^ was thinking about that
[11:25] <jacklin213> but then people who are used to player
[11:25] <jacklin213> just gonna be Player Player all day everyday
[11:26] <jacklin213> and not knowing OnlinePlayer existing, or we could just throw a javadoc comment
[11:26] <jacklin213> saying "For an online player use OnlinePlayer"
[11:26] <sk89q> well
[11:26] <sk89q> onlineplayer seems ba dtoo
[11:26] <jacklin213> :P
[11:26] <sk89q> clearly
[11:26] <sk89q> User = OfflinePlayer
[11:26] <jacklin213> hmmm
[11:26] <sk89q> not sure that makes sense
[11:27] <jacklin213> actually
[11:27] <jacklin213> that does sound nice
[11:27] *** dariusc93 ([email protected]) joined
[11:27] *** irobin591|away changed nick to irobin591
[11:27] <jacklin213> but then the issue of having the same methods between Player and User will come up
[11:27] <hoqhuuep> I disagree, user is just like UUID + name
[11:27] <sk89q> client too
[11:27] <sk89q> Player
[11:27] <sk89q> PlayerClient
[11:27] <hoqhuuep> OfflinePlayer has a position and inventory still
[11:28] <jacklin213> i guess Player extends User?
[11:28] <hoqhuuep> at least in my mind
[11:28] <sk89q> I agree with hoqhuuep
[11:28] <jacklin213> Player being the online state of the User
[11:28] <sk89q> there is also Player, PlayerClient
[11:28] <IDragonfire> i think it is a design decision
[11:28] <sk89q> however
[11:28] <shmancelot> from the perspective of the guy using the API, it would be nice if a player is a player is a player
[11:28] <jacklin213> ^ what would that be fore?
[11:28] <jacklin213> for*
[11:28] <sk89q> Entity doesn't follow that convention
[11:28] <sk89q> so I don't like it
[11:29] <sk89q> Player, PlayerEntity
[11:29] <sk89q> heh
[11:29] <sk89q> no
[11:29] <sk89q> hmm
[11:29] <TheYeti> Shouldn't a Player just be an extension of an Entity that exposes additional methods, especially if using a component system?
[11:29] <sk89q> the issue is what to do about OfflinePlayer
[11:29] <sk89q> or maybe
[11:29] <sk89q> we don't have OfflinePlayer per se
[11:29] <sk89q> at all
[11:29] <sk89q> we have stuff that "access offline things" on a User
[11:29] <sk89q> or something
[11:30] <jacklin213> i guess Player extends User?
[11:30] <jacklin213> lol
[11:30] <hoqhuuep> no, a player has a user, I think
[11:31] <hoqhuuep> or the other way :P
[11:31] <jacklin213> and methods in player interface would be the ones which can only be done if a player is only
[11:31] <IDragonfire> sk89q, that was the reason i asked, why not have a PlayerStorage (OfflinePlayerStorage) to access all the offline things
[11:31] <jacklin213> online*
[11:31] <sk89q> that might make more sense
[11:31] <TheYeti> User extends Player, as you can technically have Players that do not have Users.
[11:31] <sk89q> and we woudl avoid this problem
[11:31] <jacklin213> hows that sound?
[11:31] <sk89q> well
[11:32] <sk89q> the idea being thrown around here is
[11:32] <sk89q> something like User exists but it's really barebones
[11:32] <sk89q> just uuid/name
[11:32] <sk89q> if you want like offline inventory
[11:32] <sk89q> you can access it through PlayerDataStorage whatever
[11:32] <hoqhuuep> so I use a different interface to move a player when they're offline?
[11:32] <IDragonfire> sk89q, and for online things the sagme
[11:32] <jacklin213> i personally think it would be easier if everyone is a User
[11:32] <IDragonfire> it is more in the way of "component system"
[11:32] <sk89q> hoqhuuep: that would be the case
[11:32] <jacklin213> and they become a player on login
[11:33] <sk89q> it would be more explicit
[11:33] <sk89q> moving an offline player
[11:33] <sk89q> however
[11:33] <sk89q> then you can't just toss around OfflinePlayers
[11:33] <sk89q> then
[11:33] <sk89q> which loses some convenience
[11:34] *** Tomp13 quit (Remote host closed the connection)
[11:34] <jacklin213> i dont think u should be able to toss offlineplayers at all
[11:34] <jacklin213> in that case it would be more like
[11:34] <jacklin213> queue a task to toss the player once he/she logs on
[11:34] <sk89q> i meant
[11:34] <sk89q> toss as in use it everywhere
[11:34] <jacklin213> actually scrap that, not efficient
[11:34] <sk89q> for example if you had a teleport command
[11:35] <sk89q> you could just pass around a List<OfflinePlayer>
[11:35] <sk89q> and it would work easily
[11:36] <MageRooster> I highly approve of being able to move offline players.
[11:36] <MageRooster> And edit offline inventories.
[11:37] <jacklin213> oh actually
[11:37] <jacklin213> lets say we have the User interface
[11:37] <MageRooster> Fixing corrupted things is far easier with that sort of functionality. >.>
[11:37] <jacklin213> we can change the position to a new position
[11:38] <jacklin213> and when a User is logged in, thus turning into a player it logs the player in at the position? if that makes sense
[11:38] *** Goldman60-Y510P quit (Ping timeout: 206 seconds)
[11:39] <IDragonfire> jacklin213, normally the user is as this position ... expect a plugin interrupt it ... but i think the PlayerOfflineStorage is more a way to edit direct the uuid file and change the position in real?
[11:39] *** hoqhuuep quit (Ping timeout: 186 seconds)
[11:40] <jacklin213> hmm
[11:40] <jacklin213> how would the PlayerOfflineStorage work though
[11:40] <jacklin213> load the "player" from data files or something?
[11:40] *** hoqhuuep ([email protected]) joined
[11:42] <IDragonfire> it is not important, if it load the file or change it on next login and use a custom persist layer
[11:42] <IDragonfire> important is, that the dev not knowiing how it works
[11:42] *** irobin591 changed nick to irobin591|away
[11:42] <jacklin213> well thinking off the top of my head, i wouldnt know how it works either :P
2 Likes
Makes sense.
The “OfflinePlayer” doesn’t add anything that is only valid for an OfflinePlayer and not for any player.
Good call.
If there is proper NBT support, then you wouldn’t need to worry about about applying the changes on login, and you could just edit the player.dat directly. This needs to happen on the same thread as the one that handles the player logging in, or you’d end up having concurrency issues.
I repeat myself:
PlayerOfflineStorage hided the implementation …
You can edit the playerdata files, change it on startup …
It is not important how !!!
We talk only about a clean basic API.
We use offlinePlayer object in many of our plugins, not having that going forward would make this… complicated.
Maybe just make it but deprecate it?
As I said in IRC, I don’t see there is a need for an OfflinePlayer. It just felt like a lazy hacked together convenience interface. It also felt extremely backwards to have a player extend offlineplayer.
In my opinion we should have Player. That’s it, nothing else. Whether they are offline or online should not matter. The only difference between a user being online and offline is that an online player can have a target, they can have an ip address, and they can have a time online. None of that really makes enough difference for there to be an entire structure dedicated simply to offline players.
I still say we should have only Player and that at the most there should be getters for isOffline (or isOnline) and if so then certain methods become unavailable or will just return null as they have nothing to return.
From a developer perspective this makes it much simpler to check things like ownership of items/blocks/entities, permissions of players, etc. You simply check for player.hasSomething(). At the most I can see possibly having Player and then having User extends Player which would contain the information and methods that are only available when a player is online.
My two cents anyway.
10 Likes
I would say that there should just be Player to represent an online or offline player, but as it would have to be an Entity, I’m not sure that is best. It would represent an entity that doesn’t really exist anywhere (which doesn’t necessarily have to be bad, I suppose). I think I like the idea of a Player and a PlayerEntity better. Player should allow one to get name and UUID but also get/set location, inventory, etc. PlayerEntity would then be the actual Entity extending Player. PlayerEntity will only exist when the player is logged, but you should always be able to get a Player whether they are online or not. Player should have a method (isOnline) to check if the player is online. With this setup, one should rarely, if ever, need to actually work directly with PlayerEntity.
This is just how I feel about it. I may have missed something, but I think my point is clear.
Please don’t call anything OfflinePlayer. It would be especially terrible if Player extended OfflinePlayer.
3 Likes
OfflinePlayer for me was a problem, not a solution. getting a UUID from the name returned a fake UUID. An easy method in sponge to get a UUID from the name can be great, but not necessarily like OfflinePlayer.
Other than contacting the servers for the UUID, you’d probably have to use a cache for the UUID lookups for the offline players, I have to do that in my CommandHelper scripts when I want to get a players UUID when they aren’t online on the server. Also most other bukkit plugins that have had name to UUID conversions done on them use their own UUID cache dbs. The only problem with a cache is that the name in it will no longer be valid if they ever change their playername, for that you have to update the playername in the cache when they connect to the server.
You could implement a global playername to UUID cahce lookup for the plugins to use for offline players. If you can’t use the cache json file the server creates.
I sometimes did use OfflinePlayer, wasn’t much of a big deal to me…
I think Sponge must have a player UUID internal cache, because too many plugins need this feature, and too many plugins have at this time it own UUID cache. For exemple, Factoid, a plugin I made, uses it own cache.
For the problem if a user changes his name, the name can be changed in cache when the player go back the the server. We have just to compare the name and the UUID.
I think, the best is to have 3 way to find user information.
First: Online player check.
Second: Player cache check. I player must be connected at least one time to be recognized.
Third: HTTP check on Minecraft website. For this option, the problem is the time to have a respond. It can be give to an other thread with a request ID and answer it back like an event, or I don’t known.
The website closes the connection if we made too many requests. This is why the third method must call the first and the second.
I just want to give my opinion, but probably my suggestion is wrong.
1 Like
For a http check you would need to thread it off of the main thread, and have a callback function when it comes back, so you don’t lock up the main thread when doing a lookup on a player.
You could lock on the player.dat and then check if they wern’t already logged in.
I think there is no need for a OfflinePlayer class but rather just a single command.
Player.isOnline or Player.isOffline
I’m not a java coder, but from my experience in other languages it does seem quite strange that that the user of the API has to denote if the player is offline or not, this should be done by the API itself.
1 Like
You might just want to know if Player is online. Not because you use Player\OfflinePlayer metods, but for other reasons.
I wouldn’t have a problem with just having Player, as long as we have a .getOnlinePlayers() that returns online players and a .getPlayer() which will return something weither the player is online or not.
2 Likes