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

Rewrite Components #117

Open
VincentRPS opened this issue Dec 31, 2022 · 8 comments
Open

Rewrite Components #117

VincentRPS opened this issue Dec 31, 2022 · 8 comments
Assignees
Labels
enhancement Improves a part of the library python Pull requests that update Python code rewrite This doesn't feel right. Time to redo!
Milestone

Comments

@VincentRPS
Copy link
Member

VincentRPS commented Dec 31, 2022

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 VincentRPS added enhancement Improves a part of the library rewrite This doesn't feel right. Time to redo! python Pull requests that update Python code labels Dec 31, 2022
@VincentRPS VincentRPS added this to the 3.0.0 milestone Dec 31, 2022
@VincentRPS VincentRPS self-assigned this Dec 31, 2022
@sairam4123
Copy link

sairam4123 commented Feb 21, 2023

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
Copy link
Member Author

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
Copy link

sairam4123 commented Feb 21, 2023

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
Copy link
Member Author

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
Copy link

I understand now, thanks for the justification.

@sairam4123
Copy link

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
Copy link

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves a part of the library python Pull requests that update Python code rewrite This doesn't feel right. Time to redo!
Projects
None yet
2 participants