discord-net / Discord.Net

An unofficial .Net wrapper for the Discord API (https://discord.com/)
https://discordnet.dev
MIT License
3.34k stars 736 forks source link

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

Open Panthr75 opened 11 months ago

Panthr75 commented 11 months ago

Check The Docs

Verify Issue Source

Check your 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:

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: https://github.com/discord-net/Discord.Net/commit/8baf913c9d4e5504384062c8d74fe8f3f70efcd0

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

@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 commented 11 months ago

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.