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

Change priority to user type readers and implement user entity readers #1487

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

SubZero0
Copy link
Member

@SubZero0 SubZero0 commented Apr 20, 2020

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 OverrideTypeReaderAttribute incorrectly applies to other parameters of the same type #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.

@monoclex
Copy link

Bump - any status updates on this?

@SubZero0
Copy link
Member Author

SubZero0 commented Jun 26, 2020

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
Copy link
Member Author

Tagging @Still34 for review 🙃

@FiniteReality
Copy link
Member

Fix the build errors first.

@SubZero0
Copy link
Member Author

Done, missed a bracket in the example.

@Still34 Still34 self-requested a review June 27, 2020 05:47
src/Discord.Net.Commands/CommandService.cs Outdated Show resolved Hide resolved
src/Discord.Net.Commands/CommandService.cs Outdated Show resolved Hide resolved
src/Discord.Net.Commands/CommandService.cs Show resolved Hide resolved
src/Discord.Net.Commands/CommandService.cs Outdated Show resolved Hide resolved
@quinchs
Copy link
Member

quinchs commented Feb 3, 2022

@Cenngo Can you look at this?

@Cenngo
Copy link
Collaborator

Cenngo commented Feb 3, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants