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

feature: Add Direction.Around to GetMessagesAsync #1526

Merged
merged 6 commits into from
Jun 18, 2020

Conversation

SubZero0
Copy link
Member

@SubZero0 SubZero0 commented May 13, 2020

Summary

This PR will add the missing implementation for Direction.Around.
If the limit is inside the max for discord, it will use "around", otherwise it'll query before and after since discord doesn't give any other ways to do it.

Changes

  • Add Direction.Around logic for cache and rest
  • Do not return empty pages if cache doesn't have any
  • Use cache for Direction.After when possible

@SubZero0 SubZero0 marked this pull request as ready for review May 13, 2020 14:58
@SubZero0
Copy link
Member Author

Did a few more tests using all directions with and without cache, seems to be working as expected now.

Copy link
Member

@foxbot foxbot left a comment

Choose a reason for hiding this comment

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

This feels like a hacky way to support this, but looks good otherwise. Thanks!

@foxbot foxbot merged commit f2130f8 into discord-net:dev Jun 18, 2020
@SubZero0 SubZero0 deleted the add-direction-around branch August 17, 2020 14:57
anonymousheadless pushed a commit to anonymousheadless/Discord.Net that referenced this pull request Mar 29, 2021
* Add Direction.Around to GetMessagesAsync

* Reuse the method

* Reuse GetMany

* Fix limit when getting from cache without message id

* Fix limit when getting from rest without message id

* Change cache return

It will return in a similar way to REST
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants