Skip to content

Add RemoveEventTrigger method to EventHelper#64

Merged
ifBars merged 1 commit intoifBars:stablefrom
k073l:feat/delete-event-trigger
Mar 23, 2026
Merged

Add RemoveEventTrigger method to EventHelper#64
ifBars merged 1 commit intoifBars:stablefrom
k073l:feat/delete-event-trigger

Conversation

@k073l
Copy link
Copy Markdown
Collaborator

@k073l k073l commented Mar 23, 2026

Adds a matching Remove method to already existing AddEventHelper.

This is useful in cross-backend setting, where direct removal via RemoveListener(listener, trigger.triggers[0].callback) is not possible, as Il2Cpp has issues resolving UnityEngine.EventSystems.EventTrigger.get_triggers()'.

Release Notes

  • Added RemoveEventTrigger method to EventHelper class to enable programmatic removal of event triggers
  • The method mirrors the existing AddEventTrigger functionality and addresses cross-backend scenarios where direct access to EventTrigger.triggers via Il2Cpp is unreliable
  • Supports removal of event triggers by event type with proper validation and cleanup of both Unity listener callbacks and corresponding EventTrigger entries

Changes by Author

Author Lines Added Lines Removed
k073l 23 0

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 23, 2026

📝 Walkthrough

Walkthrough

A new public method RemoveEventTrigger has been added to the EventHelper class to enable removal of event triggers. The method validates inputs, locates matching event trigger entries, and removes both the registered callback and the corresponding entry from the trigger collection.

Changes

Cohort / File(s) Summary
Event Helper Enhancement
S1API/Internal/Abstraction/EventHelper.cs
Added using System.Linq; and introduced public method RemoveEventTrigger for unregistering event callbacks. Method validates parameters, searches for matching EventTrigger.Entry by type, removes the listener callback, and deletes the entry from the trigger collection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A trigger once bound, now set free,
With a hop and a skip, we delete with glee,
Event listeners vanish without a trace,
The Helper cleans up with LINQ's embrace!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a RemoveEventTrigger method to EventHelper, matching the PR's primary objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 70.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@S1API/Internal/Abstraction/EventHelper.cs`:
- Around line 115-122: The removal currently finds an Entry by eventID only
(FirstOrDefault(e => e.eventID == eventType)) which can pick the wrong Entry
when multiple listeners share the same eventType; update Remove logic in
EventHelper.cs to identify the exact Entry added for that listener by either (A)
matching both eventID and the callback delegate (verify entry.callback's
invocation list contains the wrapped delegate corresponding to the listener) or
(B, preferred) modify AddEventTrigger to store the created Entry reference keyed
by the original listener (e.g., a Dictionary<listener, Entry>) and then in the
removal use that stored Entry to call RemoveListener(entry.callback) and remove
that exact Entry from trigger.triggers; use symbols AddEventTrigger,
RemoveListener, Entry, trigger.triggers, eventID, entry.callback, and listener
to locate and implement the change.
- Around line 116-118: The preprocessor check around the _items access currently
uses only IL2CPPMELON; change the directive in EventHelper.cs (the block that
manipulates trigger.triggers via _items) from "#if IL2CPPMELON" to "#if
(IL2CPPMELON || IL2CPPBEPINEX)" (and keep the matching `#endif`) so IL2CPPBEPINEX
receives the same IL2CPP-specific accessor workaround as other blocks in the
file.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 352402fd-5d6e-4f77-9a4e-006b1d3ccc68

📥 Commits

Reviewing files that changed from the base of the PR and between d40d543 and 49e5088.

📒 Files selected for processing (1)
  • S1API/Internal/Abstraction/EventHelper.cs

@k073l k073l requested a review from ifBars March 23, 2026 14:00
@ifBars ifBars self-assigned this Mar 23, 2026
@ifBars ifBars merged commit e23a4b0 into ifBars:stable Mar 23, 2026
5 checks passed
@ifBars ifBars deleted the feat/delete-event-trigger branch March 23, 2026 21:52
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.

2 participants