Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore overlayplugin data, but have it defined in ActEventType #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nonowazu
Copy link
Contributor

@nonowazu nonowazu commented Dec 12, 2022

While it's set to noop now, having it defined keeps us from getting raised events when raise_on_invalid_id is True

For more information in this addition, see cactbot docs

@ayyaruq
Copy link
Contributor

ayyaruq commented Dec 12, 2022

Alternative idea: could we instead generic >255 is an extended event and ignore that way? I feel like there's a difference between intentionally no-oping ACT events, and ignoring extensions that aren't part of ACT in the first place. We also have absolutely no guarantee that 256 is purely for OverlayPlugin - any other plugin could recycle the same ID if they wanted to.

@nonowazu
Copy link
Contributor Author

If we nooped like that, it would have to go in the parser before we check to see if the ID is in the enum. We'd probably need to check to see if 256 is some "universal other" field, however – I don't like the idea that we might one day get a 257 that we actually care about and then have to rearchitect around that.

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