OnPlayerJoin and OnPlayerAuthenticated maximum confusion

[neutron]

Hey,

so while working on the Battlefield Agency plugin (where I strive to kick as fast as possible with the lowest performance impact) I noticed the following oddity, already posted by FalconTx in 2011 (!)

In BF4, the order of events is this:

player.onJoin name eaguid

(sent by the server on blaze reserve slot request)
and once actually connected
 

player.onAuthenticated name

Naively, you'd now think that to get those events, you listen to OnPlayerJoin and OnPlayerAuthenticated.
But what actually happens confused the hell out of me:

- player.onJoin fires OnPlayerJoin and OnPlayerAuthenticated

- player.onAuthenticated fires nothing

Sadly, player.onAuthenticated is exactly the point at which a player can be kicked successfully. Until now I've "solved" the problem by just spamming kicks until the player is gone, but this came with a load of new problems. For my new major update, I'd like to do things properly.
Is there a way to do this properly?

Thanks,
Neutron

[Hedius]

@ColColonCleaner do you maybe know smth about this?

[neutron]

Okay, so I can see that player.onJoin is supposed to trigger PRoCons OnPlayerAuthenticated here: https://github.com/AdKats/Procon-1/blob/master/src/PRoCon.Core/Remote/BF4Client.cs#L823

Given what was said about the events having been switched around after BFBC2, I suppose this makes sense so the Plugin interface doesn't need overloads for both methods.
But what baffles me is that it was apparently forgotten to also override DispatchPlayerOnAuthenticatedRequest in BF4Client.cs. This means that the default code in https://github.com/AdKats/Procon-1/blob/master/src/PRoCon.Core/Remote/FrostbiteClient.cs#L4298 never gets to trigger the event since the packet for BF4's player.onAuthenticated only contains two words (event and soldier name).

The fix here would be to introduce this event override in BF4Client.cs (and probably also BFH etc.):
 

        protected override void DispatchPlayerOnAuthenticatedRequest(FrostbiteConnection sender, Packet cpRequestPacket)
        {
            base.DispatchPlayerOnAuthenticatedRequest(sender, cpRequestPacket);

            if (cpRequestPacket.Words.Count >= 2)
            {
                if (PlayerJoin != null)
                {
                    this.PlayerJoin(this, cpRequestPacket.Words[1]);
                }
            }
        }

This would still leave the events switched around to keep API compatibility, but at the same time trigger events correctly.
I would just make this a PR on GitHub but I don't have a GitHub account under this name so I'm just posting it here.

The problem is just that a new PRoCon version with this fix would not be available on all layers and thus I still need a solution given the current API.
Thanks

[skulls]

The best and safest time to check players and kick them is during OnListPlayers or OnPunkbusterPlayerInfo. If a player is on the player list then they can be kicked more reliably since they are fully authenticated but sometimes it can be a bit too early. OnPunkbusterPlayerInfo happens when their map is fully loaded and are assigned a team and is most reliable but usually happens after OnListPlayers (depends how often your layer is set to fire this event). You could also combine a few events like the following:

1. Create a blank list of "to-do" kicks on plugin load
2. Check your API with player info either OnJoin(earliest) or OnAuthenticated and add banned players to to-do list
3. Check either OnListPlayers(possibly earliest) or OnPunkbusterPlayerInfo(100% reliable) against to-do list
4. Kick player and remove from to-do list

That's not currently how I have my plugin, but will do something similar in the future to avoid waiting on API calls to kick.

[neutron]

You don't really understand @skulls. This is about PRoCons event handling being broken, for which I already posted a workaround/fix. The BF4 servers player.onAuthenticated event is the exact time after which a player can be kicked successfully, but it is not passed on to the plugin correctly by PRoCon.
> OnPunkbusterPlayerInfo
Too late. Map has already loaded, crasher already had a chance to press F12. Just as an example. Also, not every server runs Punkbuster.
> OnListPlayers
Why bother with it? The player list has every player after player.onJoin, even unkickable ones, and it's slower/less efficient to retrieve the player list than to run any other command.

I've "solved" it in my own way rather efficient way now, you'll see that in our next plugin update, but I would definitely like for my patch to go through as this is still a very dirty solution.

[Prophet731]

@neutron You can submit a pull request to the procon repo (https://github.com/AdKats/Procon-1) to correct it in the procon source code.