disnake icon indicating copy to clipboard operation
disnake copied to clipboard

Fix TextChannel.clone() to copy default_auto_archive_duration property identically to Discord client

Open ChristopherJHart opened this issue 2 years ago • 6 comments

Summary

As described in #635, disnake.TextChannel.clone() previously did not copy the default_auto_archive_duration property into the new channel. This behavior differs from the behavior of the Discord client. This PR resolves this and updates the docstring of disnake.GuildChannel.clone() to indicate that the default intent of disnake.GuildChannel.clone() is to mimic the Discord client's functionality.

Checklist

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

ChristopherJHart avatar Aug 01 '22 19:08 ChristopherJHart

This should also be documented that it copies bitrate and user limit to be in sync with the client. Additionally, permission overwrites should be copied.

onerandomusername avatar Aug 02 '22 22:08 onerandomusername

Additionally, permission overwrites should be copied.

Permission overwrites are already being copied by _clone_impl, as well as channel type and name.

In any case, I double-checked, turns out the current list of copied fields includes fields that aren't cloned by the client at all - the full list (excluding obsolete store channel fields) is:

  • type
  • name
  • permissionOverwrites (filtered to only the ones the user can create)
  • bitrate
  • userLimit
  • parentId (i.e. category)

Notably, this doesn't include topic, nsfw, or rate_limit_per_user, so the current clone impl already slightly deviates from the client's. I'd argue continuing to copy all the simple fields where possible is fine, though the documentation should explicitly mention all copied fields and where it deviates from client behavior.

shiftinv avatar Aug 07 '22 16:08 shiftinv

Perhaps it would be better to add missing fields and deviate from the differences with the client, rather than mirror the client exactly (the client isn't exactly intuitive in its behaviour)

onerandomusername avatar Aug 07 '22 22:08 onerandomusername

@onerandomusername Making sure I understand what you're saying - are you saying it would be better to modify TextChannel.clone() such that it clones all fields (basically, the behavior we were proposing with a new method .full_clone() or a TextChannel.clone(full=True)) and fully deviates from the client behavior? Or am I misinterpreting what you said?

Just want to make sure I understand the exact change in behavior we'd like to see in this PR!😅

ChristopherJHart avatar Aug 14 '22 18:08 ChristopherJHart

@onerandomusername Making sure I understand what you're saying - are you saying it would be better to modify TextChannel.clone() such that it clones all fields (basically, the behavior we were proposing with a new method .full_clone() or a TextChannel.clone(full=True)) and fully deviates from the client behavior? Or am I misinterpreting what you said?

Just want to make sure I understand the exact change in behavior we'd like to see in this PR!😅

Correct, it would clone all fields that are settable when creating a new channel. It cannot fully mirror the client behavior, and currently doesn't anyways, so there's no reason to copy the client.

onerandomusername avatar Aug 14 '22 20:08 onerandomusername

@ChristopherJHart will you be updating this PR?

onerandomusername avatar Aug 28 '22 04:08 onerandomusername

Closing in favor of #837, as discussed on Discord some time ago. Regardless, thank you for the PR!

shiftinv avatar Nov 18 '22 23:11 shiftinv