search

mattgallagher92 / safe-consequences

A SAFE stack app for playing consequences, the classic pen-and-paper game
1 stars 0 forks source link

Join a room #2

Closed mattgallagher92 closed 3 years ago

mattgallagher92 commented 3 years ago

As a player I want to join a room So that I can play with people I know

mattgallagher92 commented 3 years ago
mattgallagher92 commented 3 years ago

On the branch for this issue, I've refactored the code (commits 4a47ac97f20e18add74b6d4b9e7fabb1e67ffdcd and 6a7a3c377e78bde43049e3eaf9b58c15b7dbbd34) to bring it more in line with the advice in an insightful comment made by Christer van der Meeren on an issue in the Fabulous GitHub repo, and the Reddit comment by Richard Feldman that it refers to. The gist of what I've been doing is moving page data into the root Model, rather than having it associated to different cases of the Page discriminated union.

The natural conclusion of this would be to move all data from cases into the page model, but I hit a snag when doing so: after 6a7a3c377e78bde43049e3eaf9b58c15b7dbbd34, the last piece of data associated to a union case is the Room associated with a Lobby Page. If I move this to the root Model, I'd need to make it a Room option, since it's not defined in all cases. The problem that then arises is that it would be invalid to be in a Lobby when Room was None. I would no longer be making illegal states unrepresentable, which brings its own problems.

My conclusion is that I should leave the Room data associated with the Lobby Page (and possibly other Page types in future). I can make a function if I need to access the Room option from a Page. This feels like the best option to me: the previous refactors had the effect of removing complexity; a refactor now would add complexity, which obviously is not desirable.

mattgallagher92 commented 3 years ago

Asking a player for the ID of the room that they want to join, then asking for their name and only then showing them join the room is consistent with netgames.io. They validate that the given room ID corresponds to an actual room before showing the username page. I'll use the same flow.

mattgallagher92 commented 3 years ago

In a recent commit I mentioned that I could simplify things by using FsToolkit.ErrorHandling. Unfortunately, I seem to be having issues with package imports when trying to use Paket to add it via the command suggested in the SAFE docs. I'm not sure whether the issue is with Paket, Visual Studio Code, Ionide or me, but it's going to take a bit of time to unpick. I'll create a separate issue for this and move on.

Error message on import declaration

mattgallagher92 commented 3 years ago

For MVP: no automatic updates; refresh the page to update.

Refreshing doesn't work: it causes the player to return to the homepage. Investigate solutions. Some ideas:

mattgallagher92 commented 3 years ago

In 206d8a0c2de99e8ed4a254a0cf1633d209f57e54 I used a query string in the URL to store the room ID and user ID once the player reached the lobby. The user can refresh the page and the app will then parse the IDs and attempt to reconnect to the lobby.

I have some reservations about this:

I think it's working well enough for now and that all the slight weirdness of refreshing the page and being shown different page as the app loads is okay for the MVP, but automatic updates are almost certainly going to want to be one of the first improvements I focus on after the MVP is done.

mattgallagher92 commented 3 years ago

Note: another issue with the current implementation is that the client reconnects to the lobby immediately after joining it: a new URL is pushed to the stack, triggering the urlUpdate function which issues the Reconnect command. The update speed is quick enough that this isn't really an issue in terms of performance (particularly as the user needs to parse the new page), but it's obviously a bit wasteful. I'm not going to do anything about it for now.