Skip to content

fix(IsLoadableModule): return false for abstract classes#3148

Merged
Misha-133 merged 4 commits into
discord-net:devfrom
OverwrittenCode:Mark-Abstract-As-Not-Loadable-Module
Jul 13, 2025
Merged

fix(IsLoadableModule): return false for abstract classes#3148
Misha-133 merged 4 commits into
discord-net:devfrom
OverwrittenCode:Mark-Abstract-As-Not-Loadable-Module

Conversation

@OverwrittenCode

@OverwrittenCode OverwrittenCode commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

Description

IsLoadableModule did not check the property TypeInfo.IsAbstract. Therefore, an unnecessary warning log stated that it could not be loaded.

Now, if a class is abstract, it will be ignored safely and all the inheritors will continue to load any commands.

Changes

  • fix(IsLoadableModule): return false for abstract classes
  • fix: handle other attributes
  • refactor: simplify attribute logic
  • fix: correct terminology in debug message

Related Issues

N/A

IsLoadableModule did not check the property TypeInfo.IsAbstract.
Therefore, an unnecessary warning log stated that it could not be loaded.

Now, if a class is abstract, it will be ignored safely and all the inheritors will continue to load any commands.
Comment thread src/Discord.Net.Interactions/Builders/ModuleClassBuilder.cs Outdated
@OverwrittenCode

OverwrittenCode commented Jun 27, 2025

Copy link
Copy Markdown
Contributor Author

@Cenngo your suggestion looks completely fine - I have:

  • Accepted your suggestion and slightly modified it so the switch statement is no longer needed.
  • Corrected the debug message

Do you think we can mark this PR as Ready for review?

Hold on, we might be missing some.

Screenshot of the src/Discord.Net.Interactions/Attributes/Commands folder for reference:

src/Discord.Net.Interactions/Attributes/Commands

  • AutocompleteCommandAttribute
  • ComponentInteractionAttribute
  • ContextCommandAttribute
  • MessageCommandAttribute
  • ModalInteractionAttribute
  • SlashCommandAttribute
  • UserCommandAttribute

Should I include MessageCommandAttribute and UserCommandAttribute before marking this PR ready for review?

Also, it may help to sort the is pattern matching alphabetically so it is easier to compare to the source folder.

@Cenngo

Cenngo commented Jul 1, 2025

Copy link
Copy Markdown
Collaborator

Should I include MessageCommandAttribute and UserCommandAttribute before marking this PR ready for review?

That wouldnt be necessary since both of those already inherit the ContextCommandAttribute. The PR should be good to go as is.

@OverwrittenCode OverwrittenCode marked this pull request as ready for review July 1, 2025 18:11
@OverwrittenCode

Copy link
Copy Markdown
Contributor Author

Perfect, PR is now ready to merge.

@Misha-133 Misha-133 merged commit 978f999 into discord-net:dev Jul 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants