Make structures, waypoints, and tagmembers commands return lists
#2260
Conversation
|
@kostmo There's obviously still a bunch of work to do to fix up more failing tests, but I'm curious if you have any feedback at this point. Does this seem like a reasonable direction? One thing I'm realizing is that this makes a lot more sense for |
|
However, I still don't really like the conceit of always returning a pair of the total number of waypoints and the waypoint with the requested index. What about providing two functions: Relatedly, @kostmo, do you know of a good reason why |
…aypoint(s)` pure functions
| isOrganism <- hastag item "organism"; | ||
| return $ inR item; | ||
| ); | ||
| scan down; |
There was a problem hiding this comment.
@kostmo the existing code called hastag item "organism" but then ignored the result. Was that a mistake?
There was a problem hiding this comment.
Oh, it probably is a mistake. It should return inL if it is not an organism.
|
|
||
| if (idx < total) { | ||
| if (idx < force bustCount) { |
There was a problem hiding this comment.
When I first converted this scenario it was failing, and running it with --autoplay revealed that it was failing because the base started running its program before the level was set up. The problem was that evaluating the definition of bustCount actually took multiple ticks, before the judge robot ever got around to executing its instant setup. So I wrapped it in a delay so it wouldn't be evaluated at definition time.
I'm just realizing now, however, that this means it's going to be re-evaluated every time we force it (IIRC we don't have memoizing delay). Perhaps a better idea would just be to wrap the entire program (including all the defs) in instant.
There was a problem hiding this comment.
For word-search (#999), I trapped the player behind a boulder until the playfield was set up :) Though I think that may have been before the instant command was available.
There was a problem hiding this comment.
Strangely, wrapping the entire program in instant doesn't seem to work! Not sure if this is a bug or what.
Closes #1997.
Change
structure,waypoint, andtagmembersto return lists; also makewaypoint,tagmembersandhastagpure functions, since their output within a given scenario will never change.More specifically:
waypoint : Text -> Int -> (Int * Int)to just return the waypoint with the given index (wrapping around), or throw an exception if there are no waypoints with the given name.waypoints : Text -> (rec l. Unit + Text * l)function that returns the list of all waypoints with a given name.structuretostructures : Text -> Cmd (rec l. Unit + (Int * Int) * l), returning the list of all structure locations of the given nametagmembersto have typetagmembers : Text -> (rec l. Unit + Text * l)hastagto have typehastag : Text -> Text -> BoolIn the vast majority of cases, this greatly simplified the code for various scenario setups and solutions, though there is still a bit of code duplication for things like calculating the length of a list. But even this can easily be improved once we have #495 and can make a standard library of list functions and import it within scenario code.
The reason I kept an indexing function for waypoints but not for structures or tags is that iterating through waypoints incrementally while doing other stuff in between seems like a common thing to want to do, and passing around a list of waypoints and manually indexing into it sounds tedious. However, for structures and tags this is much less common; typically we just want to get a list and then do something with all of them right away.