How to unsubscribe from an event?

[infiniteyak]

Maybe this is obvious and I'm just missing it or I'm using events incorrectly, but I can't figure out how to unsubscribe from an event. Is there a way to do this?

[yohamta]

Hey @infiniteyak 👋
thanks for the question!
It seems that we don't have that feature yet, but I think it can be easily added.
If you are interested in making a PR, please feel free to submit one.
https://github.com/yohamta/donburi/blob/main/features/events/events.go#L77

[infiniteyak]

Okay I will give it a shot. I'm not very strong at go yet though so I'm not sure the best way to accomplish this. I guess this will be educational.

[yohamta]

Great! I appreciate your willingness to contribute and help improve the project. Don't worry about not being strong in Go, we all have to start somewhere. If you have any questions or need help while working on the PR, don't hesitate to reach out to me. I'll be happy to offer guidance and support. Good luck!

[infiniteyak]

I think this works, however I am under the impression using reflect in this way is frowned upon. Should I find another way or is it acceptable?

// Unsubscribe removes a subscriber for the event.
func (e *EventType[T]) Unsubscribe(w donburi.World, subscriber Subscriber[T]) {
    if e.eventBusQuery.Count(w) != 0 {
        eventBus := e.mustFindEventBus(w)
        newSubs := make([]Subscriber[T], len(eventBus.subscribers)-1)
        for _, s := range eventBus.subscribers {
            if reflect.ValueOf(s).Pointer() != reflect.ValueOf(subscriber).Pointer() {
                newSubs = append(newSubs, s)
            }
        }
        eventBus.subscribers = newSubs
    }
}

[yohamta]

Great job! 👍
I just have a few suggestions to improve the code.

You might be able to compare interface values directly without reflection. Here's an example of what that might look like:

func (e *EventType[T]) findSubscriber() (int, error) {
    if e.eventBusQuery.Count(w) != 0 {
        eventBus := e.mustFindEventBus(w)
        for i, s := range eventBus.subscribers {
            if s != subscriber {
                return i, true
            }
        }
    }
    return 0, false
}

Additionally, I think that the swap-remove approach could be useful to avoid making a new allocation for the slice. You can see the example of this technique in this file:
https://github.com/yohamta/donburi/blob/main/internal/storage/storage.go#L61

If you have a chance, it might be helpful to add a test like TestUnsubscribe to make sure the fix is working as expected.
https://github.com/yohamta/donburi/blob/main/features/events/example_test.go#L42

Overall, keep up the great work! 😄

[infiniteyak]

Sounds good, I will make those changes before I send you a PR. Thanks for the feedback!