Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature getPlayerPing #58

Closed
ozon opened this issue Apr 6, 2013 · 4 comments
Closed

Feature getPlayerPing #58

ozon opened this issue Apr 6, 2013 · 4 comments

Comments

@ozon
Copy link
Contributor

ozon commented Apr 6, 2013

I made some thoughts about the player pings.
Currently, B3 get all pings with ´´´console.getPlayerPings´´´.
This causes a lot of traffic which we not always need.
Most parsers fire for each player a single command to the server.

I wanted getPlayerPings() to expand the parameters client_ids = Noneso that we can pass a list of client id's. This should be done for all parser and would also make it a lot broken. So rather a bad idea.

Therefore I would suggest that parsers implement getPlayerPing(player_id) or even better, the client get an attribute with the name 'ping' (which would still include a getPlayerPing).

What is your opinion on this?

@thomasleveil
Copy link
Member

Although send a rcon request to the Frostbite(2) engine for each player is not a traffic issue, this rcon spaming can be with Q3 game engine which usually support only 2 queries per second.

I wanted getPlayerPings() to expand the parameters client_ids = None so that we can pass a list of client id's. This should be done for all parser and would also make it a lot broken. So rather a bad idea.

It is a good idea IMHO as adding an optional parameter would make this change backward compatible.

Current signature is

    def getPlayerPings(self):
        """\
        returns a dict having players' id for keys and players' ping for values
        """
        # TODO implementation

I would then see :

    def getPlayerPings(self, filter_client_id=None):
        """\
        returns a dict having players' id for keys and players' ping for values.
        If filter_client_id is an iterable, only return values for the given client ids.
        """
        # TODO implementation

Any pull request for such a change is welcome :)

Therefore I would suggest that parsers implement getPlayerPing(player_id)

This change would break backward compatibility and break 3rd party plugins.

or even better, the client get an attribute with the name 'ping' (which would still include a getPlayerPing).

This would make the ping info closer to the client but I'm afraid that if more and more code from both the B3 core and plugins start to use the Client.ping this will force one rcon query per client, but for some game this can be a lot more expensive than making one rcon call which returns all player pings at once.

If a game parser needs to address this issue for a particular game, it would have to monkey patch the Client.ping attribute getter implementation ; while if we keep the ping info access only from the getPlayerPing parser method, we let game specific implementation tuning where it belongs : the game parser.

Do you see what I mean with that point ?

@ozon
Copy link
Contributor Author

ozon commented Apr 6, 2013

Thanks for your remarks.
Currently, I have implemented the following for test:

def getPlayerPings(self, client_ids=None):
    """Ask the server for a given client's pings
    :param clients_id: list of client ids
    """
    pings = {}
    if not client_ids:
        client_ids = [client.cid for client in self.clients.getList()]

    for cid in client_ids:
        try:
            words = self.write(("player.ping", cid))
            pings[cid] = int(words[0])
        except ValueError:
            pass
        except Exception, err:
            self.error("could not get ping info for player %s: %s" % (cid, err), exc_info=err)
    return pings

@thomasleveil
Copy link
Member

That's for BF3 isn't it ?
Would you mind renaming client_ids to filter_client_ids in your pull requests and also update the B3 core parser.py file with the new signature ?

@ozon
Copy link
Contributor Author

ozon commented Apr 6, 2013

That's for BF3 isn't it ?

You know my games ;)

I make the update and pull request soon.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants