Perfect PlayerDictionary and Player

Post Python examples to help other users.
User avatar
InvisibleSoldiers
Member
Posts: 81
Joined: Fri Mar 15, 2019 6:08 am
Location: Russian Federation
Contact:

Perfect PlayerDictionary and Player

Postby InvisibleSoldiers » Tue Dec 31, 2019 11:39 am

Perfect PlayerDictionary must have the following properties:
1. It should be one for the whole plugin.
2. It should add and remove player instances when client connect to the server and disconnects from the server respectively.
3. It should if necessary contains functions related to many players.

Also PlayerDictionary must store instances of perfect Player class which have the following properties:
1. It should be inherited from PlayerMixin or another good implementation which also inherited from PlayerMixin.
2. It should store only functions related to the player itself.
3. It shoudn't store functions related to many players.

In this way I do not recommend using standard Player and PlayerDictionary from Source.Python because they don't meet these requirements.
I created a sample prototype of perfect realization of PlayerDictionary and Player.

PlayerDictionary:

Player:

Edit:
Probably better though separate functions related to many players in another class so it doesn't bend the meaning of PlayerDictionary only for associating indexes with some instances.
So we can have a list with player instances for fast iterating over it and PlayerDictionary itself only for retrieving instance by index from it.
I'll redo it later.
User avatar
satoon101
Project Leader
Posts: 2607
Joined: Sat Jul 07, 2012 1:59 am

Re: Perfect PlayerDictionary and Player

Postby satoon101 » Tue Dec 31, 2019 3:57 pm

I'm honestly not sure why you think this is better than the current implementation.

Removing all the functionality of Entity seems like a horrible idea to me...

Adding get_spectators to the dictionary and having to pass a specific Player to the method doesn't make sense. If you want to deal with 1 specific Player, the method belongs on the Player object.

Why set playerinfo on instantiation? If the playerinfo never gets used, you wasted a call. We already have playerinfo only calling playerinfo_from_index once on the object anyway, because we store that in _playerinfo and if it isn't None, we just return the value we've already retrieved.

Why set permissions on instantiation? Again, if it is never used, you wasted time making that call. Also, permissions can change during a game, so not calling it at runtime could cause you to have old information.

Why remove all doc-strings? They're very helpful and help create our wiki.

Why change view_coordinates to 'get' only? And, why change the code of getting view_coordinates? Is that better than the existing?

The one thing that at least has some merit is adding all exiting players to the dictionary on instantiation. Players are already removed when they exit the server. I can understand why you would want to add all players that way. But, since __missing__ already adds them when you're trying to retrieve one, it would seem to me that that would not be necessary. And, if someone wants to do it themselves, they are more than welcome to it.

I also completely disagree with having 1 dictionary for the entirety of the server. Many scripts inherit from Player and extend its functionality to fit their plugin's specific needs. How is allowing them to create their own dictionary and use their Player class as the 'factory' not the best solution?
Image
User avatar
InvisibleSoldiers
Member
Posts: 81
Joined: Fri Mar 15, 2019 6:08 am
Location: Russian Federation
Contact:

Re: Perfect PlayerDictionary and Player

Postby InvisibleSoldiers » Tue Dec 31, 2019 6:50 pm

satoon101 wrote:I'm honestly not sure why you think this is better than the current implementation.

Read to the end.

satoon101 wrote:Removing all the functionality of Entity seems like a horrible idea to me...

PlayerMixin inhreted from BaseEntity. That's enough. And I just created a class with just the needed functions for my plugin. Maybe I wouldn't have thought of it if Source.Python by default had good implementations and not what we have now.
Why are they bad? Because they have internal cache which if you toggle off through passing caching=False anyway won't do any good because let's see on this

Syntax: Select all

@property
def spectators(self):
"""Return all players observing this player.

:return:
The generator yields :class:`players.entity.Player` objects.
:rtype: generator
"""
from filters.players import PlayerIter
for other in PlayerIter('dead'):
if self.inthandle == other.observer_target:
yield other

:smile: I think everything is clear here without any notes.
Also it doesn't meet the requreiment
InvisibleSoldiers wrote:3. It shouldn't store functions related to many players.
The object must not know about the existence of any other, it is the concern of the other class or function to apply spectators finding.

satoon101 wrote:Adding get_spectators to the dictionary and having to pass a specific Player to the method doesn't make sense. If you want to deal with 1 specific Player, the method belongs on the Player object.

No! It is the concern of the other class or function to apply spectators finding. Player object must not know about the existence of others. This is not logical. Current spectators implementation is sort of hack.

satoon101 wrote:Why set playerinfo on instantiation? If the playerinfo never gets used, you wasted a call. We already have playerinfo only calling playerinfo_from_index once on the object anyway, because we store that in _playerinfo and if it isn't None, we just return the value we've already retrieved.

It's better to check every time on reference to None? And I'm sure it will be used. At least to update player's nickname in database after he was disconnected. I repeat this is a class just for my needs.
satoon101 wrote:Why set permissions on instantiation? Again, if it is never used, you wasted time making that call. Also, permissions can change during a game, so not calling it at runtime could cause you to have old information.

That's what I was afraid. I'll change later.

satoon101 wrote:Why remove all doc-strings? They're very helpful and help create our wiki.

I wasn't going to put it on the wiki, it's my class and just an example of an ideal Player with ideal PlayerDictionary. Ideal because there is no functions related to many players inside a Player class, and PlayerDictionary will be only one for a whole plugin.

satoon101 wrote:Why change view_coordinates to 'get' only? And, why change the code of getting view_coordinates? Is that better than the existing?
Because my need is only get_view_coordinates. And yes, a little better. And again that's my class!
The one thing that at least has some merit is adding all exiting players to the dictionary on instantiation. Players are already removed when they exit the server. I can understand why you would want to add all players that way. But, since __missing__ already adds them when you're trying to retrieve one, it would seem to me that that would not be necessary. And, if someone wants to do it themselves, they are more than welcome to it.

This serves to be taken as an axiom that when player joined, we necessary should add it to the related containers because otherwise the player simply will not exist. For example in spectators counting. We avoid using PlayerGenerator every time, it's really nonsense, we already know our players. We abstract from everything able to give us the opportunity to get a player in some other way than our containers which we update on playerconnect and playerdisconnect. Below I'll show the ideal containers.

satoon101 wrote:I also completely disagree with having 1 dictionary for the entirety of the server. Many scripts inherit from Player and extend its functionality to fit their plugin's specific needs. How is allowing them to create their own dictionary and use their Player class as the 'factory' not the best solution?

I wrote "1 dictionary for the enterity of the plugin", not server, because now in standard Source.Python there is more than 1 using, and it's example of good architercrture usage only 1 PlayerDictionary.

Besides perfect PlayerDictionary, we need the ideal list of players if you need iterating over all players. There are only references, so nothing bad. We will update them both on playerconnect, playerdisconnect, now example:

The addiotinal container is serves only for iterating over it, for example when searching spectators, and playerdict only for taking player instance by index.
I swear you can't think of anything better than this, it's really perfect architecture for each plugin.
User avatar
L'In20Cible
Project Leader
Posts: 1229
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: Perfect PlayerDictionary and Player

Postby L'In20Cible » Wed Jan 01, 2020 2:25 am

I totally agree with everything Satoon said, except that I don't see the merit of having PlayerDictionary automatically syncing itself at all. I mean, this is a container, that you are trying to use as a generator. A container stores objects, while a generator generates them. They both have clearly distinct roles.

InvisibleSoldiers wrote:No! It is the concern of the other class or function to apply spectators finding. Player object must not know about the existence of others. This is not logical. Current spectators implementation is sort of hack.

This makes no sense whatsoever. You want to get players that are spectating a specific player, meaning that the logical implementation is to have that feature directly on that player. Without mentioning that building a list to return instead of yielding the players as you find them is more than redundant. I do however understand that the current implementation of Player.spectators is not flexible as to what type is being yielded, but I truly believe you are approaching it entirely the wrong way. The logical approach would be like we did for Entity.is_in_solid and allows users to pass any generator they wishes to be used. For example:

Syntax: Select all

def get_spectators(self, generator=None):
"""Return all players observing this player.

:return:
The generator yields :class:`players.entity.Player` objects.
:rtype: generator
"""
if generator is None:
from filters.players import PlayerIter
generator = PlayerIter('dead')
for other in generator():
if self.inthandle == other.observer_target:
yield other


Now that, in combination of Satoon's suggestion to PlayerIter means that anyone could simply do:

Syntax: Select all

for spectator in player.get_spectators(PlayerIter(yield_class=MyPlayer)):
pass

for spectator in player.get_spectators(my_player_dictionary.values):
pass


While "perfection" is subjective, I personally believe you are trying to reinvent the wheel by making it square. If your implementation is suitable for your projects, then go for it. But you cannot claim it to be perfect or better by adding redundancy, removing flexibility, or trying to use a concept as if it was another. Perfect or better would improves everything altogether, which is not the case of what is being suggested. That being said, this will be my last post addressing topics directly or indirectly related to your obsession about the internal caching. One of my last minute new year resolution is to adhere to the "pick your battles philosophy", and after many posts trying to have a constructive discussion by bringing valid arguments I can clearly see this debate to be a waste of time. So yeah, good luck, and happy new year! :smile:
User avatar
InvisibleSoldiers
Member
Posts: 81
Joined: Fri Mar 15, 2019 6:08 am
Location: Russian Federation
Contact:

Re: Perfect PlayerDictionary and Player

Postby InvisibleSoldiers » Thu Jan 02, 2020 12:43 am

Okay I'm a little bent about iterating over all players in method of one player, but one PlayerDictionary for a whole plugin which will be updated on player connect or disconnect alongside list with all players for iterating over, it is awesome. I'm even scared to do benchmark with PlayerGenerator every time vs the list. Also probably need to change check instance on Player and replace it to PlayerMixin in classes like RecipientFIlter, because both have index property.

Return to “Code examples / Cookbook”

Who is online

Users browsing this forum: No registered users and 0 guests