Discord.Net icon indicating copy to clipboard operation
Discord.Net copied to clipboard

Change priority to user type readers and implement user entity readers

Open SubZero0 opened this issue 4 years ago • 7 comments

Summary

This change will rollback the change from #941 that gave priority to default type readers instead of the ones added by the user while also fixing the issue that PR was merged for by creating a specific list for override type readers (so it isn't added like a global one).

It will also give the user the ability to add entity type readers of their own (example: a global IChannel type reader that applies to IMessageChannel etc, like the default one).

Changes

  • Change priority of type readers (user > default).
  • Add internal property to type readers to signal a override one and prevent #936 .
  • Add AddEntityTypeReader<T>(Type typeReaderGenericType) and AddEntityTypeReader(Type type, Type typeReaderGenericType) to CommandService
  • Add logic for entity type readers added by the user (adding a IChannel type reader will apply it to classes/interfaces that implement it, example: IMessageChannel).
  • Deprecate AddTypeReader<T>(TypeReader reader, bool replaceDefault) and AddTypeReader(Type type, TypeReader reader, bool replaceDefault) (not needed anymore since default type readers don't have priority over the ones added by the user.

How to use

  1. Create a class that inherits from TypeReader with a single generic argument that has at least a reference type constraint, e.g. class CustomUserTypeReader<T> : TypeReader where T : class, IUser
  2. Add it to the command service, e.g. AddEntityTypeReader<IUser>(typeof(CustomUserTypeReader<>))
  3. Done!

Examples (with test cases done)

The examples will say what type reader were added (following the order that they were added), the argument used, and the type reader assigned to them. Following this format: Argument type: Type Reader used. Note: default = default Discord.Net type reader, otherwise it's the one added by the user.

Adding IChannel type reader and IMessageChannel type reader

  • IChannel: IChannel
  • IMessageChannel: IMessageChannel
  • ITextChannel: IMessageChannel
  • SocketTextChannel: IMessageChannel
  • IVoiceChannel: IChannel
  • SocketChannel: IChannel

Adding only IMessageChannel type reader

  • IChannel: default
  • IMessageChannel: IMessageChannel
  • ITextChannel: IMessageChannel
  • SocketTextChannel: IMessageChannel
  • IVoiceChannel: default
  • SocketChannel: default

Adding IVoiceChannel type reader and IMessageChannel type reader

  • IChannel: default
  • IMessageChannel: IMessageChannel
  • ITextChannel: IMessageChannel
  • SocketTextChannel: IMessageChannel
  • IVoiceChannel: IVoiceChannel
  • SocketChannel: default

Notes

This PR includes the change in #1486, so if accepted, please merge that one first since they are the one that fixed and deserve the credit for finding it.

If you have any test case that you want to be done, please comment it here and I can do it since this change might have something I didn't expect and didn't test.

SubZero0 avatar Apr 20 '20 10:04 SubZero0

Bump - any status updates on this?

monoclex avatar May 31 '20 00:05 monoclex

Had to change the way to add entity readers since there's no way for type readers to know what kind of object is being expected to be returned. This could cause an unexpected error when converting types (like a SocketGuildUser to a RestGuildUser).

Now they will need to be added with AddEntityTypeReader that has 2 overloads:

  • AddEntityTypeReader<T>(Type typeReaderGenericType)
  • AddEntityTypeReader(Type type, Type typeReaderGenericType)

This will make the creation of entity type readers a bit more complex so they can know what kind of Type is being expected.

Entity type readers will need to inherit from TypeReader (like usual) but will required to have a single generic argument that has at least a reference type constraint, e.g. class CustomUserTypeReader<T> : TypeReader where T : class, IUser.

To add it to the command service, it'll be needed to pass the generic type definition of the TypeReader, e.g. AddEntityTypeReader<IUser>(typeof(CustomUserTypeReader<>)).

Another way would be adding a Type outputType to TypeReader.ReadAsync, but this would require breaking all current implementations of all type readers when updating, so I chose to just add new methods instead.

SubZero0 avatar Jun 26 '20 16:06 SubZero0

Tagging @Still34 for review 🙃

SubZero0 avatar Jun 26 '20 20:06 SubZero0

Fix the build errors first.

FiniteReality avatar Jun 26 '20 20:06 FiniteReality

Done, missed a bracket in the example.

SubZero0 avatar Jun 26 '20 20:06 SubZero0

@Cenngo Can you look at this?

quinchs avatar Feb 03 '22 18:02 quinchs

@Cenngo Can you look at this?

I need to go over this to see whats the state of this pr but ill try to wrap this up.

Cenngo avatar Feb 03 '22 20:02 Cenngo