Cog-Creators / Red-DiscordBot

A multi-function Discord bot
https://docs.discord.red
GNU General Public License v3.0
4.71k stars 2.3k forks source link

[V3] Scheduler #1309

Closed The-Tasty-Jaffa closed 4 years ago

The-Tasty-Jaffa commented 6 years ago

Add a time/looping feature for more effective and less repetitive code.

Why?

  1. Listening for events has many advantages over repeatedly checking for a particular event/condition
  2. Removes the need of repetitive functions in cogs which all perform similar tasks
Sitryk commented 6 years ago

Do you mean like, more events that cogs can listen for? Any examples of some you might use

The-Tasty-Jaffa commented 6 years ago

Do you mean like, more events that cogs can listen for?

Yes.

As an example, Instead of having something like this

def check():
    DELAY = 60 #Delay in seconds
    while self == self.bot.get_cog("cog"):
        #body

It could be done like this

@utils.loop(time=60) #also delay in seconds
def check():
        #body
The-Tasty-Jaffa commented 6 years ago

Or however it ends up getting implemented. So it could work something more like listeners currently do in discord.py

Sitryk commented 6 years ago

Developers could start using the bot.dispatch which would be really nice for cogs like audio and economy, which could dispatch events for stuff like song changing and cogs could act on those events

Bakersbakebread commented 6 years ago

There's also the listen decorator. Would be an easy integration

https://discordpy.readthedocs.io/en/rewrite/ext/commands/api.html#discord.ext.commands.Bot.listen

The-Tasty-Jaffa commented 6 years ago

However, how would calling it at different times work (for bot.listener)? (it doesn't allow for other options. For example, datetime objects cannot be passed or number of seconds.) While there could be a set time of 60 seconds I feel that doing this it would remove the usefulness of any such additions.

mikeshardmind commented 6 years ago

bot.dispatch would be the correct way to have core red send events to listeners in cogs if this were to be added. We'd need documentation on any events we'd send, as well as ensuring zero overlap with internal discord.py dispatch events.

This would be as simple for cog creators as having a

async def on_event_name(self, event_arg1, ...):

line. it would be no harder to use from the perspective of a cog creator than listening for an on message event.

Also, because of how bot.dispatch is designed, if talking about an internal scheduler for tasks, when scheduling them, cogs could just send their own event name to the scheduler (this may not be a good idea if we don't want cogs to be able to schedule for other cogs though)

Sitryk commented 6 years ago

Could we possible modify bot.dispatch to where it takes the cog name and adds that before the on_

# under redbot.core.base.RedBase

def dispatch(self, event_name, *args, **kwargs):
    super().dispatch(event_name, *args, **kwargs)
    event_name = self.__class__.__name__ + event_name
    ev = 'on_' + event_name
    if ev in self.extra_events:
        for event in self.extra_events[ev]:
            coro = self._run_extra(event, event_name, *args, **kwargs)
            discord.compat.create_task(coro, loop=self.loop)

I'm suspicious my example is wrong, but you get the gist of it, add something that identifies the caller so that it cannot fake a dispatch, there are ways around it but if someone i trying that hard then it must be important to the idea of the cog

mikeshardmind commented 6 years ago

We could, but doing it that way is as you suspected, not quite right. We'd want to modify any events not in a list of events to not get those modifications. Behavior for on_message (and other d.py builtins) in your snippet is a little off.

Also, this wouldn't prevent someone from scheduling something in another cog (there's no way to actually do that, since I could just call whatever scheduler function your cog uses from mine. I could even modify it with setattr) I don't think modifying d.py internals for this is worth it, more just a thought on whether we should be enforcing unique 3rd party event names to avoid accidentally scheduling for another cog.

The-Tasty-Jaffa commented 6 years ago

What about using a Decorator? (Using the decorator to add the function which might of already been (or not) implied)

Add the func to a dictionary with its value (by using the decorator) as to when to get called and have something which simply checks the time. (so something like time.time() would be sufficient). if it's changed subtract the difference from all the values. If any value is less than 0 then call the func and reset the value. How does that sound as a possible way of adding it?

tekulvw commented 6 years ago

If we decide to add cog events I feel like they should have their own dispatcher and not use d.py's core dispatcher to avoid any possible conflicts (however unlikely) down the road.

On another note, I am all for adding a scheduler like @The-Tasty-Jaffa suggests using decorators.

The-Tasty-Jaffa commented 6 years ago

Thank you :) Should I make a PR for it?

tekulvw commented 6 years ago

Go for it.

Tobotimus commented 5 years ago

Adding this to be included in 3.1. I have some basic design going for a scheduler on my local repo, so I'm gonna self-assign this.

The-Tasty-Jaffa commented 5 years ago

I also had something already and had it working excluding the decorators... I can PR them into your repo if you'd like.

mikeshardmind commented 4 years ago

discord.py has ext.tasks which should be a suitable answer for this, closing.