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

fix: Resolve issue about the onShutDown function for Live entities #273

Merged
merged 4 commits into from
May 2, 2021
Merged

fix: Resolve issue about the onShutDown function for Live entities #273

merged 4 commits into from
May 2, 2021

Conversation

Distractic
Copy link
Contributor

@Distractic Distractic commented May 1, 2021

Context

Currently, when using the method onShutDown of LiveEntity classes, the method only intercept the shutdown by events, not manually

Before

Manual

val message = // create message
val live = message.live()
live.onShutDown {
   // Never called
}
live.shutdown()

Event

val message = // create message
val live = message.live()
live.onShutDown {
   // Is called
}
message.delete()

Now

This issue is resolve with the attribut onShutDown present in AbstractLiveKordEntity

val message = // create message
val live = message.live()
live.onShutDown {
   // Is called
}

message.delete()
//or
live.shutdown()

Add deprecated annotation on previous onShutDown extension method
Add onShutDown value in LiveEntity to apply action in event or manual case of shutdown
@HopeBaron
Copy link
Member

HopeBaron commented May 2, 2021

You shouldn't modify the behavior of the onShutdown function in this manner

here is a Mock

//implements CoroutineScope
interface LiveKordEntity : KordEntity, CoroutineScope


abstract class AbstractLiveKordEntity(dispatcher: CoroutineDispatcher) : LiveKordEntity {
     ...

    override val coroutineContext: CoroutineContext = dispatcher + SupervisorJob()

  
    override val events: Flow<Event>
        get() = kord.events
            .takeWhile { isActive // keeps trace of the scope's life-cycle }
            ...
   
}

// changed the on method to launched in the given LiveKordEntity's scope by default
inline fun <reified T : Event> LiveKordEntity.on(scope: CoroutineScope = this, noinline consumer: suspend (T) -> Unit) =
   ...
   

See #270, The proposed solution is to provide LiveEntitys with their own coroutine scope so they can manage their own lifecycle.
The user should be able to provide a dispatcher, that we will use for the entity to create a supervisor job that manages coroutines launched under it (children).

EDIT: You can safely remove the running state variable as we now use isActive instead.
The changes are not limited to this file, the change should affect any entity that extends the abstract class.

@HopeBaron HopeBaron changed the base branch from 0.7.x to live-entity-life-cycle May 2, 2021 08:20
@HopeBaron HopeBaron merged commit 0cb5126 into kordlib:live-entity-life-cycle May 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants