Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FES message is broken if reserved words are used. #7168

Open
shibomb opened this issue Aug 3, 2024 · 7 comments
Open

FES message is broken if reserved words are used. #7168

shibomb opened this issue Aug 3, 2024 · 7 comments
Labels

Comments

@shibomb
Copy link
Contributor

shibomb commented Aug 3, 2024

p5.js version

1.9.4, 1.10.0

What is your operating system?

None

Web browser and version

Any browser (set non-English (e.g. Spanish, Japanese) to the top in language priority settings)

Actual Behavior

Set the browser's language preference setting to Japanese,
When executing code using reserved words, an error appears in the console, as if the FES message could not be generated.

スクリーンショット 2024-08-04 2 00 18

Console output:

TypeError: Cannot read properties of undefined (reading 'replaceAll')
p5.js translator called before translations were loaded

Additional information 1:

The problem does not occur if the browser's language preference is set to English language.

Console output:

🌸 p5.js says: you have used a p5.js reserved function "value" make sure you change the function name to something else.
+ More info: https://p5js.org/reference/#/p5/value 

Additional information 2:

Similar source code displays fine in OpenProcessing and local PC.

Console output:

🌸 p5.jsが言っています: p5.jsの予約済み関数 "value" を使用しました。関数名を他の名前に変更してください。
+ 詳細情報: https://p5js.org/reference/#/p5/value

Expected Behavior

Correct FES message as shown below (e.g. Japanese)

🌸 p5.jsが言っています: p5.jsの予約済み関数 "value" を使用しました。関数名を他の名前に変更してください。
+ 詳細情報: https://p5js.org/reference/#/p5/value

If output of the translated version is not possible, a normal message in English should be displayed.

🌸 p5.js says: you have used a p5.js reserved function "value" make sure you change the function name to something else.
+ More info: https://p5js.org/reference/#/p5/value 

Steps to reproduce

Steps:

  1. Set the browser's language preference setting to non-English (e.g. Spanish, Japanese)
  2. add let value = 0 in setup function.
  3. Play

Snippet:

function setup() {
  createCanvas(400, 400);
  let value = 0;
}
@raclim
Copy link
Contributor

raclim commented Aug 12, 2024

Thanks so much for reporting this! I'm wondering if this might be related to/could be addressed through the p5.js library's FES? @Qianqianye

Copy link

welcome bot commented Aug 12, 2024

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

@Qianqianye Qianqianye transferred this issue from processing/p5.js-web-editor Aug 12, 2024
@Qianqianye
Copy link
Contributor

Thanks @shibomb and @raclim. I'm tagging @limzykenneth @davepagurek @sproutleaf, who are actively working on p5.js FES, to take a look at this.

@limzykenneth
Copy link
Member

limzykenneth commented Aug 13, 2024

Looking into this but the main thing is that the correct behavior should be that the message is not displayed as value is not a top level defined variable. When running the sketch locally I don't get the message at all (which is correct) (Scratch that, looking at the wrong thing.) so I'm not sure why the editor and openprocessing is detecting it as overwriting something.

@limzykenneth
Copy link
Member

I just realised that the global variable overwrite detection has made some incorrect assumptions that created quite a bit of false positive and false negatives (all classes' members are assumed to be global variables when it usually is not the case and some other global variables are exposed but not detected eg. sin()).

The plan for @sproutleaf's FES work this will be reworked for 2.0. In the meantime, I will do a fix for 1.x to get things mostly working (not sure if I can make it work 100%).

For the translation error on the editor, it probably is a different FES problem and I'll look into it as well.

@limzykenneth
Copy link
Member

limzykenneth commented Aug 13, 2024

For the translation one there is a race condition between the init window load event and the FES sketch reader window load event. The init load event is coupled with the translation loading and as a result will take longer to resolve but the FES sketch reader when translation is required, will be asking for the translation if it encounters an error, causing the race condition (init load including loading translation should finish first before the sketch reader runs).

@raclim For the p5.js web editor, monkey patching the p5._report function created the error messages because the first argument message will be undefined when the translation has not yet loaded.

For openprocessing, I'm not entirely sure why it is working but probably have to do with the way they patched the initialization of p5.js to work in a different way to fit how they display sketches.


Both this and the other one above are quite complex to fix as things are very tangled in this part of the core code. If anyone wants to have a go do have a look and I'm happy to answer any questions based on my findings. I'll try to find a good solution in the meantime.

@raclim
Copy link
Contributor

raclim commented Aug 22, 2024

Thanks @limzykenneth for diving into this! I'll also try to take a deeper look as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment