Resolve #936

[Joe4evr]

Since #936 has been open for over a week now, I decided to take a stab at this. I'm pretty sure the logic is sound?

[foxbot]

Maybe I'm reading this incorrectly, but this PR seems to favor the default TypeReader over any custom TypeReaders a user would have added?

[FiniteReality]

I don't think it would, but it would be worthwhile to test just to be certain.

[Joe4evr]

@foxbot Only if TypeReader was not set before-hand by use of [OverrideTypeReader]. In that case, this consults the lib's built-in set of type readers first, and then look for a type reader added to the CommandService by the user.

[Nensec]

This seems to break overriding of the default type readers at the commandservice registration level and will always take the build in default rather than a newly registered one. I have a custom typereader for IUser (in order to do levensthein distance stuff) which is now no longer called

[Joe4evr]

@Nensec #975 and #976.

[Nensec]

#975 will not solve the issue I am facing with this change. I am using my own UserTypeReader which will call the original TypeReader for User (which I get through reflection by calling GetDefaultTypeReader() on CommandService) and only runs my code when the original type reader fails to produce a result.

var knownTypeReader = _commandService.GetDefaultTypeReader(typeof(IUser));
if (knownTypeReader == null)
    return TypeReaderResult.FromError(CommandError.ParseFailed, $"No typereader found for type {typeof(IUser)}");
else
{
    var knownParseResult = await knownTypeReader.ReadAsync(context, input, services);

    if (knownParseResult.IsSuccess)
    {
        return TypeReaderResult.FromSuccess(knownParseResult.Values);
    }
    else { ... }

The reason I am doing this is so I don't have to copy-paste the code from the original type reader. I would prefer to just inherit it so I can just call base.ReadAsync() rather than do this reflection though, it would also make #975 work as proposed.

[Joe4evr]

I am using my own UserTypeReader which will call the original TypeReader for User (which I get through reflection by calling GetDefaultTypeReader() on CommandService)

Well, you shouldn't be using reflection to get at the internal methods and classes in the first place.

I think the best thing to do is to just copy the UserTypeReader code, modify it to do your thing, and using the replace method once #975 is merged in. That way it's both faster and more correct.

Also, in the future please use codeblocks for code samples. Reading unformatted code is disgusting. 🤢

[Nensec]

Why are the typereaders internal anyway? What is the added benefit of it over making them inheritable?

[Joe4evr]

That is a good question. I also have a scenario where I want to piggyback off of an internal type reader, so maybe we should just open them up.