leopard icon indicating copy to clipboard operation
leopard copied to clipboard

Broadcast behavior doesn't match Scratch

Open PullJosh opened this issue 3 years ago • 7 comments

This bottle flip project loops infinitely on Scratch using broadcasts, but when translated to Leopard it stops after one time through the loop.

I am guessing that this is a bug related to broadcasts working differently in Leopard vs. Scratch.

PullJosh avatar Aug 19 '21 13:08 PullJosh

Looks like the problem is an issue when an event calls itself.

Example project illustrating the behavior: https://scratch.mit.edu/projects/817558449/

Code for the left sprite: (does NOT work when translated to LeopardJS) Screen Shot 2023-03-11 at 1 57 01 AM

Code for the right sprite: (which DOES work when translated to LeopardJS) Screen Shot 2023-03-11 at 1 57 21 AM

I have not yet tracked down why this occurs -- currently just happy to have narrowed down a simple reproducible test case.

HanClinto avatar Mar 11 '23 06:03 HanClinto

Looks like the cause is related to this section in Project.js:191, where if the trigger is already running then it appears that it... fails silently?

  _startTriggers(triggers) {
    // Only add these triggers to this.runningTriggers if they're not already there.
    // TODO: if the triggers are already running, they'll be restarted but their execution order is unchanged.
    // Does that match Scratch's behavior?
    for (const trigger of triggers) {
      if (
        !this.runningTriggers.find(
          runningTrigger =>
            trigger.trigger === runningTrigger.trigger &&
            trigger.target === runningTrigger.target
        )
      ) {
        this.runningTriggers.push(trigger);
      }
    }
    return Promise.all(
      triggers.map(({ trigger, target }) => {
        return trigger.start(target);
      })
    );
  }

I need to learn more about Javascript promises and the way things are being handled here, but is there any consequence to just pushing another instance of the old trigger onto the stack while the current one is running? Or are these triggers singletons or not re-entrant-safe, or something else that would keep us from doing that?

HanClinto avatar Mar 11 '23 07:03 HanClinto

Once my big TypeScript PR is merged, I'd like to refactor trigger handling and sequencing (#163). I'll try to make sure this project works.

adroitwhiz avatar Mar 11 '23 09:03 adroitwhiz

@adroitwhiz Sounds very good!

This was bugging me in the back of my mind this morning, so I made another test project in Scratch to experiment with running multiple "threads" at the same time. The current Scratch 3 behavior is really odd to me, and I can't explain how it works. I can't explain why this project behaves the way that it does (even just running in Scratch -- not even exporting to Leopard).

https://scratch.mit.edu/projects/817735334/

I would expect that all three sprites would match the behavior of the sprite on the left -- they would go "tick, tick, tick, pause" -- but instead I get the behavior that only the left sprite does that, and the other sprites go "tick, tick, tick, pause, tick, pause, tick, pause, tick, pause". This doesn't make sense to me.

HanClinto avatar Mar 11 '23 19:03 HanClinto

@HanClinto You're running into a peculiarity where Scratch distinctly does not support running two threads of the same script at once (within a given target, i.e. one sprite/clone). The "broadcast" block is really short for "send an event to start doing this, or start over if it was already in progress".

Sprite2 behavior:

  • set thread count to 0
  • Broadcast sprite2_tick:
    • no running sprite2_tick thread, nothing to cancel
    • change thread count by 1 (= 1), turn 15 degrees, wait 2 seconds...
  • 0.25s later: broadcast sprite2_tick ->
    • cancel currently evaluating sprite2_tick thread (silently)
    • change thread count by 1 (= 2), turn 15 degrees, wait 2 seconds...
  • 0.25s later: broadcast sprite2_tick ->
    • cancel currently evaluating sprite2_tick thread (silently)
    • change thread count by 1 (= 3), turn 15 degrees, wait 2 seconds...
    • 2s later: broadcast sprite2_tick, change thread count by -1 (= 2)
      • no running sprite2_tick thread, nothing to cancel
      • change thread count by 1 (= 3), turn 15 degres, wait 2 seconds...
      • 2s later: ad infinitum

Note that even though "change Thread Count by -1" is after broadcast sprite2_tick (at the end of "when I receive sprite2_tick"), it still gets evaluated because the thread doesn't get canceled until the start of the next frame. If you put a "wait 0 seconds" (equivalent to Leopard yield)between "broadcast sprite2_tick" and "change thread count by -1", Scratch never reaches the "change thread count by -1" block and it will start counting up.

I believe Sprite3 behavior is functionally identical to Sprite2 (with maybe a 1/30th sec desync building up each time if the project had a running visual-update loop). The only difference is sending the tick message again after 3.5 seconds. This cancels the running tick thread (at that point in the middle of a "wait 2 seconds"), and since that broadcast begins while Sprite2 (on the same time cycle) is also partway through its wait, the new thread immediately starts and stays on a 2 second cycle that is offset from Sprite2.

A little confusion may come from your thread count variables! After all, they aren't inherently measuring the number of real threads running - they're just tracking "how many times did this broadcast begin execution, minus how many times did this broadcast complete execution?". A simple way to demonstrate is by creating a "don't broadcast again" variable, and wrapping broadcast sprite2_tick (etc) in "if don't broadcast again = 0", but not wrapping "change Thread Count by -1". Once you set "don't broadcast again" and the waiting 2 seconds passes, you'll note it doesn't repeat again... but the thread count is only decremented once, showing that only one thread was still running at that point!

towerofnix avatar Mar 11 '23 19:03 towerofnix

@towerofnix Aaaah, that makes PERFECT sense -- thank you for the explanation! I was definitely mentally stuck on that one, but you explained it perfectly.

So in this case, this is where our implementation totally falls down, because we don't restart an already-running trigger -- if it's already running, we ignore it.

HanClinto avatar Mar 11 '23 19:03 HanClinto

Yep, exactly! Glad my explainer helped. Hopefully it's comprehensive enough to assist with figuring out an implementation on Leopard's end too, during the #163 refactor 📦

towerofnix avatar Mar 11 '23 21:03 towerofnix