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

[Fix] Add missing parameters to RespondWithModalAsync methods and implement missing overloads #2769

Merged

Conversation

zobweyt
Copy link
Contributor

@zobweyt zobweyt commented Aug 28, 2023

Description

This PR adds missing implementation of modal modification in module bases.

Changes

  • Add missing modifyModal parameters to the RespondWithModalAsync<T> methods. (b7ee974)
  • Add missing RespondWithModalAsync<T> overload to module bases. (e63665d)
  • Fix inactive options parameter in the RespondWithModalAsync method. (46bcd72)
  • Remove code duplication in IDiscordInteractionExtensions by using the modalInfo.FromModal method. (42f448b)
  • Remove code duplication in RestInteractionModuleBase by extracting a HandleInteractionAsync method. (1a788d4)

@Misha-133
Copy link
Member

Could you also add this overload?

@zobweyt
Copy link
Contributor Author

zobweyt commented Sep 2, 2023

should the modifyModal parameter come before options? almost everywhere it's ordered last and least used, also there will be no need to manually specify the parameter name or pass null to the options

@Misha-133
Copy link
Member

should the modifyModal parameter come before options? almost everywhere it's ordered last and least used, also there will be no need to manually specify the parameter name or pass null to the options

Yeah, it should
But in some places doing that would introduce a breaking change. So you should order params in logical order where it won't
break existing code.

@zobweyt zobweyt changed the title [Fix] Add missing modifyModal parameters to RespondWithModalAsync methods [Fix] Add missing parameters and overloads to RespondWithModalAsync methods Sep 2, 2023
@zobweyt zobweyt changed the title [Fix] Add missing parameters and overloads to RespondWithModalAsync methods [Fix] Add missing parameters to RespondWithModalAsync methods and implement overloads Sep 2, 2023
@zobweyt zobweyt changed the title [Fix] Add missing parameters to RespondWithModalAsync methods and implement overloads [Fix] Add missing parameters to RespondWithModalAsync methods and implement missing overloads Sep 2, 2023
@Misha-133 Misha-133 merged commit 4e78aec into discord-net:dev Oct 17, 2023
3 checks passed
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.

2 participants