processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.45k stars 3.29k forks source link

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

Open shibomb opened 1 month ago

shibomb commented 1 month ago

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 commented 1 month ago

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

welcome[bot] commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 3 weeks ago

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