Fix/v4/slashies cleanup #1362

OoLunar · 2022-08-01T21:20:47Z

Rewrites the internals of Slash Commands. There are no public API changes, however a few methods have been Obsoleted with alternatives suggested in the attribute message. Hopefully once this rewrite is done I'll be able to add functionality very similar to CNext, such as retrieving a list of registered commands.

Help Wanted: Please review my code or optionally make a PR to the fix/v4/slashies-cleanup branch with your new code.

EDIT: Closes #1222 due to internal breaking changes.

… \/])/\1 \2/g" {} \;

…<T> and `dotnet format` with required file headers

Fix broken links that weren't caused by my perfect regex

* clean up attribute retrieval * clean up futher, unify more things

OneYellowLemon · 2022-08-18T17:51:53Z

dbb94db broke URLs, should probably be fixed. Should I submit a PR or is that too minor?

OoLunar · 2022-08-18T17:56:08Z

I have it fixed locally, dw

OoLunar · 2022-08-18T19:09:54Z

DSharpPlus/DSharpPlus.SlashCommands/Attributes/SlashModuleLifespanAttribute.cs

Line 70 in f66f9cf

Singleton,

This is a breaking change due to the enum member being reordered.

akiraveliara · 2022-08-18T19:14:38Z

i feel a strong urge to fix that

OoLunar · 2022-08-18T19:15:19Z

It was done so that it can be correctly casted to LifetimeService- actually you're right it can be fixed

Tawmy · 2022-12-13T12:27:25Z

Does this close #1367 as mentioned in that comment of yours, @OoLunar? Just making sure it's not being forgotten about.

OoLunar · 2022-12-13T13:54:46Z

I can't say I remember explicitly patching it, however this PR isn't finished (hence the draft). When I pick this PR back up, I'll take a look

Tawmy · 2022-12-13T13:58:23Z

What's the exact status of the PR anyways? Do you want it to be tested now or would you like to work on it before testing begins? If the prior I can probably make some time today or tomorrow to do some testing.

OoLunar · 2022-12-13T14:11:55Z

Tests very much need to be done. As far as I remember, registering custom converters and all application command types is all that's required. Make sure you run them too as parameter resolving was reworked. Nullable and enums should work as intended, but if you want to test those too, go at it

Tawmy · 2022-12-13T18:55:38Z

QA [WIP]:

Command types:

User context menu
Message context menu
Slash commands

Converters for slash commands:

Boolean
- Non-nullable
- Nullable
- Nullable with optional value
Byte
- Non-nullable
- Nullable
- Nullable with optional value
DiscordAttachment
- Non-nullable
- Nullable
- Nullable with optional value
DiscordChannel
- Non-nullable
- Nullable
- Nullable with optional value
DiscordEmoji
- Non-nullable
- Nullable
- Nullable with optional value
DiscordMember
- Non-nullable
- Nullable
- Nullable with optional value
DiscordMessage
- Non-nullable
- Nullable
- Nullable with optional value
DiscordRole
- Non-nullable
- Nullable
- Nullable with optional value
DiscordUser
- Non-nullable
- Nullable
- Nullable with optional value
Double
- Non-nullable
- Nullable
- Nullable with optional value
Enum
- Choice attribute
  - Non-nullable
  - Nullable
  - Nullable with optional value
- Directly through enum
  - Non-nullable
  - Nullable
  - Nullable with optional value
- ChoiceProvider
  - Non-nullable
  - Nullable
  - Nullable with optional value
Int16
- Non-nullable
- Nullable
- Nullable with optional value
Int32
- Non-nullable
- Nullable
- Nullable with optional value
Int64
- Non-nullable
- Nullable
- Nullable with optional value
SByte
- Non-nullable
- Nullable
- Nullable with optional value
String
- Non-nullable
- Nullable
- Nullable with optional value
TimeSpan
- Non-nullable
- Nullable
- Nullable with optional value
UInt16
- Non-nullable
- Nullable
- Nullable with optional value
UInt32
- Non-nullable
- Nullable
- Nullable with optional value
UInt64
- Non-nullable
- Nullable
- Nullable with optional value

Expected behaviour for each is correct conversion without any errors. Non-nullable and nullable values are both required parameters, while nullables with optional values are optional parameters.

OoLunar · 2022-12-13T18:57:16Z

Bless your heart

Tawmy · 2022-12-13T19:30:03Z

QA Notes

Registering global application commands is broken (guildId null)
- Workaround: Pass 0 to the method instead
Cannot test the following types: Byte, DiscordMember, DiscordMessage, Int16, Int32, SByte, UInt16, UInt32, UInt64
- Error {MethodName} has an invalid type! Acceptable types are: Boolean, Int64, Double, String, TimeSpan, Enum, DiscordChannel, DiscordUser, DiscordRole, DiscordEmoji, DiscordAttachment, SnowflakeObject
- Seems like there is still a check in place that checks for the data types supported before this PR
SnowflakeObject data type is missing
Bad Request when trying to register slash command with string? parameter (both with and without default value)
- Regular string works fine
Bad Request when trying to register slash command with any type of TimeSpan parameter (non-nullable, nullable, nullable with default value)

Cannot test any of the registered commands due to a NullReferenceException:

System.NullReferenceException: Object reference not set to an instance of an object. at Microsoft.Extensions.DependencyInjection.ActivatorUtilities.GetServiceOrCreateInstance(IServiceProvider provider, Type type) at DSharpPlus.SlashCommands.SlashCommandsExtension.CreateInstance(Type type, IServiceProvider serviceProvider) in /Users/tawmy/Git/DSharpMcTestface/DSharpPlus/DSharpPlus.SlashCommands/SlashCommandsExtension.cs:line 830 at DSharpPlus.SlashCommands.SlashCommandsExtension.RunCommandAsync(BaseContext context, MethodInfo method, IEnumerable'1 args) in /Users/tawmy/Git/DSharpMcTestface/DSharpPlus/DSharpPlus.SlashCommands/SlashCommandsExtension.cs:line 759

@Erisa

* Change GuildMemberUpdatedEventArgs to contain before and after DiscordMember objects (#1359) * Fix #1325 - WebhookBuilder Throws Bad Request (#1363) * use DiscordMentions on RestWebhookPayloads * keep null value when null * remove null check from discordmentions arg * Implemented PATCH /guilds/:guild_id/roles (tested) (#1311) * Fix compile errors caused by #1333 * Update hosting.md * Remove unused DSharpPlus.MSTest folder * Cache threads upon thread creation (#1366) * categorization for the cnext default help command (#1371) * Fix memberBefore missing roles in GuildMemberUpdateEventArgs when not… (#1364) * Fix memberBefore missing roles in GuildMemberUpdateEventArgs when not in cache * Update cached guild member in OnGuildMemberUpdateEventAsync internal event handler * Update DiscordClient.Dispatch.cs Co-authored-by: Lunar Starstrum <lunar@forsaken-borders.net> * Ported CommandsNext cooldown attribute to Slash Commands (#1375) * Ported CommandsNext cooldown attribute to Slash Commands; Fixed compiler warnings and suggestions * Still untested; Attempted to resolve @Erisa's review * Made all exceptions inherit from ApplicationCommandExecutionChecksFailedException (previously Exception) * Start on converters * Added custom converter support: - No, I did not implement the checks in the converters. I felt that should be done in a separate area of code - Yes, I did break my Enter, Backspace, D and Apostrophe keys because of this PR. Don't ask how. * Fixed startup errors; Added DiscordMessageArgumentConverter; Added (un)register converter methods (untested). * Fix registering converters * (untested) Add number converters; Fix enums not registering choices correctly * Add DiscordMember converter; Switch from ArgumentException to new MissingArgumentConverterException * Actually fix Nullable and update TimeSpan converter to match with CNext * Add params/array support * Boundry checks * [ci-skip] Resolve naming errors * "One less variable allocation" * Remove unsupport NETSTANDARD1_3 support; Un-Emziware those variable names * Remove unused property * Extra parameter limit attribute check * Remove UnregisterConverter(s) methods * Changed BCL types to use framework names * Pass InnerException to prevent passing TargetInvocationException * Fix enum support (again) --------- Co-authored-by: Bob Loblaw <razvanmandache@gmail.com> Co-authored-by: waffle-lord <76401815+waffle-lord@users.noreply.github.com> Co-authored-by: Iris Féanorá <61018569+ExaInsanity@users.noreply.github.com>

* Added better logging and adds ReturnType check to sub methods in command groups * fix merge issue

* Oops! I sneezed. * One of you hooligans needs to fix your IDE formatting. `dotnet format` * Fix context menu processors requiring exact types instead of ish types * ConfigureCommands event from #1680 * Documentation; Changed equality implementation on `ConverterDelegateFactory`; Fixed runtime startup exceptions * Move `GetConverterFriendlyBaseType` to `IArgumentConverter` to prevent static implementation on every command processor * Several bug fixes to argument parsing and conversion logic - Finish moving `GetConverterFriendlyBaseType` - Prevent `AddConverter` from throwing when the same converter is attempting to be added again - Early return on `ParseParametersAsync` when no parameters need to be parsed - Move default value logic from `ParseParametersAsync` to `ParseParameterAsync` - `ParseParameterAsync` now provides the value that failed to parse when returning `ArgumentFailedConversionValue` - `ExecuteConverterAsync<T>` now handles multi-argument parameters * Shadow `InteractionConverterContext.Argument` so `ConverterContext.Argument` can still be the raw value * Fix enum parsing for text commands, which now properly supports boxing (is it called boxing?) * There can only be one reply to a Discord message * Defer multi-argument parameter parsing to the base implementation * Theoretically add support for `TextMessageReplyAttribute` to most argument converters * Use `IArgumentConverter.GetConverterFriendlyBaseType` * Configure commands event args. This is non-negotiable. * Use `IArgumentConverter.GetConverterFriendlyBaseType]`; Defer multi-argument parameter parsing to the base implementation * `IArgumentConverter.GetConverterFriendlyBaseType`; Theoretically better support multi-argument parameters with slash registration * Update deps * Resolve 1 nullable warning * Fix logic within user command registration, no longer allowing it to be forever false * Remove old event registration logic * Expose `IServiceProvider` instead of pointlessly hiding it * Switch from `AsyncEventArgs` to `DiscordEventArgs` * Move converter result values to their own namespace instead of repeatedly redeclaring on generic types * Remove ConverterFriendlySearchFlags; Add new `EnumConverter<T>`; Use the generic version of EnumConverter for parameters. * Fix compiler errors * Fix params registering all parameters as required * Fix choice providers not registering; Correctly isolated slash command logic away from the command parameter builder; Add an autocomplete provider for enums with more than 25 fields * Remove object constructors and replace them with more type-specific constructors for compile-time safety. * Vast improvements to the autocomplete and choice providers API - `SlashAutoCompleteProviderAttribute.AutoCompleteAsync` and `SlashChoiceProviderAttribute.GrabChoicesAsync` will now truncate when more than 25 elements are provided. - When options are truncated, it will log which source gave too many options, which it previous didn't. - When the type cannot be created, it will log which type failed along with the exception. - Try to use the numeric values for enums when possible, serializing only to `string` when the base type is `ulong`. - Support double and float enums, because apparently that's a thing? - Change the method signatures to return an enumerable of the object directly instead of dictionaries - Remove `SnakeCasedNameAttribute` in it's entirety, replacing it with `IParameterNamingPolicy`. * Fix compiler error * Expose the conversion result to `ArgumentParseException`; Documentation on related members. * Missed a spot * Fields to properties * Correctly apply limit * Just git- I mean get * Line feeds? * `git diff --name-only --diff-filter=AM master | grep ".cs" | xargs -I{line} echo \'{line}\' | xargs dotnet csharpier` * Update deps * Add generic overloads * Format code in docs * Grammar; New empty `T : new()` generic restraint on `AddConverter<T>()` * Update autocomplete/choice provider documentation * Update the missing slash commands section * Rename `DSharpPlus.Commands.Processors.SlashCommands.ParameterNamingPolicies` to `DSharpPlus.Commands.Processors.SlashCommands.NamingPolicies` * New `Naming Policies` article * Document multi-argument parameters * Don't throw when enums fail to convert * Support all time units from nanosecond to years * Give converters their own section; Briefly go over built-in argument converters and their quirks * Add a guide for invoking argument converters manually * Enforce max argument count in MAPs * Undo formatting from `dotnet csharpier` The new formatting now follows two very simply rules: - If it does not go beyond the end of my 1080p screen with the file bar open, it stays inlined. - If Roslyn didn't yell at me about it. * Allow 3rd party command processors to implement their own generic enum converter * Begone MAPs, welcome in VAPs * Remove raw numbers :( * Footnote * Formatting * #1362 was never merged * Throw away the rest of the MAPs * docs * Interface checks * Variadic * Attempt to resolve #1950 * Rename `ParameterNamingPolicy` to `NamingPolicy`; Fix inverted if statement * Use `GetCultureInfo` * More docs * Localizing interactions * Double space * Fix final GitHub review

OoLunar added 5 commits July 25, 2022 01:04

dotnet format DSharpPlus.SlashCommands

d4203b2

Manual format with some code "improvements"

c82a0c0

Refactored file structure

e4a3865

find DSharpPlus.SlashCommands/ -type f -exec sed -i -E "s/(\/{2,})([^…

dbb94db

… \/])/\1 \2/g" {} \;

Almost finished cleaning up SlashCommandExtension.cs

462f442

OoLunar added enhancement help wanted big-change v4.x obsolete: slash-commands labels Aug 1, 2022

OoLunar and others added 4 commits August 1, 2022 16:36

Variable renaming and correct constraints in ExtensionMethods.GetName…

06cd11f

…<T> and `dotnet format` with required file headers

Documentation

a682558

Fix broken links that weren't caused by my perfect regex

clean up attribute retrieval (#1365)

0a7df6e

* clean up attribute retrieval * clean up futher, unify more things

Formatting

240e128

OoLunar added 2 commits August 18, 2022 13:38

Implement DiscordInteraction.Member

e2227b2

Obsolete SlashModuleLifespan in lieu of ServiceLifetime

f66f9cf

No longer a breaking change

8189fe1

This was referenced Aug 29, 2022

Fix CI #1381

Closed

Fix/v4/slashies-cleanup custom converter support #1382

Merged

akiraveliara self-assigned this Aug 29, 2022

VelvetToroyashi mentioned this pull request Jan 28, 2023

Add custom slash command argument converters #1222

Closed

Improving SlashCommandsExtension (#1474)

d7b2caf

* Added better logging and adds ReturnType check to sub methods in command groups * fix merge issue

OoLunar added the on-hold label Mar 22, 2023

OoLunar closed this Apr 18, 2023

akiraveliara deleted the fix/v4/slashies-cleanup branch January 30, 2024 19:08

OoLunar added a commit that referenced this pull request Oct 4, 2024

#1362 was never merged

1e21b6b

Uh oh!

Fix/v4/slashies cleanup #1362

Fix/v4/slashies cleanup #1362

Uh oh!

Conversation

OoLunar commented Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OneYellowLemon commented Aug 18, 2022

Uh oh!

OoLunar commented Aug 18, 2022

Uh oh!

OoLunar commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akiraveliara commented Aug 18, 2022

Uh oh!

OoLunar commented Aug 18, 2022

Uh oh!

Tawmy commented Dec 13, 2022

Uh oh!

OoLunar commented Dec 13, 2022

Uh oh!

Tawmy commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OoLunar commented Dec 13, 2022

Uh oh!

Tawmy commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QA [WIP]:

Uh oh!

OoLunar commented Dec 13, 2022

Uh oh!

Tawmy commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QA Notes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

OoLunar commented Aug 1, 2022 •

edited

Loading

OoLunar commented Aug 18, 2022 •

edited

Loading

Tawmy commented Dec 13, 2022 •

edited

Loading

Tawmy commented Dec 13, 2022 •

edited

Loading

Tawmy commented Dec 13, 2022 •

edited

Loading