swarm-game / swarm

Resource gathering + programming game
Other
820 stars 51 forks source link

Return real result in Web API `/code/run` #1426

Open xsebek opened 10 months ago

xsebek commented 10 months ago

Currently we don't wait for the evaluation to finish:

❯ curl --header "Content-Type: text/plain;charset=utf-8" --data "move" localhost:5357/code/run
Sent

Getting back the result would make it possible to control Swarm remotely... or at least from different window.

xsebek commented 10 months ago

@byorgey @kostmo I am not sure what the best way to pass back the result from AppState to Web API is.

Maybe an MVar (Either Text Text) would do, as the web could just wait for it to be filled by the controller.

kostmo commented 10 months ago

Perhaps this:

For each /code/run request that the web API makes, the web process generates a unique integer ID. This ID gets passed as an additional field of the RunWebCode constructor.

When the game process receives the request, it shall populate a game-internal Map of these integers to their execution statuses (which can be one of two possible constructors: InProgress or Complete <result>).

The web process then uses a different web API endpoint to poll the state of the game-internal execution-status Map. A web UI would use this to display "progress spinners", updating at intervals of, say, 1 second. Alternatively (and probably less usefully), "blocking" could be done within the web process until the command-Complete state is observed for the requested ID, with some timeout (also bad, because the execution time of a command can be arbitrarily long).

xsebek commented 10 months ago

@kostmo there is a type in Servant for sending values as they are produced - StreamGet.

I played with it and it's not too bad - just do the blocking IO in Effect, send result in Yield and then either Stop or recurse.

I think this would be more efficient, AFAIK we do not allow running other commands while the last one is not finished. If that is so, should we really have a Map if there is only one? We could also add that UID to REPL history. 🤔

kostmo commented 10 months ago

we do not allow running other commands while the last one is not finished

Correct, and in fact now we could indicate this circumstance to the caller, by providing a Rejected state:

data RejectionReason = AlreadyRunning | ParseError String
data WebInvocationState = Rejected RejectionReason | InProgress | Complete Value

The more I think about it, the less appealing it seems to do blocking in the web server. I'm not sure I fully understand StreamGet, but I like that the state of the swarm game, thus far, is strictly read-only via IO. Also I'm thinking about edge cases in the web browser, such as possibly having multiple tabs open for the same game, that would accidentally allow multiple overlapping commands to be issued.

xsebek commented 10 months ago

such as possibly having multiple tabs open for the same game, that would accidentally allow multiple overlapping commands to be issued

@kostmo I don't understand how that would happen as in that case we would just reject it.

Anyway I like the datatype suggestions and agree it would be nice to keep the state readonly if possible.

I have to read more Servant docs to determine which path is better, but I hope one MVar might be a small price for good user experience. I will try to hack on it over the weekend and we shall see what works.

byorgey commented 2 weeks ago

it would be nice to keep the state readonly if possible

I would go further and say we must keep the state read-only at all costs! There should be a single code path for updating the game state --- the event handler. Anything else would just be asking for trouble.