FIX: #421 `when` and `onEvent` now initialize at INIT time, not at COMPILE time - [merged] #697

Closed benloh closed 1 year ago

In GitLab by @daveseah on Jul 18, 2022, 13:47

Merges dev-sri/421-eventish-fix -> dev-next-gui

ELIMINATE PHANTOM EVENT HANDLERS

Issue #421 describes the case where event handlers are piling-up on recompile. This merge request addresses this by fixing the bugs in onEvent and when that modifies the simulation state at compile time instead of during an appropriate program phase; every time the GUI recompiles a blueprint, another copy of the handler was added to the simulation state.

These keywords now properly emit additional setup code into PROGRAM INIT automatically to do the registration at simulation start, not during compilation.

TESTING

[ ] Do scripts compile and run in MAIN as before?
[ ] Does recompiling a blueprint multiple times outside the simulator cause extra handlers to be generated?
[ ] Does moving when into PROGRAM EVENT still work? Does it work the same as in PROGRAM UPDATE? It should be in EVENT logically, but it will run in UPDATE as well
[ ] Does onEvent work both in PROGRAM EVENT and PROGRAM UPDATE? It should be in EVENT logically, but it will run in UPDATE as well

TECHNICAL CHANGES

The script-bundler now has AddToProgramOut( prog:TSMCProgram, progName:string ) that will write the provided program code to any recognized program name (e.g. INIT).
The "User Program Names" are now defined in dc-sim-data-util and used by dc-sim-data to determine valid symbols for program pragmas. This data structure also determines what's "enforced" in every script.
The Agent class now implements AgentEVENT() which will run the bundle.EVENT program, which executes immediately after all agents have run UPDATE. There is now a new SIM/AGENTS_EVENT phase in api-sim-gameloop and related phase execution calls in sim-agents. This was needed to support the existence of a when keyword inside of # PROGRAM EVENT. Another way of looking at this is that the EVENT phase works just like the UPDATE phase, but can now rely on a completely updated set of agents across the sim to be operated on.
The when keyword now emits an init script into PROGRAM INIT no matter which PROGRAM pragma it is in. This init script registers the tests when thee simulation engine is making agent instances; before it made them too early in the cycle during COMPILE.
The onEvent keyword now emits an init script into PROGRAM INIT no matter which PROGRAM pragma it's in. This init script registers the event when the simulation engine is making agent instances; before, it made them too early in the cycle during COMPILE so recompiling a script would create multiple event loops to be run. Note that onEvent does not create loop coode, so it can appear anywhere.

Additionally, there is some "secret smarts" enabled:

The when keyword automatically puts its loop code into PROGRAM EVENT no matter what its context is. This can be disabled via a switch named FORCE_INTO_EVENT defined in when.tsx. Technically when would also work during PROGRAM UPDATE but it doesn't belong there

BONUS: REMOVE EXTRANEOUS PROGRAMS

We can technically remove PROGRAM CONDITION because it's not used anymore. It has been removed from the required directives list.
We can technically remove PROGRAM EVENT because technically it's not needed, but we are leaving it in so it is a visible parallel to PROGRAM UPDATE.

Statements that go into PROGRAM EVENT:

when statements
onEvent statements

Statements that go into PROGRAM UPDATE:

every statements
any code that is not required to be in another program

NOTICE: Possible complications with `every`

This keyword generates idiosyncratic timers that make assumptions about running inside a frame tick with a period of 30fps, which isn't the case with all block-defining statements.

the every keyword can appear inside of a when statement, because EVENT contains the runtime event polling code and runs every frame like UPDATE.
However, inside a onEvent statement the code block is fired intermittently and its time is not calculated against system elapsed time, so timekeeping will be unreliable

In GitLab by @daveseah on Jul 18, 2022, 13:48

added 2 commits

07024c28 - 1 commit from branch dev-next-gui
abc2a8d3 - Merge branch 'dev-next-gui' into dev-sri/421-eventish-fix

Compare with previous version

In GitLab by @daveseah on Jul 18, 2022, 18:34

added 1 commit

0da948f7 - 421-eventish-fix: remove PROGRAM CONDITION from user program names list

Compare with previous version

In GitLab by @daveseah on Jul 18, 2022, 18:51

added 1 commit

3afb5955 - 421-eventish-fix: add PROGRAM EVENT support to Agent

Compare with previous version

In GitLab by @daveseah on Jul 18, 2022, 19:25

added 1 commit

d148f43a - 421-eventish-fix: when now ALWAYS writes into EVENT program, no matter what

Compare with previous version

I'm getting multiple complaints about the use of an unknown toUpperCase function:

class-symbol-interpreter.ts:601
script_bundler.ts:182
transpiler-v2.ts:150

It appears that the objects that toUpperCase is being called on are not necessarily strings, which causes js to choke.

We could just wrap the errant lines in a String() method, e.g.

      if (CHECK.IsValidBundleProgram(String(bundleOut).toUpperCase()))

But I want to make sure there aren't deeper issues at play here (e.g. bundleOut should be defined AND a string, and if isn't there's something else wrong up the chain).

Will test more tomorrow.

In GitLab by @daveseah on Jul 19, 2022, 24:47

added 2 commits

98b98881 - 421-eventish-fix: (cosmetic) fix bad fn names in script-bundler methods
04c37298 - 421-eventish-fix: fix crash in compiler due to error in GetUserProgramNames()

Compare with previous version

In GitLab by @daveseah on Jul 19, 2022, 24:49

@benloh ScriptEditor crash fixed; script-bundler GetBundleOutSymbols() was wrapping an array with Object.keys() when it should have returned it as-is. The keys of an array are the numeric indices.

Testing

`onEvent`

Add an onEvent Start [[ ]] block
Add a dbgOut 'one' line.
Run the sim
Console should show one line.
Add a dbgOut 'two' line.
Run the sim
You should see one followed by two only once:
```
one
two
```
If you see repeats, then the first compile of the program is still running, e.g.
```
one
one
two
```

`when`

Add this blueprint:

# BLUEPRINT Character12
# PROGRAM DEFINE
useFeature Costume
featCall Costume setCostume 'circle.json'
useFeature Physics
addFeature Touches
featCall agent.Touches monitor 'Character12' b2b
# PROGRAM INIT
# PROGRAM EVENT
when Character12 touches Character12 [[
dbgOut 'TOUCH EVENT'
]]
# PROGRAM UPDATE
when Character12 touches Character12 [[
featCall agent.Costume setGlow 1
]]
# PROGRAM CONDITION

Add two instances, set them next to each other so they touch
Run the sim
You should see both glow
You should see multiple "TOUCH EVENT" in console.
Comment out dbgOut 'TOUCH EVENT'
Change featCall agent.Costume setGlow 1 to dbgOut 'NEW'
Save to server
Run the sim
The two should no longer glow
The console output should only show "NEW" -- if the characters are still glowing or you see "TOUCH EVENT" then the program was not cleared.

Confirmed onEvent works. Confirmed when works inside PROGRAM EVENT and PROGRAM UPDATE.

One note: We still need to modify mx-sim-control to do a full reset and re-compilation of blueprints. Otherwise old programs continue to run. The tests above won't work without this fix.

I already have code in my other branch that does this. Pushing it here momentarily.

added 1 commit

2a34c03f - 421-eventtish-fix: SIm reset needs to clear EVERYTHING including script event...

Compare with previous version

@daveseah there's a demo when test called dies that I believe we should remove or comment out, if only so dies does not show up as a when test option.

It's in sim-conditions:25.

Wanted to make sure you weren't using it for something else before commenting out.

In GitLab by @daveseah on Jul 19, 2022, 11:25

re: simulator resets - yah, I didn't know where you were doing it figured you would take care of it!

re: mystery test - yah, go ahead and remove it!

added 1 commit

98dcb7cf - 421-eventish-fix: Remove non-functioning 'dies' when test example

Compare with previous version

mentioned in commit be1b630035875e5d5c75c988af660816e0cd37f4

theRAPTLab / gsgo