roflmuffin / CounterStrikeSharp

CounterStrikeSharp allows you to write server plugins in C# for Counter-Strike 2/Source2/CS2
https://docs.cssharp.dev
Other
741 stars 111 forks source link

feat: improve docs and add more null checking for handle accesses #417

Closed roflmuffin closed 3 months ago

roflmuffin commented 4 months ago
SkleKng commented 4 months ago
  • GetPlayerFromIndex ensures its of type cs_player_controller but does not check connected status

Why not check connected status? What use case would there be for getting a player by index who may be disconnected? I feel like this is just going to lead to more plugin crashes due to something that should be checked by the function itself.

roflmuffin commented 4 months ago
  • GetPlayerFromIndex ensures its of type cs_player_controller but does not check connected status

Why not check connected status? What use case would there be for getting a player by index who may be disconnected? I feel like this is just going to lead to more plugin crashes due to something that should be checked by the function itself.

A disconnected player controller is still a valid entity. Like in competitive games for instance, the player is disconnected but still appears in the player list so you can interact with the schema system for them, like access player name etc.

SkleKng commented 4 months ago
  • GetPlayerFromIndex ensures its of type cs_player_controller but does not check connected status

Why not check connected status? What use case would there be for getting a player by index who may be disconnected? I feel like this is just going to lead to more plugin crashes due to something that should be checked by the function itself.

A disconnected player controller is still a valid entity. Like in competitive games for instance, the player is disconnected but still appears in the player list so you can interact with the schema system for them, like access player name etc.

Still not sure how I feel about this, but atp it's just semantics. My reasoning - if GetPlayers doesn't return disconnected players, that implies that a player must be connected. Therefore, GetPlayerFromIndex should return null if a player is disconnected. If you want to get a disconnected player, use GetEntityFromIndex. As you said, a disconnected player controller is a valid entity, but not a valid player.

roflmuffin commented 4 months ago
  • GetPlayerFromIndex ensures its of type cs_player_controller but does not check connected status

Why not check connected status? What use case would there be for getting a player by index who may be disconnected? I feel like this is just going to lead to more plugin crashes due to something that should be checked by the function itself.

A disconnected player controller is still a valid entity. Like in competitive games for instance, the player is disconnected but still appears in the player list so you can interact with the schema system for them, like access player name etc.

Still not sure how I feel about this, but atp it's just semantics. My reasoning - if GetPlayers doesn't return disconnected players, that implies that a player must be connected. Therefore, GetPlayerFromIndex should return null if a player is disconnected. If you want to get a disconnected player, use GetEntityFromIndex. As you said, a disconnected player controller is a valid entity, but not a valid player.

This functionality is not changing in this PR though, we are just using a more robust way of checking connection status, rather than using userid -1