Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opaque robot type #212

Closed
byorgey opened this issue Oct 12, 2021 · 5 comments · Fixed by #303
Closed

Opaque robot type #212

byorgey opened this issue Oct 12, 2021 · 5 comments · Fixed by #303
Assignees
Labels
C-Moderate Effort Should take a moderate amount of time to address. G-Robots An issue having to do with robots. L-Language design Issues relating to the overall design of the Swarm language. S-Nice to have The bug fix or feature would be nice but doesn't currently have much negative impact. Z-Feature A new feature to be added to the game. Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality.

Comments

@byorgey
Copy link
Member

byorgey commented Oct 12, 2021

Currently, all commands which need to identify a robot (e.g. install, give, reprogram) take a string identifying the robot by name. We also store robots by name internally in the robotMap. However, this has several disadvantages: it's not very type safe (for example, are you supposed to give "base" "tree" or give "tree" "base"? And if you misspell the name of a robot, it fails at runtime instead of typechecking time), and necessitates frequent, relatively costly string comparisons.

The basic idea is to add a new type robot which is opaque to the user but is internally represented by Int. Here are my current thoughts:

  • Add a new base type robot.
  • Change the type of any commands that currently take or return the name of a robot:
    • give : robot -> string -> cmd ()
    • install : robot -> string -> cmd ()
    • reprogram : robot -> cmd a -> cmd ()
    • build : cmd a -> cmd robot
  • Note that robots can still have a name, which is displayed when they are viewed. But the name no longer needs to be unique.
  • However, note that the build command doesn't take a name any more; in many cases players are probably fine with giving it a descriptive variable name (e.g. lambda_getter <- build get_lambda), but never plan to view it so don't really care what its "display name" is.
  • However, we can add a command setname : string -> cmd () which lets a robot set its display name.
  • Internally, we can add a new type of Value, VRobot :: Int -> Value. It can pretty-print as something like r105 or <105>, it doesn't really matter that much.
  • What about robots that want to e.g. give something back to, say, the base? How can they get a reference to it? I propose adding a special variable parent : robot which is always a reference to a robot's parent (i.e. the robot that built it). So the code for building a robot to get you a thing might look like this:
    child <- build {move; thing <- grab; turn back; move; give parent thing}
    
    While we're at it we could also add a special global variable base : robot which is a reference to the base.
  • We might also want to add a way to get the results of previous commands entered at the REPL, so if you did build and forgot to bind the result to something, it's not lost forever.
@byorgey byorgey added Z-Feature A new feature to be added to the game. Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality. C-Moderate Effort Should take a moderate amount of time to address. S-Nice to have The bug fix or feature would be nice but doesn't currently have much negative impact. L-Language design Issues relating to the overall design of the Swarm language. G-Robots An issue having to do with robots. labels Oct 12, 2021
@polux
Copy link
Collaborator

polux commented Oct 21, 2021

What should parent return if the parent no longer exists? Should it return a handle to a non-existent robot or should it fail immediately?

@byorgey
Copy link
Member Author

byorgey commented Oct 21, 2021

I think it would be safe to return a handle to a non-existent robot (but I could be convinced otherwise). We would just have to make sure that robot handles are never reused, but that should be easy.

@byorgey
Copy link
Member Author

byorgey commented Oct 21, 2021

I don't think it should fail immediately if the parent doesn't exist. That would provide a way to instantly communicate a bit of information over any distance. =)

@xsebek
Copy link
Member

xsebek commented Oct 22, 2021

I would also like to have some function for robots to detect each other in the wild - say meet : cmd (() + robot). 🤖

@byorgey
Copy link
Member Author

byorgey commented Oct 22, 2021

Agreed! meet : cmd (() + robot) sounds great. I presume if there is more than one robot there, you just get an arbitrary one? Maybe later there could be a meetAll : cmd (list (robot)).

@xsebek xsebek mentioned this issue Nov 18, 2021
@byorgey byorgey self-assigned this Jan 31, 2022
@mergify mergify bot closed this as completed in #303 Mar 2, 2022
mergify bot pushed a commit that referenced this issue Mar 2, 2022
The basic idea of this change is to create a new `robot` type and use it to identify robots instead of `string` names.  Internally, a `robot` value is just a (unique) `Int`. 

Closes #212 .

This ended up turning into a sort of constellation of related changes.

- Add the `robot` type and change the type of various built-in functions which used to take a robot name so they now take a `robot` (`give`, `install`, `reprogram`, `view`, `upload`) and change `build` so it returns a `robot`.
- All internal data structures that store robots are now keyed by a unique (`Int`) robot ID rather than by name.
- Add a `setname` command for setting a robot's display name (which no longer needs to uniquely identify a robot).
- Import a big list of words which we can use to randomly pick names for robots, just for fun.  This is why the diff says "+31,050  -265"; I did not write 31 thousand lines of code.
- Add constants `base`, `parent`, and `self` for getting a `robot` value referring to the base, one's parent, and one's self, respectively.
- Top-level binders like `r <- build {move}` now export a variable binding which can be used in later expressions entered at the REPL; additionally, unlike Haskell, a binder can now appear as the last statement in a block.
- Fix the pretty-printer for `Value` by doubling down on our current strategy of injecting `Value`s back into `Term`s and then pretty-printing the result.  I am now convinced this is the Right Way (tm) to do this; it only required adding a couple additional kinds of `Term` which represent internal results of evaluation and cannot show up in the surface language (`TRef`, `TRobot`).
- Update the tutorial.
- While updating the tutorial, I noticed that #294 had introduced a bug, where the inventory display no longer updated when 0 copies of an entity are added to the inventory (as with `scan` + `upload`), so I fixed that by changing the way inventory hashes are computed.

I tried running the benchmarks both before & after this change.  I was hoping that it might speed things up to be using `IntMap` and `IntSet` instead of looking things up by `Text` keys in a `Map` all the time.  However, if I'm interpreting the results correctly, it seems like it didn't really make all that much difference, at least for the particular benchmarks we have.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Moderate Effort Should take a moderate amount of time to address. G-Robots An issue having to do with robots. L-Language design Issues relating to the overall design of the Swarm language. S-Nice to have The bug fix or feature would be nice but doesn't currently have much negative impact. Z-Feature A new feature to be added to the game. Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants