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

Merge changes from current #190

Merged
merged 8 commits into from
Sep 14, 2023
Merged

Merge changes from current #190

merged 8 commits into from
Sep 14, 2023

Conversation

scottbot95
Copy link
Contributor

  • Merge in the changes from current branch.
  • Remove deprecated methods from CooldownTracker (I did this as a sperate commit so it would be really easy to make it a separate PR if you would prefer that)
  • Updated Command to new CooldownTracker methods. I included this on the next branch since it seems like it's somewhat of a breaking change since it's adding a field to Command and people shouldn't be making those on their own but they could be

sadorowo and others added 7 commits May 10, 2023 23:02
* Add Infallible to __NonExhaustive enum variants

* add: subcommmand_required

* Fix a few things

- The code was not properly formatted (`cargo fmt --all`)
- There was a redundant is_subcommand value added to a bunch of things (including find_command - this PR was a breaking change and would have belonged to the `next` branch! But since is_subcommand was redundant and I removed it, this PR becomes not-breaking again (which makes sense; this kind of small feature shouldn't need breaking changes))
- The is_subcommand check was in the wrong place. The parse_invocation is really only for parsing, not for any action. If it returns an error, it should be an error in _parsing_, not in execution or usage. And it also belongs after the track_edits related checks, because if a message was edited that poise isn't even allowed to care about, it shouldn't throw an error too (this would have been another problem with the is_subcommand check in parse_invocation)

* Improve text

---------

Co-authored-by: kangalioo <[email protected]>
Co-authored-by: xtfdfr <[email protected]>
* feat: added support for responding to an mci with a modal

* feat: added example, simplified modal send as mci interface

* fix: fixed example

* Some changes ™

- Move component modal example into modal.rs instead of making new file
- Make component modal example prefix_command too to catch any oversights that may make component modals rely on a slash command interaction being present (e.g. previously, execute_component_interaction took ApplicationContext and couldn't be invoked in prefix context at all)
- Deduplicate code between execute_modal and execute_component_interaction (and rename the latter to be clearer)
- Remove Modal::execute_component_interaction function in favor of free-standing execute_modal_on_component_interaction function - I want to avoid an explosion of utility trait functions for any combination of defaults yes/no, component yes/no... That said, I realize we're already halfway in this mess with execute and execute_with_defaults both existing so I'd also be open to go all the way and add execute_component_interaction and execute_component_interaction_with_defaults
- Add `impl AsRef<serenity::Context> for poise::Context` to make free-standing execute_modal_on_component_interaction function work well; and because it makes sense in general to have

---------

Co-authored-by: kangalioo <[email protected]>
* fix the behavior of poise::reply::builder::CreateReply.reply

* unnecessary change by mistake

* change back to original

* add reply()

* remove reply_reply()

* Adhere to unwritten codebase conventions

Made on phone in GitHub's absolutely janky web UI, and untested. Let's see how this goes

---------

Co-authored-by: kangalio <[email protected]>
* Bump `tokio` to `1.25.1`

* Bump `env_logger` to `1.10.0`

* Bump `tokio` to `1.25.1`
That way, you can have different cooldown durations depending on which user / guild / whatever is invoking the command
- Renames `CooldownTracker::new_2` and `CooldownTracker::remaining_cooldown_2` to `CooldownTracker::new` and `CooldownTracker::remaining_cooldown` respectively.
- Adds a new `cooldown_config` field to `Command` wrapped in a Mutex to allow updating by the user
- Adds a new example using a command check to modify the `cooldown_config` on the `Command`
- updates `#[poise::command]` to new `Command` structure
@scottbot95
Copy link
Contributor Author

Hmmm the error seems to be caused by f9fce51 but the error was masked in current due to the unrelated failures

Needed for dependency updates in serenity-rs#186 that
were done to resolve some `cargo audit`
vulnerabilities
@scottbot95
Copy link
Contributor Author

scottbot95 commented Aug 12, 2023

Oops pushed more changes prematurely to my forked next branch.

Bumped MSRV to 1.63 to allow build to succeed. Although the doc build is failing now for reasons? The if: github.ref == 'refs/heads/current' || github.ref == 'refs/heads/next' check seems to be getting confused by the fact that my fork's branch name is also next. But that seems like a spec violation by github so... 🤷

Edit: Oh nvm I'm just confusing myself but looking at builds that happened on my fork. I didn't realize Github ran actions on forks. Perhaps I turned that on at some point?

@scottbot95 scottbot95 marked this pull request as ready for review August 12, 2023 02:40
@kangalio kangalio merged commit cca5c13 into serenity-rs:next Sep 14, 2023
11 checks passed
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.

6 participants