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

Fix player not being internally disconnected in some situation and add client_disconnected forward #264

Merged
merged 1 commit into from Aug 13, 2015

Conversation

Arkshine
Copy link
Member

Issue reported by @WPMGPRoSToTeMa here #252.

This fixes the situation of a player is marked as initialized by AMXX but has not yet spawned (put in server), if he disconnects/crashes/timeout at this point, pfnClientDisconnect won't be triggered by the engine. Result: AMXX will think that this player is still initialized and as consequence code relying on it could provide some unexpected result, also client_disconnect won't be called.

While it's necessary that this change be reflected on client_disconnect, instead for backward compatibility reason a new forward client_disconnected has been introduced to be used as replacement. client_disconnect is marked as deprecated.

Technical note: Core hooks SV_DropClient, reHLDS could be supported later, once release is considerate as stable, and/or there is a more friendly approach of the API.

@Arkshine Arkshine force-pushed the fix/dropclient branch 2 times, most recently from f02f013 to 3494d49 Compare July 17, 2015 12:13

DETOUR_STATIC_CALL(SV_DropClient)(cl, crash, "%s", buffer);

pPlayer->Disconnect();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be inside if (cl->edict)

@Arkshine Arkshine force-pushed the fix/dropclient branch 4 times, most recently from d5ec961 to f39e08d Compare July 17, 2015 17:14
@Arkshine Arkshine added the bug label Jul 18, 2015
@Arkshine Arkshine changed the title Fix player not being internally disconnected in some situation and add client_drop forward Fix player not being internally disconnected in some situation and add client_disconnected forward Jul 21, 2015
@Arkshine
Copy link
Member Author

So, after some discussions, it would be more appropriate/convenient/usable to have a replacement instead. There we go, a new forward client_disconnected which is merged with client_drop.
client_disconnect is still there for backward compatibility but is now marked as deprecated.
Any thoughts on this?

@RauliTop
Copy link

Have the commits take into account FM_ClientDisconnect forward from fakemeta?

Also, why client_disconnect is called when player disconnects or map change (1 called for each player connected) and FM_ClientDisconnect only called when player disconnects?

I'm talking about FM_ClientDisconnect in Pre

@Arkshine
Copy link
Member Author

I'm not sure why you're talking about FM_ClientDisconnect, fakemeta is unrelated here or I don't understand.

@RauliTop
Copy link

You said: "...some unexpected result, also client_disconnect won't be called."
I'm asking if FM_ClientDisconnect it's called or not too.

Also, I asked other thing, but any problem if no answer, it was only curiosity.

@Arkshine
Copy link
Member Author

If you follow #252 this is a situation where pfnClientDisconnect is not called. Essentially, if a client is connecting but has not yet spawned (meaning put in server at least) and if he crashes/timeout at this point, pfnClientDisconnect won't be called.

FM_ClientDisconnect is just a wrapper of pfnClientDisconnect engine function. But talking about another module is not really related here.

@xPaw
Copy link
Contributor

xPaw commented Aug 10, 2015

Looks fine to me now.

}
else
{
AMXXLOG_Log("client_drop forward has been disabled - check your gamedata files.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwd name and description are wrong, the forward is not fully disabled

@Arkshine Arkshine force-pushed the fix/dropclient branch 2 times, most recently from dab4daa to 15ad5bc Compare August 13, 2015 21:02
@Nextra
Copy link
Contributor

Nextra commented Aug 13, 2015

🚢

@Arkshine Arkshine force-pushed the fix/dropclient branch 2 times, most recently from e3a0d0a to ed4faf7 Compare August 13, 2015 22:08
Arkshine added a commit that referenced this pull request Aug 13, 2015
Fix player not being internally disconnected in some situation and add client_disconnected forward
@Arkshine Arkshine merged commit a9b1836 into alliedmodders:master Aug 13, 2015
@Arkshine Arkshine deleted the fix/dropclient branch August 13, 2015 22:22
Arkshine added a commit to Arkshine/amxmodx that referenced this pull request Feb 22, 2017
Introduced in alliedmodders#264.
Edict is reset once SV_DropClient is called, so that second check would be always false.
Arkshine added a commit that referenced this pull request Feb 25, 2017
…disconnected (#414)

* Rename client_disconnected to client_disconnecting

* Add client_disconnected as post forward

* Fix client not properly disconnected internally

Introduced in #264.
Edict is reset once SV_DropClient is called, so that second check would be always false.

* Reflect changes on the concerned plugins

* Revert renaming, let's add only client_remove as post forward
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants