Index

Robots?

Or maybe some screen navigation first.

I'll start out as before, with inter-screen navigation options.

From the arena, users will need to be able to exit back to the main menu. This would cause the arena to be unloaded, so a confirmation from users first would be polite. It would also be convenient for them to be able to access the settings directly (without unloading the arena). But that wouldn't pause the arena so a warning about that should be shown on the settings screen when accessed while the arena is loaded.

Feature: Arena Screen
In order to battle robots how and when they want
Users of Rolbots Arena should be able to
Exit from the arena back to the main menu after confirming, and also access the settings screen from the arena screen.

Scenario: Navigation options visible on arena screen

Given a user who is at the arena screen
Then the user can see the following options:
    * ExitArena
    * Settings

Scenario: Use the Main Menu option to exit to the main menu

Given a user who is at the arena screen
When the user chooses the exit arena option
Then the confirm exit arena screen is shown next

Scenario: Confirming exit arena

Given a user who is at the confirm exit arena screen
When the user chooses the really exit option
Then the arena is unloaded
And the main menu screen is shown next

Scenario: Confirm options

Given a user who is at the confirm exit arena screen
Then the user can see the following options:
    * ReallyExit
    * Remain
And the user is warned that exiting the arena will unload it

Scenario: Remaining in the arena

Given a user who is at the confirm exit arena screen
When the user chooses the remain option
Then the arena is not unloaded
And the arena screen is shown next

Some new scenarios for the Settings feature are required too:

Scenario: Option to return from settings to arena

Given a user who is at the arena screen
When the user enters the settings screen
Then the user can see the following options:
    * Arena
And the arena is not unloaded
And the user is warned that the arena is not paused

Scenario: Use the Arena option to return to the arena

Given a user who is at the settings screen
When the user chooses the arena option
Then the arena screen is shown next

For simplicity I shall change the option for returning from the settings screen to the arena or main menu to be called Back in both cases.

Now for some step implementations:

let [<Given>]``a user who is at the arena screen`` () =
    let controller = Controller UserFamiliarity.New
    controller.DoAction UserAction.Continue
    controller.FinishedLoading()
    controller
    
let [<Given>]``a user who is at the confirm exit arena screen`` () =
    let controller = Controller UserFamiliarity.New
    controller.DoAction UserAction.Continue
    controller.FinishedLoading()
    controller.DoAction UserAction.ExitArena
    controller

let [<Then>]``the arena (is|is not) unloaded`` unloaded (controller:Controller) =
    let loaded =
        match unloaded with
        | "is not" -> true
        | _ -> false
    Assert.Equal(loaded, controller.IsLoaded)

Plus some existing common steps get the expected additional cases.

I'll add the tests for the changes to Settings first. This can be done by extending the existing tests to cover multiple ways of getting to the settings screen.

let controllerSM () =
    let controller = Controller UserFamiliarity.Returning
    controller.DoAction UserAction.Continue
    controller.DoAction UserAction.Settings
    controller
    
let controllerSA () =
    let controller = Controller UserFamiliarity.New
    controller.DoAction UserAction.Continue
    controller.FinishedLoading()
    controller.DoAction UserAction.Settings
    controller

let settingses = 
    seq{
        yield testSolo controllerSM
        yield testSolo controllerSA
    }

let [<Theory; MemberData("settingses")>]``option to return from settings is present`` c =
    let controller:Controller = c()
    let options = [UserAction.Back]
    Assert.Equal<IEnumerable<UserAction>>(options, controller.AvailableActions)

let navData =
    seq{
        yield testPair (controllerSM,Screen.MainMenu)
        yield testPair (controllerSA,Screen.Arena)
    }
    
let [<Theory; MemberData("navData")>]
    ``basic settings navigation works: back from settings reached from screen [original] goes to screen [next]`` original next =
    let controller:Controller = original()
    controller.DoAction UserAction.Back
    Assert.Equal(next, controller.CurrentScreen)
    
let [<Theory; MemberData("settingses")>]``other actions should do nothing on Settings`` c =
    checkUnavailableActions c
    let cycle () =
        let controller = controllerSM()
        controller.DoAction UserAction.Back
        controller.DoAction UserAction.Settings
        controller
    checkUnavailableActions cycle

For example, basic settings navigation works: back from settings reached from screen [original] goes to screen [next] is a function which is used for two test cases, one which reaches the settings screen via the main menu and checks that Back goes to the main menu, and the other which reaches settings via the arena and checks that Back goes to the arena.

Naturally the arena cases currently fail. But the main menu cases pass. That probably means I did it right. Now I need to have the controller remember the previous screen so Back can go back.

    member __.DoAction action =
        prior <- currentScreen
        currentScreen <-
        match action with
        | _ when not (List.contains action __.AvailableActions) -> currentScreen
        | UserAction.Settings   -> Screen.Settings
        | UserAction.Arena      -> Screen.ArenaLoading
        | UserAction.Back       -> prior
        | UserAction.Continue   ->
            match currentScreen, userFamiliarity, loadFinished with
            | Screen.Title, New, _          -> Screen.ArenaLoading
            | Screen.Title, Returning, _    -> Screen.MainMenu
            | Screen.ArenaLoading, _, true  -> Screen.Arena
            | _ -> currentScreen

No, that's not right. It's changing the value of prior too soon. And if I set it after, it would be doing it too late. I need to briefly remember it while it's being updated:

    member __.DoAction action =
        let leave = currentScreen
        currentScreen <-
            match action with
            | _ when not (List.contains action __.AvailableActions) -> currentScreen
            | UserAction.Settings   -> Screen.Settings
            | UserAction.Arena      -> Screen.ArenaLoading
            | UserAction.Back       -> prior
            | UserAction.Continue   ->
                match currentScreen, userFamiliarity, loadFinished with
                | Screen.Title, New, _          -> Screen.ArenaLoading
                | Screen.Title, Returning, _    -> Screen.MainMenu
                | Screen.ArenaLoading, _, true  -> Screen.Arena
                | _ -> currentScreen
        prior <- leave

Something's still wrong... oh I forgot the user needs to Continue from the Arena Loading screen to get to the Arena, so the setup is wrong.

With that easily fixed, I still have the problem of the Back option being missing when going from Arena to Settings... oh because it's not getting there because I never added the option to get to Settings from Arena because I'm doing the Settings tests first. Oops, I did that out of order.

let controllerA () =
    let controller = Controller UserFamiliarity.New
    controller.DoAction UserAction.Continue
    controller.DoAction UserAction.Arena
    controller.FinishedLoading()
    controller.DoAction UserAction.Continue
    controller

let controllerE a () =
    let controller:Controller = a()
    controller.DoAction UserAction.ExitArena

let [<Fact>]``basic arena actions are present`` () =
    let controller = controllerA()
    let options = [UserAction.ExitArena;UserAction.Settings]
    Assert.Equal<IEnumerable<UserAction>>(options, controller.AvailableActions)

There, now I can go add those. (controllerE will be used later for exit confirmation tests.)

let [<Theory; MemberData("navDataA")>]
    ``basic arena navigation works`` action next =
    let controller:Controller = controllerA()
    controller.DoAction action
    Assert.Equal(next, controller.CurrentScreen)

Once the minor change to make that pass is done, the settings test which was failing is also passing. But there's plenty more of those to do...

After fixing up controllerE() to do what I want, I have my next unit test:

    
let [<Fact>]``confirm exit arena actions and warning are present`` () =
    let controller:Controller = controllerE()
    let options = [UserAction.ReallyExit;UserAction.Remain]
    Assert.Equal<IEnumerable<UserAction>>(options, controller.AvailableActions)
    Assert.Equal("exiting the arena will unload it", controller.Status)

The first part is easily implemented. The second part... well the status is a series of dots.

    member __.Status =
        match currentScreen with
        | ConfirmExitArena -> "exiting the arena will unload it"
        | _ ->
        status <- status + "." //this is just a placeholder because there's nothing to check status of
        status

Easy! Arena Exit Confirmation navigation is simple to test and implement too. I won't include the code. After that is testing that the arena becomes unloaded. That's simple too, but slightly different to other tests, so I'll include it.

let loadedData =
    seq{
        yield testPair (UserAction.ReallyExit,false)
        yield testPair (UserAction.Remain,true)
    }
        
let [<Theory; MemberData("loadedData")>]
    ``exit arena unloading works`` action loaded =
    let controller:Controller = controllerE()
    controller.DoAction action
    Assert.Equal(loaded, controller.IsLoaded)

The quick and dirty way of making that work is kinda evil:

    member __.DoAction action =
        let leave = currentScreen
        currentScreen <-
            match action with
            | _ when not (List.contains action __.AvailableActions) -> currentScreen
            | UserAction.Settings   -> Screen.Settings
            | UserAction.Arena      -> Screen.ArenaLoading
            | UserAction.ExitArena  -> Screen.ConfirmExitArena
            | UserAction.ReallyExit -> 
                                        loadFinished <- false
                                        Screen.MainMenu
//snip

    member __.FinishedLoading() =
        loadFinished <- true

Yeah I've got a side effect in the middle of the match block. Not great form.

Given that this should have fixed the corresponding behaviour test, I was worried when it did not. It turned out that the new scenario did not specify from where the user entered the settings screen, and Main Menu was still assumed and tested. So I had to fix that by adding a parameter to that step:

let [<Given>]``a user who is at the settings screen from the (main menu|arena) screen`` origin =
    match origin with
        | "main menu" ->
            let controller = Controller UserFamiliarity.Returning
            controller.DoAction UserAction.Continue
            controller.DoAction UserAction.Settings
            controller
        | "arena" ->            
            let controller = Controller UserFamiliarity.New
            controller.DoAction UserAction.Continue
            controller.DoAction UserAction.Arena
            controller.FinishedLoading()
            controller.DoAction UserAction.Continue
            controller.DoAction UserAction.Settings
            controller
        | _ -> invalidOp ("Unknown settings entrypoint: " + origin)

Now I need to add tests for the warning about Settings not being paused in the Arena. There should not be a warning when Settings are accessed from the Main Menu.

Currently the 'user is warned' behavour tests is a stub, so...

let [<Then>]``the user is warned that (.*)`` warning (controller:Controller) =
    Assert.Equal(warning, controller.Status)

Now I'll get more behaviour test failures and I can write the unit tests.

Here I briefly confused myself because I already wrote the unit test for the unload-arena warning, so now that the behaviour test is working properly, it already passes

Here's the unit test for the pause warning:

let warnData =
    seq{
        yield testPair (controllerSM,"")
        yield testPair (controllerSA,"the arena is not paused")
    }
    
let [<Theory; MemberData("warnData")>]``pause warning shown iif Settings was accessed from Arena`` c warning =
    let controller:Controller = c()
    Assert.Equal(warning, controller.Status)

Both parts fail! Nice. Making one part pass is easy, but for both I need another condition in the Status member.

    member __.Status =
        match currentScreen, prior with
        | ConfirmExitArena, _ -> "exiting the arena will unload it"
        | Screen.Settings, Screen.Arena -> "the arena is not paused"
        | Screen.Settings, _ -> ""
        | _ ->
        status <- status + "." //this is just a placeholder because there's nothing to check status of
        status

That does nicely. Buuuut... I know there's some bad implementation lying around, and I think I have an idea of a unit test which could catch it.

let [<Theory; MemberData("navData")>]
    ``advanced settings navigation works: after making wrong actions, back from settings reached from screen [original] goes to screen [next]`` original next =
    property{
        let controller:Controller = original()
        let options = controller.AvailableActions
        let! action = GenX.auto<UserAction> |> GenX.notIn options
        controller.DoAction action
        controller.DoAction UserAction.Back
        return next = controller.CurrentScreen
    } |> Property.check

This makes sure that performing a wrong action (proved by other tests to be ignored) does not affect the right action (returning to arena or main menu).

All it needs to set right is a simple condition on updating the value of prior, so that a failed screen change doesn't overwrite it:

        if currentScreen <> leave then prior <- leave

Moving Time

It's time to do some refactoring, and I have a big idea. All these tests are related to one main thing: Navigation. There's not really any non-navigation behaviour here. So I think I should move all of the tests I've written so far into a Navigation folder/namespace for easy sorting. Then I have a good reason to put subsequent tests and behaviours into separate files.

I've also renamed the UserAction type to Navigation, because it doesn't really represent actions.

Another refactoring idea I had was to reduce the number of mutable fields by using records instead:

type ArenaLoading = {LoadFinished:bool}

type Screen =
| Title
| MainMenu
| ArenaLoading of ArenaLoading
| Settings
| Arena
| ConfirmExitArena
//snip
    member __.AvailableActions =
        match currentScreen with
        | Screen.Title          -> [Continue]
        | Screen.MainMenu       -> [Arena;Settings]
        | Screen.Settings       -> [Back]
        | Screen.Arena          -> [ExitArena;Settings]
        | Screen.ConfirmExitArena -> [ReallyExit;Remain]
        | Screen.ArenaLoading {LoadFinished=true}   -> [Continue]
        | Screen.ArenaLoading {LoadFinished=false}   -> []
//snip various similar changes
    member __.FinishedLoading() =
        currentScreen <- Screen.ArenaLoading {LoadFinished=true}

    member __.IsLoaded =
        match currentScreen with
        | Screen.ArenaLoading {LoadFinished=f} -> f
        | Screen.Arena -> true
        | _ -> false

And of course a few minor changes to tests. I initially somehow got FinishedLoading() wrong, leaving it at {LoadFinished=false} possibly due to copypaste, which broke a third or so of the tests.

IsFinished is still broken for the Settings screen (it always comes out as true) but I'll fix that after another refactor (which I should have done first.)

Before doing that I renamed Navigation.Arena to Navigation.EnterArena for clarity and consistency. Navigation.Settings becomes Navigation.OpenSettings.

The next step is removing prior.

type Screen =
| Title
| MainMenu
| ArenaLoading of ArenaLoading
| Settings of Settings
| Arena
| ConfirmExitArena
and Settings = {Prior:Screen}

It gets used in DoAction as follows: (with a lot snipped)

            | Navigation.OpenSettings   -> Screen.Settings {Prior=currentScreen}
//...
            | Navigation.Back       ->
                match currentScreen with
                | Screen.Settings {Prior=prior} -> prior
                |_ -> currentScreen

And now I can resolve the issue in IsLoaded:

    member __.IsLoaded =
        match currentScreen with
        | Screen.ArenaLoading {LoadFinished=f} -> f
        | Screen.Arena                         -> true
        | Screen.Settings {Prior=Screen.Arena} -> true
        | _ -> false

Unfortunately there's a problem with the (arena loading|main menu|settings|arena|confirm exit arena) screen is shown next

I need to alter it to treat the two cases of settings separately so they can be compared (because equality compares all fields.)

let [<Then>]``the (arena loading|main menu|settings|arena|confirm exit arena)(| from )(|main menu| from arena) screen is shown next`` nextscreen _ prior (controller:Controller) =
    Assert.Equal( 
        match nextscreen with
        | "arena loading" -> Screen.ArenaLoading {LoadFinished=false}
        | "main menu" -> Screen.MainMenu
        | "settings" ->
            match prior with
            | "main menu" -> Screen.Settings {Prior=Screen.MainMenu}
            | "arena" -> Screen.Settings {Prior=Screen.Arena}
            |_ -> invalidOp ("Unknown or invalid prior scene: " + prior)
        | "arena" -> Screen.Arena
        | "confirm exit arena" -> Screen.ConfirmExitArena
        | s -> invalidOp ("Unknown scene: " + s)
    , controller.CurrentScreen)

That should do it. It's a little worrying how long the function name is growing but it's test code so... shrug

Actually, I can do something about that. There's no significant reason why all the options must be hardcoded into the test function name. I can just match all strings of the correct length, like so:

let [<Then>]``the (.{4,13})(| from )(|main menu|arena) screen is shown next``
    (nextscreen:string) _ (prior:string) (controller:Controller) =
    Assert.Equal( 
        match nextscreen with

The only operation idfference is that now a new test with a screen that's not yet handled will result in a test failure with a helpful message rather than the test framework crashing unable to find an appropriate step function. So this is better. I made the same change for the common step function for DoAction.

Actually that's wrong, because I missed the long case 'confirm exit arena', and that currently overlaps with 'confirm exit ' + '' + 'arena'. So what I really need is:

let [<Then>]``the (.{3,17}\w)(| from )(|main menu|arena) screen is shown next``

This prevents it from matching a screen name ending in a space.

Pipelining

I added a couple of functions for convenience:

module Controller =
    let doAction action (controller:Controller) =
        controller.DoAction action
        controller

    let finishLoading (controller:Controller) =
        controller.FinishedLoading()
        controller

Now I don't have to explicitly return controller from all the behaviour steps:

let [<Given>]``a user who is at the confirm exit arena screen`` () =
    let controller = Controller UserFamiliarity.New
    controller  |> doAction Navigation.Continue
                |> finishLoading
                |> doAction Navigation.Continue
                |> doAction Navigation.ExitArena

It's kinda bad form in that it implies it's doing 'return new copy' rather than returning the original mutated.

Immutability

It's not actually that difficult to replace the Controller class with a simple record and change the functions which mutate it into functions which return a new record. The previous change was to prepare for this one.

module Controller =
    type ScreenState = {Familiarity:UserFamiliarity;Screen:Screen;Status:string}
    let Controller userFamiliarity =
        {Familiarity=userFamiliarity;Screen=Title;Status=""}

    let availableActions state =
        match state.Screen with
        | Screen.Title          -> [Continue]
        | Screen.MainMenu       -> [EnterArena;OpenSettings]
        | Screen.Settings _     -> [Back]
        | Screen.Arena          -> [ExitArena;OpenSettings]
        | Screen.ConfirmExitArena -> [ReallyExit;Remain]
        | Screen.ArenaLoading {LoadFinished=true}   -> [Continue]
        | Screen.ArenaLoading {LoadFinished=false}   -> []

    let nextScreen action state =
        match action with
        | _ when not (List.contains action (availableActions state)) -> state.Screen
        | Navigation.OpenSettings   -> Screen.Settings {Prior=state.Screen}
        | Navigation.EnterArena     -> Screen.ArenaLoading {LoadFinished=false}
        | Navigation.ExitArena  -> Screen.ConfirmExitArena
        | Navigation.ReallyExit -> Screen.MainMenu
        | Navigation.Remain     -> Screen.Arena
        | Navigation.Back       ->
            match state.Screen with
            | Screen.Settings {Prior=prior} -> prior
            |_ -> state.Screen
        | Navigation.Continue   ->
            match state.Screen, state.Familiarity with
            | Screen.Title, New                             -> Screen.ArenaLoading {LoadFinished=false}
            | Screen.Title, Returning                       -> Screen.MainMenu
            | Screen.ArenaLoading {LoadFinished=true}, _    -> Screen.Arena
            | _ -> state.Screen

    let doAction action state =
        {state with Screen=nextScreen action state}

    let finishLoading state =
        {state with Screen=ArenaLoading {LoadFinished=true}}

    let isLoaded state =
        match state.Screen with
        | Screen.ArenaLoading {LoadFinished=f} -> f
        | Screen.Arena                         -> true
        | Screen.Settings {Prior=Screen.Arena} -> true
        | _ -> false

    let statusMessage state =
        match state.Screen with
        | ConfirmExitArena -> "exiting the arena will unload it"
        | Screen.Settings {Prior=Screen.Arena} -> "the arena is not paused"
        | Screen.Settings _ -> ""
        | _ -> "."

With a few changes to tests to use the new structure, most of it passes. Except for the progress status, as I'm currently not mutating it.

Progress

While the ScreenState is storing a Status string for use in checkStatusChanged, there's currently no place to update it. Putting the mutation as a side effect in a getter is exactly what I'm trying to avoid with these changes. I could add a function which returns both the status and a new state but that's trying to do too many things all at once.

Since the changing status message is meant to reflect loading progress, but an immutable state record can't have undergone any loading, I need to add a function to represent this.

    let progressLoading state =
        {state with Status=state.Status+"."}

Then I need to change the recursive test function to use it.

let [<Fact>]``progress status updates every second for 6 seconds`` () =
    let state = controllerLR()
    let rec checkStatusChanged times previous =
        match times with
        | 0 -> ()
        | _ ->
        wait 1
        Assert.NotEqual<string>(previous.Status, state.Status)
        checkStatusChanged (times-1) (state |> progressLoading)
    checkStatusChanged 6 (state |> progressLoading)

Previously, it passed the status string to the next recursion, rather than the (altered) state object.

I also need to make a corresponding change to the relevant behaviour test step:

let [<When>]``the user waits for a second``(state:ScreenState) =
    wait 1
    state |> progressLoading

Naming

I renamed the Status field to ProgressStatus to avoid confusion with statusMessage (there was some).

I also renamed some functions with Action in them to Navigation, as well as updating parameter names and comments to say navigation or nav instead of action. They're not actually actions so they shouldnt be called that.

What's next?

I was about to start on the arena but instead I started learning Rust...

As much as I'm liking F# I think Rust will ultimately be more suitable. So stand by for a reboot of this project.