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

[Bug]: BaseSocketClient and DiscordRestClient do not properly implement [Feature] Premium Subscriptions #2808

Open
3 tasks done
Panthr75 opened this issue Dec 7, 2023 · 4 comments
Assignees

Comments

@Panthr75
Copy link
Contributor

Panthr75 commented Dec 7, 2023

Check The Docs

  • I double checked the docs and couldn't find any useful information.

Verify Issue Source

  • I verified the issue was caused by Discord.Net.

Check your intents

  • I double checked that I have the required intents.

Description

DiscordRestClient and BaseSocketClient were modified to include the interface implementation override for member Task<IEntitlement> IDiscordClient.CreateTestEntitlementAsync(ulong, ulong, SubscriptionOwnerType, RequestOptions?), but not for:

  • Task IDiscordClient.DeleteTestEntitlementAsync(ulong, RequestOptions?)
  • IAsyncEnumerable<IReadOnlyCollection<IEntitlement>> IDiscordClient.GetEntitlementsAsync(int?, ulong?, ulong?, bool, ulong?, ulong?, ulong[]?, RequestOptions?)
  • Task<IReadOnlyCollection<SKU>> IDiscordClient.GetSKUsAsync(RequestOptions?)

This means that when using an object of type IDiscordClient and calling one of those non-overriden members, you will get unexpected behaviour, as it will use the implementation inside BaseDiscordClient which returns null pretty much.

This isn't the first time this has happened: 8baf913

Maybe some automated tooling needs to be put into place so that BaseSocketClient and DiscordRestClient properly override the default BaseDiscordClient implementation, or a reconsideration of how these classes are structured may need to take place.

Version

3.13.0

Working Version

3.12.0

Logs

N/A

Sample

DiscordRestClient restClient = new();
// initialization and loading...

// works fine.
_ = await restClient.GetSKUsAsync();

IDiscordClient interfaceClient = restClient;

// wont work fine, calls `BaseDiscordClient.IDiscordClient.GetSKUsAsync(RequestOptions?)` instead of `DiscordRestClient.GetSKUsAsync(RequestOptions?)`
_ = await interfaceClient.GetSKUsAsync();

Packages

N/A

Environment

N/A

@Panthr75 Panthr75 added the bug label Dec 7, 2023
@Misha-133 Misha-133 self-assigned this Dec 7, 2023
@Panthr75
Copy link
Contributor Author

Panthr75 commented Dec 8, 2023

Maybe I could make a source analyzer/generator for this?
We could also enforce some style rules as well, so it's a win/win.

@Misha-133
Copy link
Member

Maybe I could make a source analyzer/generator for this? We could also enforce some style rules as well, so it's a win/win.

cc @quinchs

@Misha-133
Copy link
Member

Misha-133 commented Dec 11, 2023

@Panthr75 there's no need to override these interface members.
In your example

_ = await interfaceClient.GetSKUsAsync();

will actually call the method implementation from DiscordRestClient, not BaseDiscordClient
Same applies to other methods you've mentioned.

You an easily verify it by placing breakpoints in DiscordRestClient.

@Misha-133 Misha-133 removed the bug label Dec 11, 2023
@Misha-133
Copy link
Member

Removing the bug label, but will leave the issue open
A source analyzer for ensuring that all IDiscordClient methods are actually implemented in DiscordRest/SocketClient would be nice IMO.

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

No branches or pull requests

2 participants