Feature - Implemented Combat filter functions in JavaScript

[Obeliks]

I started working with the JavaScript Runtime, and I couldn't figure out how to use adventure() with a custom combat filter function defined in the same script. The function is never found, and thus always returns void. (Shouldn't it throw an error if the requested function is not found? Ash seems to do that from looking at the code, but the wiki for adventure() says otherwise)

So I dug around the code a bit, and it seems that the JavascriptRuntime always searches the topLevelScope for exported functions. However, the functions exported by the main module are not merged into the global context, and are thus lost after the top-level execution.

I tried this change, and now I can use locally defined functions as combat filter functions.

Can anybody with some knowledge of the Javascript runtime give me a hint if this is the right way to go, or am I missing something?

Is there any good use case for looking up the function name in the top level scope (here)?

I also had a problem that run_combat always returns void, because the engine state is always EXIT (perhaps because of the filter function ). I tried to re-set the engine state during non-top-level executions, but without much success. Any ideas?

 

[gausie]

I think I'm going to "fix" this by special casing those functions in the JavascriptRuntime to be able to take functions as params

 

[Obeliks]

That sounds like an ideal Implementation from the frontend side.
But you still need to "remember" those functions for the invocation somewhere (or refactor quite some stuff to be able to pass functions from and to the runtime), right?

 

[gausie]

I will look into it, or you can if you like!

 

[gausie]

@Obeliks This is my proposed approach. I'm waiting on some feedback from @ikzann since he is the expert, but this means that we'll certainly merge something that works!

 

[Obeliks]

Ah, not bad, I didn't think about simply keeping the references in the global scope. Should the cleanup of temporaryName perhaps happen in a finally-block?

I will try the patch in an hour or two...

 

[gausie]

Should the cleanup of temporaryName perhaps happen in a finally-block?

If there's an exception, we'll yeet the scope anyway so I don't think there's any reason.

 

[Obeliks]

I finally got around testing these, and it doesn't seem to work for me.

I'm basically running this code:

export function creationCombatHandler(round, opp, text) {
  const macro = generateWeirdeauxMacro(text);
  printHtml(`creationCombatHandler returned: <pre>${macro}</pre>`);
  return `"${macro}"`;
}

export function main() {
  adv1(Mansion, -1, creationCombatHandler);
}

but I get this in the output log:

Macro override "org.mozilla.javascript.gen.file__C__Users_micro_KoLmafia_scripts_weirdeaux_weirdeaux_a878ef1a_js_16@3e7e47fb" returned void.

and then it calls the CCS, since it couldn't execute the combat filter function.

 

[Obeliks]

I just noticed that this got already merged, so I better test the upstream code

 

[Obeliks]

Works like a charm, of course. Sorry for the inconvenience

 

[gausie]

No worries! It's a cool feature that I'm using now too!