Skip to content

fix: validate emote ownership on keybind-triggered emotes - #2867

Closed
FelipeDaros wants to merge 1 commit into
QuentinGruber:devfrom
FelipeDaros:fix/emote-effect-ownership-check
Closed

FelipeDaros wants to merge 1 commit into
QuentinGruber:devfrom
FelipeDaros:fix/emote-effect-ownership-check

Conversation

@FelipeDaros

@FelipeDaros FelipeDaros commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Command.PlayDialogEffect (sent by the client when an emote keybind is pressed) only validated that the player had fists/hands equipped, without checking whether the player actually owns the emote.
  • Non-default emotes (e.g. granted from cases) could be triggered with any effectId by a modified client, bypassing the ownership check already enforced by the /emote chat command.
  • This mirrors the same account-item ownership check used by /emote in handlers/commands/commands.ts, applying it to the keybind path as well.

🤖 This PR was developed with the assistance of AI (Claude Code).

…ffect

Non-default emotes (e.g. granted from cases) sent via the emote keybind
packet were not checked against the player's account items, unlike the
/emote chat command. A modified client could trigger any effectId. Mirror
the ownership check already used by the /emote command.
@FelipeDaros FelipeDaros mentioned this pull request Aug 7, 2026
@johnoclockdk

Copy link
Copy Markdown
Collaborator

This is a no from me for the following reasons:

  1. This was implemented using AI. Please disclose AI usage in the next PR.
  2. It doesn’t look like this has been properly tested, as the emote keybind is still not being sent to the server.

@Z1Meme

Z1Meme commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Closing this pull request, no packet is currently being sent from the client when hitting emote keybinds, which is the core issue. This PR is just adding server logic for clientside functionality that has not yet been unlocked.

@Z1Meme Z1Meme closed this Aug 18, 2026
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