pycord-v3 icon indicating copy to clipboard operation
pycord-v3 copied to clipboard

Rewrite Components

Open VincentRPS opened this issue 2 years ago • 8 comments

The Proposal

People have heavily complained about the current implementations lack of addressing many key things, as well as not doing its job quite good.

When initially implementing components I thought they were good, though once I took a deeper look at it I could notice all of the weird inconsistencies and the plethora of bad naming schemes, and many other things decided during its implementation.

This issue rewrites the entire system. It does not detail internals, yet only the user-end.

Example

my_view = View()

@my_view.button(name='Click Me')
async def click_me(pre: pycord.Prelude) -> None:
    await pre.send('Clicked!')

# ...
view = my_view()
message = await pre.send('This is a View!', view=view)
await view.overwatch(message)

This would slightly change some logic, forcing .send, .respond, and .resp.send to return a message object.

VincentRPS avatar Dec 31 '22 08:12 VincentRPS

Could I recommend letting us subclass Widget, maybe like sending the widget from the bot just like cog.

Here's an example:

from pycord.ext import widgets

class GameWidget(Widget):
    def __init__(self, bot):
        self.bot = bot
    
    def __disable__(self):
         # a overridable method to change the disable behaviour
         pass

   def __enable__(self):
         pass
   
@widgets.button(name="test", style=widgets.ButtonStyle.primary)
def test_btn(self):
      # do something
      pass

@widgets.menu(name="test")
def test_menu(self, value):
      # do something
      pass

# want same options?
@test_menu.options()
@test_menu2.options()  # just add another option
def test_menu_opts(self, menu: widgets.Menu, autocomplete: Optional[str]):
     # return a list of strings, or a list of Widget.options
     # this method is also helpful to implement autocomplete if discord ever implements autocomplete for select menus


   def send(self, ctx):
        # a overridable method which is run when this Widget is used by the bot on a command
        # change the widget to match the ctx if necessary.


def setup(bot):
     bot.add_widget(GameWidget(bot))

this comment gives us an idea of implementing one widget every file, it uses extensions system to implement the widgets.

note: the above example was written using a mobile, please expect indentation issues.

sairam4123 avatar Feb 21 '23 13:02 sairam4123

I don't exactly see the point to subclass widget, but if you want to you can do so without being finicky and having to play around with overriding functions, or other such.

As well, adopting a cog-like design would 1 not have any real use and 2 just adds more boilerplate. I do plan on adding an event dispatcher specially for views so you can listen for enable and disable though.

As for the widget-per-file design, my question is "why?" It adds unnecessary files and unnecessary boilerplate and honestly isn't something people need nor want, and with the current and new system can still be done but easier.

And on a final clause, I do plan on adding automatic widget instance creation for commands, like:

@bot.command(...)
async def my_command(inter: pycord.Interaction, widget: Widget = wcb(my_widget)) -> None:
    # use widget like normal
    ...

VincentRPS avatar Feb 21 '23 14:02 VincentRPS

As for the widget-per-file design, my question is "why?" Separation of Concern, we could consider each widget as a class and save them to a separate file and load the widget whenever necessary, though widget is lightweight and doesn't take a lot of memory, it's a matter of taste rather.

I like to keep my widgets in separate files (it's a thing I do cus of godot). it really helps and keeps the code clean, so I recommended it. It's your choice.

sairam4123 avatar Feb 21 '23 14:02 sairam4123

The thing is widgets are really short, so forcing separation would lead to unnecessary overhead and having like 30 files which are less then 100 lines long (what I normally see as the minimum justifiable for a module file)

VincentRPS avatar Feb 21 '23 15:02 VincentRPS

I understand now, thanks for the justification.

sairam4123 avatar Feb 21 '23 15:02 sairam4123

Hi Vincent, I do think that subclassing widgets could be a good idea when you have a big widget like this:

Code
    class CreateNewsTask(Widget):
        
        welcome_lbl: Label # will be just a message
        cof_btn: Button
        guild_btn: Button
        
        button_group: ButtonGroup = button_group(buttons=[cof_btn, guild_btn])  # could also be action row
        
        news_text: NewsTextModal
        set_txt_btn: Button
        
        title_text: NewsTextModal
        set_title_btn: Button

        users_select: SelectMenu = select_menu(type='users')
        roles_select: SelectMenu = select_menu(type='roles')
        initial_time_delta: SelectMenu = select_menu(disabled=True)
        divisions: SelectMenu = select_menu(disabled=True)

        send_btn: Button
        cancel_btn: Button
        confirm_btn_group: ButtonGroup = button_group(buttons=[send_btn, cancel_btn])

        # use decorators to implement callbacks!
        @cof_btn.pressed
        async def cof_btn_pressed(self, itc: Interaction):
            self.cof = True
            cof_btn.style = ButtonStyle.danger
        
        @users_select.selected
        async def users_selected(self, itc: Interaction):
            self.users = users_select.values  # values is of type User|Member
        
        @set_text_btn.pressed
        async def set_text_btn_pressed(self, itc: Interaction):
            await itc.send_modal(news_text)
            await itc.wait_for_response()
            self.text = await itc.get_response()

        
        async def send(self, ctx: Context):
            # do the widget initialization to send widget
            pass
        
        async def disable(self):
            # disable this widget
            pass
        
        async def enable(self):
            # enable this widget
            pass
Implementing this through your proposed idea would be difficult to read (I haven't tried it, will post that code too), the widget will be sent in a separate thread, actually we could name this ThreadWidgets (an alternative to Modals).

What do you think of this proposed ThreadWidget? Please let me know.

Note: this is just an idea I had for a few days, I hope you won't mind me posting it here! :)

sairam4123 avatar Apr 13 '23 15:04 sairam4123

The thing is widgets are really short, so forcing separation would lead to unnecessary overhead and having like 30 files which are less then 100 lines long (what I normally see as the minimum justifiable for a module file)

For small widgets, it's better to keep them in the same file, but if the widget is big, like around ~470 lines, it could be a seperate file, though the subclassing would make it much smaller and simpler. I hope I am clear on what I am trying to convey here!

sairam4123 avatar Apr 13 '23 15:04 sairam4123

Hi Vincent, I do think that subclassing widgets could be a good idea when you have a big widget like this:

Code Implementing this through your proposed idea would be difficult to read (I haven't tried it, will post that code too), the widget will be sent in a separate thread, actually we could name this ThreadWidgets (an alternative to Modals). What do you think of this proposed ThreadWidget? Please let me know.

Note: this is just an idea I had for a few days, I hope you won't mind me posting it here! :)

I'm sorry I don't really get much of the code nor the point? Subclassing and instancing have no different in this situation though I could still try to recreate what you're trying to do easily.

view = pycord.View()
modal = pycord.Modal(...)
# You can also use the builder method!
# modal = pycord.ModalBuilder().title(...).option(...)


@view.button(...)
async def cof_set_btn_pressed(ctx: pycord.VContext) -> None:
    await ctx.send("Set!", ephemeral=True)

    if ctx.disabled:
        await ctx.disable()
    else:
        await ctx.enable()

@view.user_select(...)
async def users_selected(ctx: pycord.VContext) -> None:
    print(ctx.users)

@view.button()
async def send_modal(vctx: pycord.VContext) -> None:
    fut = await vctx.send(modal)
    resp = await fut
    vctx.ctx.text = resp.content

This holds much less boilerplate and is much easier to understand.

VincentRPS avatar Apr 13 '23 15:04 VincentRPS