Bug JavaScript bugs

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
Tour Guide, available from https://github.com/cdrock/TourGuide/branches/Release installs both relay_TourGuide.ash and relay_TourGuide.js in /relay.

This results in two entries labelled TourGuide in the relay browser run script dropdown. While the author could have made different choices this raises the question of how to handle similarly named scripts? Since an ash script and a js script are two different things, I think there is a feature request to distinguish between the two types in menus.

And, when I run relay_TourGuide.jsI I get
JavaScript evaluator exception: syntax error (file:/D:/Dropbox/dist/relay/relay_TourGuide.js#555)
and at this point I cannot tell whether that is a scripting error or an implementation error so am reporting it here.
Now that we support JS, relay_ should be reserved for scripts that are to be executed by the mafia interpreter not the browser. The feature request should be to rename relay_TourGuide.js to TourGuidIne.js or similar
 

fronobulax

Developer
Staff member
Now that we support JS, relay_ should be reserved for scripts that are to be executed by the mafia interpreter not the browser. The feature request should be to rename relay_TourGuide.js to TourGuidIne.js or similar

I'm not quite following you.
 

lostcalpolydude

Developer
Staff member
Now that we support JS, relay_ should be reserved for scripts that are to be executed by the mafia interpreter not the browser. The feature request should be to rename relay_TourGuide.js to TourGuidIne.js or similar
I'm pretty sure the main purpose of pretending relay_ is to make it show in the "run script" dropdown, specifically to be able to run it in the browser. Or at least that has been the purpose for the last 10+ years.

The actual report there seems to be that this dropdown strips the file extension, which is relevant now that there are two possible file extensions and duplicates are possible (thanks to 20523).
 

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
I'm pretty sure the main purpose of pretending relay_ is to make it show in the "run script" dropdown, specifically to be able to run it in the browser. Or at least that has been the purpose for the last 10+ years.
No this isn't correct. relay_ scripts are executed by the mafia interpreter when selected from the "run script" dropdown and any output from write or similiar is rendered in the browser.

I am certain that relay_TourGuide.js was at no point intended to be shown in the dropdown, so the immediate solution is for the developers to update their package for compatiblity with the current revision of mafia.

In the future we could have an issue where two scripts genuinely want to show up in the "run script" dropdown and have the same filename and a different extension. I agree we should find some way to disambiguate that but its definitely not something that is likely to ever happen except by mistake!
 

philmasterplus

Active member
Looks like @fredg1 (most active contributor of the TourGuide fork) has already renamed relay_TourGuide.js. They just haven't released it yet.

Fred, if you see this, may we ask for a mini-release that contains the rename? I for one keep getting confused which "TourGuide" is the correct one in the relay scripts menu.

Edit: The latest release has fixed it. Thank you!
 
Last edited:

fronobulax

Developer
Staff member
Whether relay_TourGuide.js was intended to show up in the relay script menu, or not, it did and that points out that KoLmafia only displays a portion of the pathname in menus. An easy solution would be to include the extension in the menu entry. A less cluttered solution might be to break the menu into two sections - ash and js - and use menu position to disambiguate for the user. Whatever we decide for the relay script menu is likely to be considered for the ScriptMRU list as well.
 

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
Whether relay_TourGuide.js was intended to show up in the relay script menu, or not, it did and that points out that KoLmafia only displays a portion of the pathname in menus. An easy solution would be to include the extension in the menu entry. A less cluttered solution might be to break the menu into two sections - ash and js - and use menu position to disambiguate for the user. Whatever we decide for the relay script menu is likely to be considered for the ScriptMRU list as well.
This isn't an actual problem, as I explained here. If anyone has the time and the inclination (i.e. not me right now) the best solution would be to append the extension to (and only to) any clashing select options.
In the future we could have an issue where two scripts genuinely want to show up in the "run script" dropdown and have the same filename and a different extension. I agree we should find some way to disambiguate that but its definitely not something that is likely to ever happen except by mistake!
 

xKiv

Active member
This isn't an actual problem, as I explained here. If anyone has the time and the inclination (i.e. not me right now) the best solution would be to append the extension to (and only to) any clashing select options.
I thought that you can have multiple copies of relay_X.ash *and* relay_X.js throughout the relay/*/ hierarchy and the dropdown would display all of them (without paths), but only the relay/ directory itself seems to be scanned for that purpose. This means that you are right for the browser dropdown - showing the extension for clashing options would help resolve clashes.
It is also enough for the script menu in mafia, because that has submenus for subdirectories.

I can't tell if MRU stores scripts with extensions.
 

Phillammon

New member
When a function is exported as main in a js script, and has parameters specified that are not supplied as arguments, Mafia barrels through with them set to undefined. In ASH, mafia prompts the user for input, which is the expected behaviour.
 

philmasterplus

Active member
What you're looking for is probably difficult to achieve. Between default parameters, the arguments object, and rest parameters (...args), it's practically impossible to guess the answer to "how many arguments does this function need?"

If you want to accept multiple arguments, you could try using a low-level CLI argument parser that works with Rhino (e.g. minimist).
 

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
When a function is exported as main in a js script, and has parameters specified that are not supplied as arguments, Mafia barrels through with them set to undefined. In ASH, mafia prompts the user for input, which is the expected behaviour.
I would say this is working as intended. User prompting is an ASH feature.
 

philmasterplus

Active member
To be clear, this is not the multiple args case but the any args at all case

Unfortunately, the arguments object is still a thing.

JavaScript:
function main() {
  print("I got " + arguments.length + " arg(s)");
}

In this case, should KoLmafia ask the user for the argument(s)? If so, it would have to statically analyze the code for uses of "arguments"--which is probably brittle.
 

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
The solution here is to use user_confirm and probably for us to add a bunch of other prompt types to the stdlib
 

Phillammon

New member
I'm seeing threads on that from 2007, but the recommendation was "Just use arguments to main", hence this being my request. If we can get user input for ash and js generically that'd be wonderful.
 

fronobulax

Developer
Staff member
I am unlikely to be an author of JS for KoLmafia, but this feels like an unexpected consequence of the integration and similar to several other issues that were addressed by changing code in the integration layer, on the ASH side or both.

I would expect a parser to be able to detect how many arguments the author of a main expected to handle. I would expect execution to be able to determine how many arguments were provided. In a world where ASH is the model I would expect a difference between provided and expected to be reported to the user and I'd even be willing to have it declared as an error the stops execution.

So it sounds like a reasonable request. I can certainly accept "not going to do it" as a response but if there are technical reasons I'm not sure I understand enough to understand them. Perhaps declaring "main" means different things in JS land than ASH?
 

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
I am unlikely to be an author of JS for KoLmafia, but this feels like an unexpected consequence of the integration and similar to several other issues that were addressed by changing code in the integration layer, on the ASH side or both.

I would expect a parser to be able to detect how many arguments the author of a main expected to handle. I would expect execution to be able to determine how many arguments were provided. In a world where ASH is the model I would expect a difference between provided and expected to be reported to the user and I'd even be willing to have it declared as an error the stops execution.

So it sounds like a reasonable request. I can certainly accept "not going to do it" as a response but if there are technical reasons I'm not sure I understand enough to understand them. Perhaps declaring "main" means different things in JS land than ASH?
I think these expectations come from not fully understanding how JS works as a language (or other similar languages).

I've added `user_prompt` which should address the issue for now
 

fronobulax

Developer
Staff member
I think these expectations come from not fully understanding how JS works as a language (or other similar languages).

I've added `user_prompt` which should address the issue for now

Could be, but I also believe that if enough time and money were thrown at the problem, it could be addressed by KoLmafia parsing the script file before the JavaScript interpreter ever got a hold of it. So asking the question is one way to address the knowledge gap.
 

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
Could be, but I also believe that if enough time and money were thrown at the problem, it could be addressed by KoLmafia parsing the script file before the JavaScript interpreter ever got a hold of it. So asking the question is one way to address the knowledge gap.
There's a very big team at Microsoft working on this (static analysis of JavaScript) so I recommend you go to them with your findings!
 

fronobulax

Developer
Staff member
There's a very big team at Microsoft working on this (static analysis of JavaScript) so I recommend you go to them with your findings!

If I were in a mood I would say there is an unwarranted snarkiness to your response.

The problem I am wondering about is a file that defines main with a fixed number or arguments and an author who expects the first thing to be "executed" by an interpreter is it calls main with arguments provided by a source external to the file. I accept that in general static analysis of JS is difficult. But in the context of KoLmafia, my speculation is focused on things that could happen prior to KoLmafia passing a file to a JS interpreter. Perhaps a preprocessor that detected an exported main and inserted code?

Maybe preprocessors aren't in any practical toolbox anymore?
 
Top