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

feat: Polls #2408

Open
wants to merge 125 commits into
base: master
Choose a base branch
from
Open

feat: Polls #2408

wants to merge 125 commits into from

Conversation

NeloBlivion
Copy link
Member

@NeloBlivion NeloBlivion commented Mar 21, 2024

Summary

Implements polls and related features. WIP while the original PR becomes more stable over the experimental phase.
References:

Notes:

  • While the API for PollMedia denotes both text and emoji as optional, text must be non-null for both questions and answers; as such, it's required here.
  • For Poll and PollAnswer, PollMedia is abstracted to its attributes (e.g. Poll.PollMedia.text is assigned to Poll.question). An undocumented attribute _media still remains. After reconsidering, this wasn't great. For __init__s you can just pass the strings, but otherwise you'd access through Poll.question.text, for example.
  • Will work on a new AsyncIterator for answer.users Complete

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

discord/abc.py Outdated Show resolved Hide resolved
discord/enums.py Outdated Show resolved Hide resolved
discord/raw_models.py Outdated Show resolved Hide resolved
discord/state.py Show resolved Hide resolved
@plun1331 plun1331 added priority: medium Medium Priority status: in progress Work in Progess feature Implements a feature upcoming discord feature Involves a feature not yet fully released by Discord labels Mar 22, 2024
@NeloBlivion
Copy link
Member Author

NeloBlivion commented Apr 18, 2024

As the main PR is merged, this is effectively ready

  • poll can be sent with content and embed(s), but not file(s) due to clashes with upcoming poll features that are undocumented (unless i should implement these early?)
  • poll cannot be edited after being sent, but content and embeds on the same message can.
  • New Intents.polls and Permissions.send_polls
  • While polls rely on message_content intent, they are cached separately through Client.polls and Client.get_poll. Polls will be cached through the following:
    • Message sends
    • ANY message edit
    • Any form of message fetch - e.g. channel.fetch_message for individual messages, or channel.history will get several polls
  • on_message_edit will fire when a poll ends (naturally, or Message.end_poll(), or poll.end() AND the votes have been finalized.
  • Intents.polls is a default intent that is required for tracking poll votes through on_poll_vote_add and on_poll_vote_remove, both using the args poll, user, answer

Now for the creation of polls, I wasn't entirely sure on the best approach. question only supports text, so it can be initialized as follows:

  • Poll(some_question)
  • Poll(question=PollMedia(some_question))

But answers are more complicated, due to supporting text, emoji and soon attachment. So you can either pass answers as a list of PollAnswer, or use Poll.add_answer chaining like you would with embeds:

  • Poll(question=some_question, answers=[PollAnswer(some_answer), PollAnswer(another_answer, emoji=...)])
  • Poll(some_question).add_answer(some_answer)

Due to the extensiveness of the polls API, I feel this is one of those instances where it's beneficial to add extra convenience for the user instead of sticking to it religiously.
The other args are completely optional, using the defaults present on Discord's UI when creating polls (allow_multiselect=False, duration=24, no other layout_type yet.)
Due to results being nullable for some reason at random, a few conveniences have been added:

  • poll.results.is_finalized -> poll.has_ended()
  • poll.results.total_votes() -> poll.total_votes()
  • Answer counts are typically stored in poll.results.answer_counts, but each PollAnswer has the count attribute; returning None if results is also None, 0 if the answer isn't in answer_counts, otherwise the relevant count.

If you are able to, please stress test this PR and see if the internally cached vote counts are able to keep up with realtime counts on Discord, especially when bouncing between 1-0 votes on specific answers; this part involves manual deletion and creation of PollAnswerCount objects, so it may be possible for something to go wrong that I couldn't catch in solo tests.

@Lulalaby Lulalaby self-requested a review April 18, 2024 21:50
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

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

Changelog

Copy link
Member

@JustaSqu1d JustaSqu1d left a comment

Choose a reason for hiding this comment

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

documentation suggestions

  • fix minor mistakes
  • more formal writing
  • formatting

docs/api/events.rst Outdated Show resolved Hide resolved
docs/api/events.rst Outdated Show resolved Hide resolved
docs/api/events.rst Outdated Show resolved Hide resolved
docs/api/events.rst Outdated Show resolved Hide resolved
docs/api/events.rst Outdated Show resolved Hide resolved
discord/poll.py Outdated Show resolved Hide resolved
discord/poll.py Outdated Show resolved Hide resolved
"""Add an answer to this poll.

This function returns the class instance to allow for fluent-style
chaining.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add an example?

discord/poll.py Outdated Show resolved Hide resolved
discord/raw_models.py Outdated Show resolved Hide resolved
NeloBlivion and others added 4 commits April 23, 2024 18:34
Co-authored-by: JustaSqu1d <89910983+JustaSqu1d@users.noreply.github.com>
Signed-off-by: UK <41271523+NeloBlivion@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog needed feature Implements a feature priority: medium Medium Priority status: in progress Work in Progess upcoming discord feature Involves a feature not yet fully released by Discord
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants