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

[webhook] webhook for custom filter #116

Merged
merged 7 commits into from
Nov 9, 2023

Conversation

nmurillon
Copy link

Goal of the PR

Add a webhook for custom filters

How to test?

  • Set enable to true in the section custom_filter_pokemon_encounter of the file discord.yml
  • Enable some of the custom filters in custom_catch_filters.py

Note:
There is a new way to enable custom filters. Instead of returning a boolean, you can return a CustomFilterMatched which allows to know exactly which filter matched. You can return CustomFilterMatched.NONE in case no filters matched, or CustomFilterMatched.UNKNOWN, if you don't want to have the details

Results you can get from this

image
image

Impacts

Changing the return type from a boolean to a CustomFilterMatched may cause some breaking changes if some people already have custom filters for a given profile. Let me know what you think about it.

This also impacts #108 as the discord schema has been updated

@40Cakes
Copy link
Owner

40Cakes commented Nov 5, 2023

Hi mate, this looks fantastic, thank you for taking the time to work on this.

I am going to push a few changes, I really hope you don't mind (or are offended) but I would appreciate your thoughts on it as well - it's just that I often deal with users in the support channel that struggle to modify the simplest thing, even just commenting a single line out, and to be real I don't wanna be explaining what enums are to them 😭

Instead I think it'd be simpler for end users to just be able to return any string of their choosing, and the bot can detect if it was a str returned or a False bool, accept any str as a match.

Also custom filters were being run twice for each encounter, I've just modified it to pass down the result to whatever needs it instead of calling it again.

@nmurillon
Copy link
Author

I am going to push a few changes, I really hope you don't mind (or are offended)

Sure, that's the goal of the PR after all, to discuss about the changes made, what could be done differently, etc... :)
Let me know if you want me to perform the changes, I will have some free time coming today and next week

@40Cakes
Copy link
Owner

40Cakes commented Nov 5, 2023

I am going to push a few changes, I really hope you don't mind (or are offended)

Sure, that's the goal of the PR after all, to discuss about the changes made, what could be done differently, etc... :) Let me know if you want me to perform the changes, I will have some free time coming today and next week

Pushed, no rush! 😄

@40Cakes 40Cakes merged commit 473271f into 40Cakes:main Nov 9, 2023
hanzi added a commit to hanzi/pokebot-gen3 that referenced this pull request Dec 1, 2023
Fixes 40Cakes#116

If the stats system cannot initialise for some reason, it catches _all_ errors, suppresses them and then just exits the process.

Most of the time that probably happened due to messed up `custom_hooks` or `custom_catch_filters` files (containing syntax errors or other problems that manifest on import.)

I have removed that catch-all handler entirely since Python's default behaviour should already be to just exit whenever there is an exception -- but at least now, there will be an error message.

I have searched the code base for similar snippets and found another one in `write_symbol()`, which I have also updated.
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