From 97cfb1985202d3bd8101b9d55b2ac6d51edfc00d Mon Sep 17 00:00:00 2001
From: Tony Garnock-Jones <tonyg@leastfixedpoint.com>
Date: Sat, 11 Dec 2021 14:04:31 +0100
Subject: [PATCH] Move stop continuation *before* a stopping facet's assertions
 are retracted.

This is a kind of ad-hocish response to an interesting problem. In
previous Syndicates, assertion changes were gathered into *patches*
which *netted out* intra-turn changes. In this implementation, each
change is relayed unchanged, so there is *no netting* going on. The
closest we get is a convention that when things are being replaced,
incoming assertions should be made before outgoing ones are retracted,
so that the kind of glitch that's seen is a double-up of records,
rather than a brief window where no records are present.

So here, by moving the stop continuation into an onStop for the
stopping facet (that nonetheless *executes* in that facet's parent's
context), we allow the following pattern to run without problem
glitching:

    react {
      assert Something(1);
      stop on asserted Condition() => react {
        assert Something(2);
      }
    }

Observers like this...

    during Something(_) => { // **A**
      ...
      during Something($specific) => { // **B**
      }
    }

will see

    + Something(1)
    + Something(2)
    - Something(1)

rather than what they saw before this change,

    + Something(1)
    - Something(1)
    + Something(2)

The consequence is that the line marked **A** above will *remain
active* after this change, with the facet at **B** being replaced,
rather than briefly tearing down the **A** facet when `- Something(1)`
and then creating a fresh one when `+ Something(2)`.

This came up in flappy bird, where I had

    react {
        at mainDs {
            assert ui.html('#board-area', template`<h1 class="score">${score.value}</h1>`);
        }
        at gameDs {
            stop on asserted GameOver() => react {
                at mainDs {
                    assert ui.html(
                        '#board-area',
                        template`<h1 class="score">${score.value}<br/>GAME OVER</h1>`);
                }
            }
        }
    }

which, in combination with the particular implementation of index.ts
in the html package, caused the UIFragment responder to get confused
and to not show the GAME OVER message - the previous score message was
removed, but the new one wasn't manifested in the DOM. Changing the
last `ui.html` above to `ui.context('foo').html` caused the problem to
go away (by making it effectively two unrelated fragments, rather than
a replacement of the content of a single fragment).
---
 packages/core/src/runtime/actor.ts | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/packages/core/src/runtime/actor.ts b/packages/core/src/runtime/actor.ts
index e234370..917fdfc 100644
--- a/packages/core/src/runtime/actor.ts
+++ b/packages/core/src/runtime/actor.ts
@@ -303,12 +303,15 @@ export class Turn {
     }
 
     stop(facet: Facet = this.activeFacet, continuation?: LocalAction) {
-        if (facet.parent === null) {
+        const facetParent = facet.parent;
+        if (facetParent === null) {
             this.stopActor();
         } else {
-            this.enqueue(facet.parent, () => {
+            this.enqueue(facet, () => {
+                if (continuation) {
+                    facet.onStop(() => Turn.active._inFacet(facetParent, continuation));
+                }
                 facet._terminate(true);
-                if (continuation) continuation();
             });
         }
     }