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

Implementing the Stopwatch #15

Merged
merged 17 commits into from
Mar 13, 2024
Merged

Conversation

b0r1ngx
Copy link
Contributor

@b0r1ngx b0r1ngx commented Mar 9, 2024

Hello, Arkadii, how i said previously i want to add a stopwatch feature.

No matter how easy the task is, I decided to study different types of implementation, and think found one good idea (source: https://akjaw.com/kotlin-coroutine-flow-stopwatch-part1). I'm partly implemented same idea in core.

When i start implement and integrate logic in-to this project im get stuck & overwhelmed (also with this mark, I'm created TODO's in code with some ideas), with thing of convenient and correct creating and usage the Stopwatch in a project.

Now the feature is not fully-integrated in the Game (it's not work when GameState goes to Failed or Win)

I will be appreciate to receive feedback from you, since you are the creator of the project and also have more experience in architectural design

1) Added dependencies of kotlinx:
1.1) -datetime (to get time)
1.2) -coroutines-core (to count time with coroutines)

2) Implemented core logic of Stopwatch (wanted to make it like module - but make it later, because not so easy, for me, to adopt Stopwatch in project)

3) Using Stopwatch in project (now only start, when user pop first tile, and stop (reload) when user clicks `RestartButton`). To use Stopwatch in project:
3.1) include StopwatchViewModel in GameComponent
3.2) using collectAsState on the Flow of current tick of Stopwatch

Current bugs:
4) Stopwatch not stops even if user goes to GameStatus
4.1) WIN
4.2) FAILED
@b0r1ngx
Copy link
Contributor Author

b0r1ngx commented Mar 9, 2024

For now I'm want to write some tests for the Stopwatch.

@arkivanov
Copy link
Owner

Thanks for working on this! I suggest to not over-engineer the timer. We can simply do the following.

  1. Add a new property to GameState - e.g. GameState.timer: Int with the default value 0.
  2. Add a new Intent in GameStore - Intent.TickTimer.
  3. Handle the new Intent - increment the timer value if its less than 999, or do nothing otherwise.
  4. Add a new dependency: com.arkivanov.essenty:lifecycle-coroutines:2.0.0-alpha02.
  5. In DefaultGameComponent - create a private scope using com.arkivanov.essenty.lifecycle.coroutines.coroutineScope function.
  6. In DefaultGameComponent - just start/stop a coroutine job when GameState.gameStatus switches to/from STARTED. Inside that job - run an infinite loop with delay(1.seconds) and send Intent.TickTimer to GameStore.

Here is a rough example of how we can start/stop the job. It's from the top of my head, haven't tested it.

internal class DefaultGameComponent(...) {
    private val scope = coroutineScope()

    init {
        scope.launch {
            store.stateFlow
                .map { it.gameStatus == GameStatus.STARTED }
                .distinctUntilChanged()
                .collectLatest { isStarted ->
                    if (isStarted) {
                        while (true) {
                            delay(1.seconds)
                            store.accept(Intent.TickTimer)
                        }
                    }
                }
        }
    }
}

Optionally, we can cover this logic with integration tests.

  1. Pass the main CoroutineContext in DefaultGameComponent class and use it in coroutineScope.
  2. Create DefaultGameComponentTest class in commonTest source set.
  3. Create ComponentContext using DefaultComponentContext.
  4. Create StoreFactory using DefaultStoreFactory.
  5. Use StandardTestDispatcher and pass it as ComponentContext to DefaultGameComponent.
  6. Pass TestCoroutineScheduler to StandardTestDispatcher to control timings - you can advance the TestCoroutineScheduler timer.

Let me know if you have any questions!

@b0r1ngx
Copy link
Contributor Author

b0r1ngx commented Mar 9, 2024

Glad to see you with information,

it seems that previously in project flow (and coroutines) isn't used at all, so using store.stateFlow doesn't allowed. If we doesn't import coroutines, how i already made before you write so now we can use flows, but did i must use some extension functions, to collect here State as Flow?

I tried to implement other logic inside scope, instead of that example that you give (doesn't think that it is right, or an idea inside is right, but i wanna just run a project with that):

    init {
        scope.launch {
            state
                .map { it.gameStatus == GameStatus.STARTED }
                .let { isStarted ->
                    if (isStarted.value)
                        while (true) {
                            delay(1.seconds)
                            store.accept(Intent.TickTimer)
                        }
                }
        }
    }

Its compiled & builded, but at start of the app starts run I get this error:
[MVIKotlin]: Main thread id is undefined, main thread assert is disabled
Exception in thread "main" java.lang.IllegalStateException: Module with the Main dispatcher is missing. Add dependency providing the Main dispatcher, e.g. 'kotlinx-coroutines-android' and ensure it has the same version as 'kotlinx-coroutines-core'

tried to fix it with (at build.gradle.kts, composeApp module):

        jvmMain.dependencies {
            implementation(compose.desktop.currentOs)
            implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.8.0")
//            implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core-macosArm64:1.8.0")
        }

Seems like the error is raising because of usage: com.arkivanov.essenty:lifecycle-coroutines:2.0.0-alpha02

I'll also try to find information at your Essenty-repo, but doesn't understand how to fix it

@b0r1ngx
Copy link
Contributor Author

b0r1ngx commented Mar 9, 2024

Ahh.. lets I just commit, to you see more things, that i done by your guide

2) Now the version is crashed with error: [MVIKotlin]: Main thread id is undefined, main thread assert is disabled
                                          Exception in thread "main" java.lang.IllegalStateException: Module with the Main dispatcher is missing. Add dependency providing the Main dispatcher, e.g. 'kotlinx-coroutines-android' and ensure it has the same version as 'kotlinx-coroutines-core'
@arkivanov
Copy link
Owner

Oh, right. To fix the crash you need to add to jvm.main the following dependency: https://github.com/Kotlin/kotlinx.coroutines/tree/master/ui/kotlinx-coroutines-swing.
And please ignore the "Main thread id is undefined" message, it's just a warning.

@b0r1ngx
Copy link
Contributor Author

b0r1ngx commented Mar 9, 2024

Oh, right. To fix the crash you need to add to jvm.main the following dependency: https://github.com/Kotlin/kotlinx.coroutines/tree/master/ui/kotlinx-coroutines-swing. And please ignore the "Main thread id is undefined" message, it's just a warning.

Ah.. at the first my google-search-attempt I found this (https://github.com/JetBrains/compose-multiplatform/releases/tag/v1.1.1), but thinks, ehh, swing should it help us? - thinks no:c

Your advice is again helps! Its runs without crash, but yea, timer logic in scope is not works.

composeApp/build.gradle.kts Outdated Show resolved Hide resolved
composeApp/build.gradle.kts Outdated Show resolved Hide resolved
composeApp/build.gradle.kts Outdated Show resolved Hide resolved
@b0r1ngx
Copy link
Contributor Author

b0r1ngx commented Mar 11, 2024

Optionally, we can cover this logic with integration tests.

  1. Pass the main CoroutineContext in DefaultGameComponent class and use it in coroutineScope.
  2. Create DefaultGameComponentTest class in commonTest source set.
  3. Create ComponentContext using DefaultComponentContext.
  4. Create StoreFactory using DefaultStoreFactory.
  5. Use StandardTestDispatcher and pass it as ComponentContext to DefaultGameComponent.
  6. Pass TestCoroutineScheduler to StandardTestDispatcher to control timings - you can advance the TestCoroutineScheduler timer.

Let me know if you have any questions!

Hello, I have a question about changes that we make for tests:

  1. Passing CoroutineContext in DefaultGameComponent, should we also change signatures of DefaultRootComponent and function DefaultRootComponent(componentContext: ComponentContext, storeFactory: StoreFactory): DefaultRootComponent, to provide a DefaultRootComponent with CoroutineContext ?
    1.1. Or we can use something like:
gameComponentFactory = { ctx, settings ->
    DefaultGameComponent(componentContext = ctx, storeFactory = storeFactory, settings = settings, coroutineContext = SupervisorJob() + Dispatchers.Default)
},
  1. If the first question is correctly resolved by me (check next commit), I'm done with your guide and now can start writing some integration tests.

@arkivanov
Copy link
Owner

Or we can use something like

Thanks for the update! The current approach looks good. DefaultRootComponent doesn't require CoroutineContext, so there is no need to pass it there. And no need to change DefaultRootComponent function for now.

2) Wrote first test for stopwatch
@b0r1ngx
Copy link
Contributor Author

b0r1ngx commented Mar 12, 2024

Hello, I'm trying to work with TestCoroutineScheduler, and understand how to shift it, shifting is happening good, but DefaultGameComponent itself looks do nothing, timer is just always on start (at zero)

I'm tried to use:

  1. advanceTimeBy for coroutineScheduler
  2. delay() in runTest { ... }

Can you please give some suggestions, what we can do?

I'm tried to change gameStatus with next lines of code:

var gameState = gameComponent.state.value
gameState = gameState.copy(gameStatus = GameStatus.STARTED)

Maybe I need to use something like you do at GameStoreTest, for change GameState.gameStatus, like click function?

@arkivanov
Copy link
Owner

Can you please give some suggestions, what we can do?

It's hard to say without the code. I will try locally on your branch and update here. Thanks for working on this!

@arkivanov
Copy link
Owner

The following tests seems to work fine.

   @Test
    fun GIVEN_created_WHEN_cell_clicked_THEN_status_STARTED() {
        clickCellPrimary(x = 0, y = 0)

        assertEquals(GameStatus.STARTED, state.gameStatus)
    }

    @Test
    fun GIVEN_created_WHEN_cell_clicked_THEN_timer_shows_0() {
        clickCellPrimary(x = 0, y = 0)

        assertEquals(0, state.timer)
    }

    @Test
    fun GIVEN_game_started_WHEN_one_second_passed_THEN_timer_shows_1() {
        clickCellPrimary(x = 0, y = 0)

        coroutineScheduler.advanceTimeBy(1.seconds)
        coroutineScheduler.runCurrent()

        assertEquals(1, state.timer)
    }

    private fun clickCellPrimary(x: Int, y: Int) {
        gameComponent.onCellTouchedPrimary(x = x, y = y)
        gameComponent.onCellReleased(x = y, y = y)
    }

@b0r1ngx
Copy link
Contributor Author

b0r1ngx commented Mar 12, 2024

It's an awesome possibility to start with, thanks for equip me with that 👨‍💻🎨

2) Deprecate using stopwatch in strings
@b0r1ngx b0r1ngx requested a review from arkivanov March 12, 2024 21:08
2) shift timer to different values
@arkivanov arkivanov merged commit a32a344 into arkivanov:master Mar 13, 2024
1 check passed
@arkivanov
Copy link
Owner

Thank you!

@b0r1ngx b0r1ngx deleted the b0r1ngx/stopwatch branch March 14, 2024 14:51
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