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(interactions): deserialize channel from data #1012

Merged
merged 18 commits into from
Aug 24, 2024

Conversation

shiftinv
Copy link
Member

Summary

Implements discord/discord-api-docs@88a7618, deserializing the channel field from the interaction payload and no longer using the channel_id field.

This has no effect for channels which are already cached, but it now provides a proper channel object in cases where it isn't (no bot in guild, private threads, ...).

A few comments on safety

The continued assumption is that all non-PING interactions contain this channel field, which is what we're already assuming for channel_id. However, this is not documented.
Making the channel field optional in the future is definitely worth consideration, imo, but for now it might be too much of a breaking change in terms of typing.

Similarly, we expect some fields to exist in this channel field, particularly id + type (and name + parent_id + thread_metadata in the case of threads)
While all these are documented to exist in interaction.resolved.channels, this might not be the case for interaction.channel, which really only guarantees id and type.
In the end it's probably a fair assumption, but still something we should improve in the future.

related thread

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running pdm lint
    • I have type-checked the code by running pdm pyright
  • 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, ...)

@shiftinv shiftinv added t: enhancement New feature t: api support Support of Discord API features s: needs review Issue/PR is awaiting reviews labels Apr 24, 2023
@onerandomusername onerandomusername force-pushed the feature/interaction-channel branch from 958443b to fd14756 Compare May 7, 2023 00:15
@shiftinv shiftinv force-pushed the feature/interaction-channel branch from fd14756 to be8b678 Compare May 7, 2023 10:28
@onerandomusername
Copy link
Member

The continued assumption is that all non-PING interactions contain this channel field, which is what we're already assuming for channel_id. However, this is not documented.

I really don't like this assumption, and would like some fallback to current behavior in the event that the channel data is not present (it was already rolled back once for breaking d.js).

@shiftinv
Copy link
Member Author

shiftinv commented Jul 5, 2023

Looks like the assumption that parent_id is included in threads doesn't always hold for interaction.channel, unlike interaction.data.resolved.channels: discord/discord-api-docs#6270

I can't reproduce it, but I was afraid this might happen. It makes this field a lot more annoying to work with; not sure how to handle this properly right now, I don't want to just slap a try/except around it.

edit: looked into it further, the linked issue may be slightly inaccurate and we actually do get this data here

@shiftinv shiftinv mentioned this pull request Mar 23, 2024
8 tasks
shiftinv and others added 8 commits August 21, 2024 20:51
I don't think there are even cases where `resolved["messages"]` can contain a message
that *isn't* in the current channel, so `channel_id == parent.channel.id` should always match.
Still, we fall back to `guild.get_channel()` in case it doesn't.

Additionally, we also fall back in the same way to `state.get_channel()` to make use of the DM cache if `parent.channel` is
a `PartialMessageable`, since that means it's a dm/group channel, for which support will be added in another PR.
@shiftinv shiftinv merged commit 975657a into master Aug 24, 2024
28 checks passed
@shiftinv shiftinv deleted the feature/interaction-channel branch August 24, 2024 11:55
@shiftinv shiftinv added this to the disnake v2.10 milestone Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: needs review Issue/PR is awaiting reviews t: api support Support of Discord API features t: enhancement New feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants