User implementation requires discussion about spongeforge HOT 10 CLOSED

During a discussion in IRC I realized that a good target for User might be GameProfile. It fits the general purpose of User, even if its functionality will have to be extended somewhat (for timestamps and the ban/whitelist checks which are stored else where).

from spongeforge.

There's no need to use a mixin for User.

Instead, User can be a wrapper style class, say SpongeUser. See BlockWrapper for example.
The class SpongeUser will hold a reference to the user's GameProfile and access things from the server instance like whitelist, player data directory (for accessing inventory data) etc.

from spongeforge.

@Deamon5550 @simon816 The issue with both of these solutions is that the User implementation would then be removed from the normal entity hierarchy. They're both doable, but wouldn't function properly without special measures that are beyond me

from spongeforge.

@Minecrell

You may want to comment here due to your API changes with GameProfile.

from spongeforge.

I don't think we can use a mixin for User. authlib.GameProfile is not a good target for that because we added sponge.GameProfile to the API recently and it implements that already. Even without, GameProfile represents a different level, an identification and a few properties of players without any persistence.

Because of that we will most likely need to create an extra class for User. I don't think you can call it a wrapper class, because there is no real underlying class for this in Minecraft. Instances of this class would be created only when they're needed by a plugin.

@caseif I don't think an User, in general, should be an entity. It's neither existent in the world nor actually any living entity. Although you could possibly implement most of the entity methods by just modifying the saved data, but it would still not apply to the correct definition of entities in the SpongeAPI.

from spongeforge.

iirc the point of User was to allow manipulation and access of offline data, I always advocated having these methods confined to User and just doing getUser() from player type entities.

from spongeforge.

@modwizcode I agree, so it would be player.getUser().isBanned() for example. (Or maybe getData)

It may be useful to see some old comments on the use of an offline player, SpongePowered/SpongeAPI#72 (comment)
Before OfflinePlayer was renamed to User, I made a comment that Player is not a type of OfflinePlayer.
There seems to be a missing method in the API to access offline players, you cannot get user data without them being online when retrieving it. I think Server#getUser(UUID) should be added.

from spongeforge.

@modwizcode I have to agree with that as well, it seems like the most logical relationship. However there's no reason that Player can't still implement User and simply proxy all the method calls to the internal User instance for convenience purposes.

from spongeforge.

I have a PR open, SpongePowered/Sponge#87
Any further discussion can happen there.

from spongeforge.

@simon816 Don't close the issue until it has been resolved.

from spongeforge.