nextcord / nextcord-ext-menus

A nextcord menu and pagination extension that makes working with reaction menus and button component menus a bit easier
https://menus.docs.nextcord.dev
MIT License
26 stars 9 forks source link

inherit_buttons on parent is ignored when subclassing multiple times #17

Open DenverCoder1 opened 3 years ago

DenverCoder1 commented 3 years ago

This is an issue that existed in discord-ext-menus as well.

The issue applies to reaction menus (MenuPages) as well as button menus (ButtonMenuPages)

Example:

class ButtonNonInheriting(menus.ButtonMenuPages, inherit_buttons=False):

    @nextcord.ui.button(emoji='\N{WHITE LEFT POINTING BACKHAND INDEX}')
    async def my_first(self, button, interaction):
        await self.go_to_previous_page()

    @nextcord.ui.button(emoji='\N{WHITE RIGHT POINTING BACKHAND INDEX}')
    async def my_next(self, button, interaction):
        await self.go_to_next_page()

    @nextcord.ui.button(emoji='\N{OCTAGONAL SIGN}')
    async def my_stop(self, button, interaction):
        await self.stop()

class ButtonInheritNonInheriting(ButtonNonInheriting, inherit_buttons=True):

    @nextcord.ui.button(emoji='\N{WHITE HEAVY CHECK MARK}')
    async def my_check(self, button, interaction):
        await self.message.edit(content="Check!")

The expected behavior would be that since ButtonNonInheriting does not inherit the pagination buttons, ButtonInheritNonInheriting should also not and it should show just the 4 buttons appearing in the two classes.

Instead, all buttons from the parent class of ButtonNonInheriting, reappear when using the ButtonInheritNonInheriting class.

image

parker02311 commented 2 years ago

I think the only approach to this would be to have users disable inherit_buttons on ButtonInheritNonInheriting because it should still inherit ButtonNonInheriting buttons even while disabled.

DenverCoder1 commented 2 years ago

Good observation.

That does work for Button menus, but for reaction menus it works differently.

class NonInheriting(menus.MenuPages, inherit_buttons=False):
    @menus.button("\N{WHITE LEFT POINTING BACKHAND INDEX}")
    async def my_first(self, payload):
        await self.go_to_previous_page()

    @menus.button("\N{WHITE RIGHT POINTING BACKHAND INDEX}")
    async def my_next(self, payload):
        await self.go_to_next_page()

    @menus.button("\N{OCTAGONAL SIGN}")
    async def my_stop(self, payload):
        await self.stop()

class InheritNonInheriting(NonInheriting, inherit_buttons=False):
    @menus.button("\N{WHITE HEAVY CHECK MARK}")
    async def my_check(self, payload):
        await self.message.edit(content="Check!")

In this example, only the check mark reaction appears.

parker02311 commented 2 years ago

Thats a good point, but I dont think there really is a way to fix that. I can look into it further but yeah...

parker02311 commented 2 years ago

@DenverCoder1 what if we changed the inherit_buttons to be a thing in a init function instead?

Like if we did

def __init__(self):
    super().__init__(inherit_buttons=True)

Maybe that would make fixing this like 10x easier