Fix invalid client index in Basebans menu

[Rainyan]

Fix #1768

The sm_admin-triggered Menu flow for banning players is caching client indices inside the basebans.sp PlayerInfo struct, and can then incorrectly use them in the menu callback without checking for the related client's UserId validity. This leads to bug #1768 occurring when the ban target player disconnects from the server before the banning admin could complete the banmenu UI flow.

Since the related menu callbacks can't rely on the cached client index, I have removed the basebans.sp PlayerInfo.banTarget member entirely, in favor of the PlayerInfo.banTargetUserId, and instead call GetClientOfUserId(...) to get & validate the target client index where necessary. The PrepareBan(...) function has been refactored to take a ban target UserId (instead of the target client index) accordingly.

---

To reproduce this bug, follow these steps:

• Admin joins server
• "Cheater"/undesired player joins server
  • You can debug this with bot clients if you temporarily delete the COMMAND_FILTER_NO_BOTS in basebans/ban.sp (so that the ban player list can be populated by fakeclients)
• Admin opens the sm_admin -> "Player Commmands" -> "Ban Player" menu
• Admin selects the cheater
• Cheater disconnects
• Admin selects the ban duration, and/or reason for ban
• Server throws the error as described by Basebans client index is invalid #1768

[peace-maker]

Thank you, this should have been this way from the beginning ❤️

[peace-maker]

I think the best fix would be to use BanIdentity if the player left instead of failing the ban, but that can be done in a different PR if you don't want to work on it here.

[Rainyan]

I think the best fix would be to use BanIdentity if the player left instead of failing the ban, but that can be done in a different PR if you don't want to work on it here.

Thanks for the review. I can work the suggested changes into this PR if that seems fine.

[peace-maker]

Yes, adding it to this one is cool. You could even look if the player left and rejoined again while the menu was open. I don't think BanIdentity would kick the player in that case.

[Rainyan]

I've added the BanIdentity SteamID banning route for disconnected targets.

I've also changed the ban menu player items formatting to Player Name (AUTHID) from Player Name (USERID), for a more uniform look, using the custom AddTargetsToMenuByAuthId, which mimics the native AddTargetsToMenu2, except for tracking players by authid rather than userid. Or, alternatively, I could keep the old userid formatting for the ban menu visual look, even though it's somewhat misleading, as the ban would not happen by the expired userid for any disconnected client?

I still need to refactor the PrepareBan to refer to the struct array directly, and add kicking for reconnected targets who would dodge the userid kick before this could be considered for merging. Edit: done

The PrepareBan logic now follows:

• Does the userid resolve to a valid client index? -> BanClient(index of userid)
• Else, is there a client with the ban target authid connected? -> BanClient(index of authid)
• Else -> BanIdentity(authid)