arkivanov / Minesweeper

Minesweeper game implemented in Kotlin and Compose Multiplatform
https://arkivanov.github.io/Minesweeper/
Apache License 2.0
75 stars 2 forks source link

Implementing the Stopwatch #15

Closed b0r1ngx closed 7 months ago

b0r1ngx commented 7 months ago

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

b0r1ngx commented 7 months ago

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

arkivanov commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

arkivanov commented 7 months ago

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 commented 7 months ago

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.

b0r1ngx commented 7 months ago

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)
    },
  2. 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 commented 7 months ago

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.

b0r1ngx commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

arkivanov commented 7 months ago

Thank you!