Cache results of setup_controls so we don't send redundant controls setup to the client

[tcr]

The ClientImpl::setup_controls method in mercutio-client/src/client.rs will send a new "controls" payload to the client with the following info:

1. Which key combinations the Frontend should listen to
2. Which buttons the Frontend should display
3. Which buttons are highlighted (i.e. representing the result of the latest action).

Because we already know the previous ClientCommand::Controls we sent to the client, it would be helpful if we could

1. Cache the most recently sent ClientCommand::Controls data in pub struct Client, and
2. Not send a new payload if this structure hasn't changed.

There's a few ways to do it, but the trivial way would be to just track the previously sent information and then compare it with the new packet.

One option here is to extract the "keys" and "buttons" fields of ClientCommand::Controls into a "ControlState" struct, so we don't have to match against the ClientCommand enum to compare these values. Then we can change ClientCommand::Controls to ClientCommand::Controls(ControlState) rather than imbedding the data directly.

[briankabiro]

Hey, @tcr. Apologies for the delayed response.

I've been having issues implementing this fully: realized that I need to improve my knowledge of Rust to do it well. I'm going to raise a PR so that you can track my progress and offer some guidance. 😅

Have opened a PR for #45 and marked it as WIP for the same reason.

[tcr]

Hey @briankabiro no problem! Sorry for my late response too.

I’m happy to work through a task with you! I think this issue will be easier to start with. Let me describe a good staring point.

In edit-common/src/commands.rs, there is an enum UserToFrontendCommand which has a variant named Controls. It has the properties keys and buttons. If you pull those out into a new struct Controls (with those fields), then change UserToFrontendCommand::Controls to just have one argument, the new struct Controls, then we have a path forward to do the rest of this task. We can land a PR that just makes this change, and then follow up with how we can use this to stop sending so many updates to the front end.

Take a crack at that and submit a PR, if you’re interested. If you run “./x.rs test” it will compile everything that relies on it and run the test suite. You’ll probably need to change code elsewhere in the project that reads from the Controls enum too, but hopefully it will be straightforward. And just push your code and let me know if you get stuck. Hope this is helpful :)

[briankabiro]

Thanks for the pointers and reaching out! Getting to work on it and then update if I run into any issues. 👍

[tcr]

@briankabiro So now that Controls the struct is broken out, it can be stored directly on an object.

https://github.com/tcr/edit-text/blob/master/edit-client/src/client.rs#L304-L331

If you look at fn setup_controls(), it creates a Controls { keys, buttons } object (and wraps it in UserToFrontendCommand::Controls and sends it off. If we can store the most recently sent Controls struct, and each time setup_controls is called we check if the old Controls struct and the new one are equal, then we can decide only to call self.send_client when we create a Controls object that has been changed. Thus cutting down on unnecessary packets sent to the client.

1. I would add a last_controls: Option<Controls>, property to pub struct Client.
2. Everywhere a new struct Client is declared, last_controls can be set to the value None.
3. Conditional logic like if Some(controls_object) != self.last_controls { ... } can avoid you having to explicitly pattern match against None or Some, we just care about equality.

[briankabiro]

Appreciate all the help, @tcr. Was strapped for time, but working on this now. 💯

[briankabiro]

Think we can close this fella now. 😃