Inner build does not correctly require Drill

[xsebek]

From #373 by @byorgey:

So I ran into one additional issue while testing out some things. In creative mode I tried something like this:

def forever = \c. c ; forever c end
def unblock = try {drill forward} {} end
def push = unblock; move end

build {forever (push; build {turn right; forever push})}

...
However, it doesn't work. The initial robot works fine, drilling along and building another robot at each step. However, all those secondary robots don't end up with a drill device and crash with a fatal error when they try to execute the drill command.

The problem was present before, but not visible until the robots stopped without a drill.

In this new iron age, it is immediately a fatal bug once the robot attempts to drill.

Screenshots

A bronze age screenshot from the main branch:

^{It's almost the same now too, so I won't include another screenshot here.}

Log of the robot missing a drill:

Additional context

I traced the issue to the requiredCaps function which did not require CDrill.

[byorgey]

Oh, I like your use of log and robotNamed to be able to find and view the log for the inner robot. =D Thanks for the detailed work tracking this down!

[xsebek]

Btw. it is the same with if which also gets the previous behaviour because the drill will not run before meeting rocks:

def push = b <- blocked; if b {drill forward} {}; move end

[xsebek]

Surprisingly this program in Classic mode does not run into a Fatal error:

def forever = \c. c ; forever c end;
def unblock = try {drill forward} {} end;
def push = unblock; move end;
log "Hi, I am base";
r <- build {
  wait 2;
  log "Hi, I am builder";
  forever (build {log "Hi, I am pusher"; turn forward; forever push}; log "- robot built")
};
give r "logger";
give r "drill";

These are all the robots logs:

Hi, I am base
Hi, I am builder
- robot built
build: You do not have required devices, please obtain: 
  - logger
Hi, I am pusher
You do not have the devices required for:
  'drill'
  please install:
   - metal drill or drill

EDIT: But to be clear it is still wrong, the drill should have been installed.

[xsebek]

Once #400 is merged, you can test this with an included scenario:

$ cabal run test:swarm-integration -- -p '/build with drill (#394)/'
Up to date
Tests
  Test scenario solutions
    Regression tests
      build with drill (#394): FAIL (expected: Awaiting fix (#394)) (0.04s)
        test/integration/Main.hs:155:
        Fatal error: Drill is required but not installed?!
        Please report this as a bug at
        <https://github.com/swarm-game/swarm/issues/new>.
         (expected failure)

All 1 tests passed (0.04s)

[xsebek]

@byorgey is this #540?

[byorgey]

No, it's not #540. That one happens when you have a def and a build in the same expression, separated by a semicolon. You can tell that's not the problem since the push command does correctly lead to a drill being required for the outer robot, just not for the inner robots. However, it could be that somehow the correct requirements context with push in it is not being passed along to the requireCaps function for the inner build. I am still not sure.

[byorgey]

OK, so the issue seems to be that newly built robots were not inheriting the robotContext of their parent, so the inner robot did not know what the requirements were for push. Copying the robotContext over is easy enough, but first I want to understand why a lot of other things weren't already broken by this. Like, why were built robots still able to use definitions from the parent robot, if they were starting with an empty context?

EDIT: Ah, I understand why now:

• The contents of build is a delayed command which carries along with it the environment that was in effect when it was written. This actually makes sense: when I write build {foo; bar} the meaning of foo and bar is determined based on what is in scope at the instant that I write it, not later when it actually runs.
• The same goes for the store: it is carried along in the CESK machine, and the CESK machine of the child robot has its initial store copied from the current store of the parent robot. The Store in the RobotContext is kind of ignored for all but the base robot.
• The type context is also irrelevant, because the child robot won't need to do any typechecking.
• The requirements context is quite relevant, though, as we can see from this issue. However, it seems like that's kind of due to the hacky way that we handle requirements checking. Once we implement Handle requirements properly as part of the type system #231 I kind of wonder if we can get rid of the RobotContext entirely (that is, we'll just have a single top-level RobotContext but they won't be stored in robots any more).

Should have a fix soon.

[byorgey]

Hmm, this actually gives me an idea how to fix #540...