Skip to content

fix(clickgui): cancel pending bind listeners and modifier timers - #9387

Open
c8dhjp4tyv-bit wants to merge 2 commits into
CCBlueX:nextgenfrom
c8dhjp4tyv-bit:fix/bind-listener-lifecycle
Open

c8dhjp4tyv-bit wants to merge 2 commits into
CCBlueX:nextgenfrom
c8dhjp4tyv-bit:fix/bind-listener-lifecycle

Conversation

@c8dhjp4tyv-bit

Copy link
Copy Markdown
Contributor

Canceling a bind after pressing a modifier leaves input listeners and its timeout alive. Restarting immediately lets the old timeout finish the new session. Successful input also leaves the unused keyboard or mouse waiter subscribed, and component destruction does not cancel these waits.

Add optional AbortSignal support to waitMatches. Give each bind session its own controller, cancel the losing waiter, and clear listeners, timers and modifier state on completion, cancellation or destruction. Guard asynchronous continuations with the session's signal.

Validation:

  • cd src-theme && node --test tests/bind-lifecycle.test.mjs: eight tests pass; five fail on the original implementation. No new dependencies.
  • Full Gradle build, Detekt, theme build and client game tests passed on a separate branch combining the three contributions.
  • npm run check reports the same 21 existing errors in seven files on the base revision and this branch, with no additional diagnostics.

https://github.com/c8dhjp4tyv-bit/LiquidBounce/actions/runs/38117439329

@liquidbounce-support-bot

Copy link
Copy Markdown
Contributor

Thanks for the contribution and for including tests that fail on the old implementation.

A couple of small things a reviewer will probably bring up:

  • tests/bind-lifecycle.test.mjs isn't hooked into any package.json script or CI step, so as things stand it only runs when someone starts it by hand. It might be worth adding a test script, or saying in the PR whether the test is meant to stay in the repo.
  • The test pulls the component's script out with a regex on <script lang="ts"> and swaps in the export let setting line as a plain string. Any formatting change in BindSetting.svelte will break it, so a short comment saying that would help whoever runs into it later.
  • The deleteListener change (removing the map entry once it has no handlers left) goes a bit beyond the bind lifecycle fix. Calling it out in the description would make review easier.

No user-facing behaviour changes, so the docs shouldn't need updating. A maintainer will do the actual review.


🤖 Automated support reply — a human maintainer will review if this doesn't help.

Add npm test and run it after the theme build in verify-pr. Document the script-only fixture and fail clearly when its source matches need updating.
@MukjepScarlet
MukjepScarlet requested review from 1zun4 and SenkJu and removed request for SenkJu October 11, 2026 08:26

This branch has not been deployed

No deployments
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.

1 participant