Skip to content

feat: add SBPP_BanPlayerBySteamId native for offline player bans#1511

Open
Sigibert wants to merge 3 commits into
sbpp:mainfrom
Sigibert:feat/ban-player-by-steamid
Open

feat: add SBPP_BanPlayerBySteamId native for offline player bans#1511
Sigibert wants to merge 3 commits into
sbpp:mainfrom
Sigibert:feat/ban-player-by-steamid

Conversation

@Sigibert

Copy link
Copy Markdown
Contributor

Summary

Adds SBPP_BanPlayerBySteamId — a new native for banning players who are no longer connected to the server.

SBPP_BanPlayer requires an in-game client index. This makes it impossible to ban players via automated systems (TK managers, anti-cheat, etc.) when the target disconnects before the ban fires. The new native accepts a SteamID string and player name directly, bypassing the need for an active client slot.

API

// sourcebanspp.inc
native void SBPP_BanPlayerBySteamId(int iAdmin, const char[] steamId, const char[] name, int iTime, const char[] sReason);
Parameter Description
iAdmin Client index of the admin issuing the ban (0 = server/automated)
steamId SteamID string in STEAM_0:X:Y format
name Player name for display in the SBPP panel; may be empty string
iTime Ban duration in minutes (0 = permanent)
sReason Ban reason

Implementation

  • Performs a duplicate-check SELECT first (same pattern as CommandAddBan) -- skips the INSERT if an active ban for that SteamID already exists
  • Inserts into sb_bans with length in seconds (iTime * 60) and ends = UNIX_TIMESTAMP() + (iTime * 60)
  • Uses IFNULL((SELECT aid ...), '0') for the admin ID lookup, matching the existing pattern
  • Falls back to a server-lookup subquery for sid when serverID == -1 (server not yet registered in SBPP)
  • Fires the existing SBPP_OnBanPlayer forward with iTarget = -1 to signal an offline ban to any listening plugins

sourcebanspp.inc changes

  • Declares the new native with a full doc comment
  • Adds MarkNativeAsOptional("SBPP_BanPlayerBySteamId") in __pl_sourcebanspp_SetNTVOptional so plugins that use this native can still load when running without SBPP

SBPP_BanPlayer requires an in-game client index, making it impossible
to ban players who disconnect before an automated ban fires. The new
native accepts a SteamID string and player name directly, allowing
plugins to record a ban in the SBPP database for players who are no
longer connected.

Implementation:
- Checks for an existing active ban first (duplicate-prevention SELECT)
- Inserts into sb_bans with length in seconds (iTime * 60) and the
  IFNULL admin-lookup subquery, matching the pattern used by CommandAddBan
- Uses a server-lookup subquery when serverID == -1 (unregistered), or
  the cached serverID directly
- Fires SBPP_OnBanPlayer with iTarget = -1 to signal an offline ban

sourcebanspp.inc:
- Declares the new native with full doc comment
- Adds MarkNativeAsOptional in __pl_sourcebanspp_SetNTVOptional
pack.WriteString(adminAuth);
pack.WriteString(adminIp);

char steamIdEscaped[MAX_AUTHID_LENGTH * 2 + 1];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need to check for steamid type since enforcement is done with SteamID2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added a SteamID2 format check to the steamId provided by the native. Hope that fits ur concerns.

SBPP enforces bans on connect using AuthId_Steam2. If a caller passed
a SteamID64 or SteamID3 the ban would be stored but never fire.
Now throws a native error immediately if the steamId argument does not
start with 'STEAM_'.

Addresses review comment from rumblefrog on PR sbpp#1511.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.


DB.Query(DB_OnBanBySteamIdSelect, query, pack, DBPrio_High);

Call_StartForward(g_hFwd_OnBanAdded);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i don't know your use case, but nit if this should be checking the insertion result before firing, otherwe lgtm

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 97ed2bf — the forward now fires from the INSERT callback (DB_OnBanBySteamIdInsert) only once results != null, instead of firing synchronously right after issuing the SELECT.

Addresses review nit on PR sbpp#1511 (rumblefrog): the forward was fired
synchronously right after issuing the async duplicate-check SELECT,
before the ban row was ever written. It now fires from
DB_OnBanBySteamIdInsert only when the INSERT query succeeds.
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

Successfully merging this pull request may close these issues.

3 participants