Event rework

[shayypy]

This issue outlines two enhancements to how events work in guilded.py. The first is the addition of more granular event control: restraining categories of events based on what your client needs. The second is a rework to how parameters are passed to event callbacks.

Granular events

discord.py implements a similar idea with its enable_debug_events parameter for Clients. This enhancement proposes the addition of this parameter as well as a use_discordpy_style_events parameter in order to "switch" event behavior to be more akin to discord.py. This would currently only detail event names (e.g., server_join vs. guild_join - only one or the other), but its effects are greater when considering the following change:

Event parameters

This change is considerably more impactful. If you are familiar with the JavaScript event system (with its Event and inheritors) then this may ring a bell for you. Instead of the following:

async def on_message(message):
    # message is a ChatMessage
    ...

you would handle events like so:

async def on_message(event):
    # event is a MessageEvent
    message = event.message
    # message is a ChatMessage

This change could also come with the decision to drop raw_ events in exchange for a more unified interface:

async def on_message_update(event):
    before = event.before  # could be None. Such a value indicates that the message was not cached.
    after = event.after  # always a ChatMessage

    # To perform cache-dependent logic, the user will essentially mirror the previous internal behavior:
    if before is None:
        return

This also flattens relatively convoluted events like raw_message_reaction_add and makes the whole event system function similarly. For example:

async def on_message_reaction_add(event):
    message = event.message  # could be None, but the user will always have event.message_id
    emote = event.emote  # Emote
    # The user will also have event.channel(_id), event.created_by(_id), and event.server(_id)

Guilded does not provide the rich data in its ChannelMessageReactionCreated/ChannelMessageReactionDeleted payloads that would make Reaction a better model to have here.

This would also enable events to include miscellaneous metadata, like that which is seen in TeamMemberRemoved:

async def on_member_remove(event):
    member = event.member  # could be None, but the user will always have event.server_id and event.user_id
    kicked = event.kicked
    banned = event.banned

This removes the need for extraneous member_leave and member_kick events which are not present in the bot API.

Conclusion

The latter part of this issue is currently available for use as an opt-in experiment, more details here. Feel free to discuss either of these in the #development channel (see the support header) or in this issue's comments.

[mtl1979]

If the bot really needs to know if a user was banned, then it would make more sense to have separate event for it. Most bots don't need to know if member was "just" kicked from server, as it is usually temporary state. Personally I think it's more important for bots to know if a new user joined, or if existing user will likely never come back. Members can get removed for various reasons, including inactivity, guideline violations, etc.

For other event types, it might be useful to include some relevant metadata, but it should be evaluated case-by-case.

[shayypy]

it might be useful to include some relevant metadata, but it should be evaluated case-by-case

The "relevant metadata" mentioned is that which is provided by Guilded in their events, not the library. You might see how it is less useful for some use cases (e.g. logging) to have an on_member_remove event that only provides the developer with member and simply discards the isKick and isBan properties. Additionally, with the standard-style "flattened" design (separate events for leave, kick, and ban), an event such as TeamMemberRemoved with isBan:true would, when naturally named, conflict with the TeamMemberBanned event, which represents a team ban creation rather than a member removal as a result of a ban.

Most bots don't need to know if member was "just" kicked from server, as it is usually temporary state

What do you mean by this? Events received live over the gateway, processed, then discarded, would be included in "temporary state", no?

Personally I think it's more important for bots to know if a new user joined, or if existing user will likely never come back.

on_member_join exists in the library (along with dozens of other events), it's just not shown in the issue because it isn't very interesting. You can view the events.py file if you want to see the interfaces for all events supported by this experiment. I don't know what you mean by "if an existing user will likely never come back" - how would the library (or Guilded's API) determine that? Would that be an expected wrapper feature if it isn't part of the API?

[mtl1979]

it might be useful to include some relevant metadata, but it should be evaluated case-by-case

The "relevant metadata" mentioned is that which is provided by Guilded in their events, not the library. You might see how it is less useful for some use cases (e.g. logging) to have an on_member_remove event that only provides the developer with member and simply discards the isKick and isBan properties. Additionally, with the standard-style "flattened" design (separate events for leave, kick, and ban), an event such as TeamMemberRemoved with isBan:true would, when naturally named, conflict with the TeamMemberBanned event, which represents a team ban creation rather than a member removal as a result of a ban.

Proper or best naming of events is another issue. guilded.py doesn't have to be 1-1 match with Guided as it currently mimics Discord and doesn't try to implement own API. Maybe in time it will diverge a lot, but that is something yet to be seen.

Most bots don't need to know if member was "just" kicked from server, as it is usually temporary state

What do you mean by this? Events received live over the gateway, processed, then discarded, would be included in "temporary state", no?

Something the bot doesn't necessarily need to act on... In contrast to banning, when the bot can possibly delete all stored data for the user to preserve space when the number of users is significant.

Personally I think it's more important for bots to know if a new user joined, or if existing user will likely never come back.

on_member_join exists in the library (along with dozens of other events), it's just not shown in the issue because it isn't very interesting. You can view the events.py file if you want to see the interfaces for all events supported by this experiment. I don't know what you mean by "if an existing user will likely never come back" - how would the library (or Guilded's API) determine that? Would that be an expected wrapper feature if it isn't part of the API?

When user joins a server that the bot is on, bot needs to create entry for the user either automatically or when user registers with the bot. That information might be preserved even if the user is kicked from the server, but could be deleted if the user is banned from all the servers the bot is on. This means the bot needs to track all servers that specific user has been seen.

[shayypy]

Proper or best naming of events is another issue. guilded.py doesn't have to be 1-1 match with Guided as it currently mimics Discord and doesn't try to implement own API.

Can't say I agree with you 100% there. Yes, event names do not have to mirror the API (and they already don't in many places), but I try to name them in a way that makes sense and isn't completely out of left field. Discord compatibility is important to me when developing, as developers migrating from that platform are the majority. Perhaps, in this case, you would prefer something like on_ban_create?

Something the bot doesn't necessarily need to act on... In contrast to banning, when the bot can possibly delete all stored data for the user to preserve space when the number of users is significant.

When user joins a server that the bot is on, bot needs to create entry for the user either automatically or when user registers with the bot. That information might be preserved even if the user is kicked from the server, but could be deleted if the user is banned from all the servers the bot is on. This means the bot needs to track all servers that specific user has been seen.

These issues are both up to the bot developer, not the library. It's not my place to decide how you should manage data about members in your servers. If you, as the bot developer, decide that you want to erase member data on kick and ban, but not when a user manually leaves, then you can perform that check with MemberRemoveEvent's kicked and banned properties.

async def on_member_remove(event):
    if not event.kicked and not event.banned:
        # delete member data

[mtl1979]

Bot developers, like me, really prefer events that are fine-grained enough that no extra tests are needed. I've used Discord and Telegram APIs before Guilded, so I know what shortcomings those APIs have.

A lot of the details should indeed be implemented in the bot code itself, but when it comes to deciding which API is better, I really think the old one is better of these two choices.

[shayypy]

So what is the deciding factor for your preference here--just more potentially-compact code? Currently, on_member_kick is an extraneous event that exists due to lack of possible detail in the on_member_remove event (outside of passing extra parameters, which is unwieldy and breaking); it doesn't exist in the Guilded API.

I was under the impression that a "closer" event system like the one detailed above--one that reflects all the data in the payloads you receive--would be preferred (I certainly prefer it), but maybe the supplemental event makes more sense to the masses. Perhaps this would make more sense with cases other than member_remove, like you mentioned? There unfortunately aren't really any other events to showcase this with.

[mtl1979]

I would think that IRC style differentiation would be cleanest... Basically when user is only removed, fire MemberKicked event, but when the user is also banned, fire MemberBanned event... There is slight difference between adding a ban and then performing the actual throwing out of the user.