Pycord-Development / pycord-next

Go Build Apps - without worrying about speed, reliability, or simplicity.
https://v3-docs.pycord.dev
MIT License
92 stars 12 forks source link

Rewrite Components #117

Open VincentRPS opened 1 year ago

VincentRPS commented 1 year ago

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.

sairam4123 commented 1 year ago

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.

VincentRPS commented 1 year ago

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
    ...
sairam4123 commented 1 year ago

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.

VincentRPS commented 1 year ago

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)

sairam4123 commented 1 year ago

I understand now, thanks for the justification.

sairam4123 commented 1 year ago

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

Code ```python 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 commented 1 year ago

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!

VincentRPS commented 1 year ago

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.