engine icon indicating copy to clipboard operation
engine copied to clipboard

RuntimeTypeInspector.js: Add RTI build target + Examples integration

Open kungfooman opened this issue 2 years ago • 12 comments

Fixes #2529

I'm excited to introduce RuntimeTypeInspector.js, which I believe will benefit PlayCanvas as a whole and our debugging workflows in specific. RTI is designed to type-check the arguments of every function and method. It detects type errors while the engine is running in a way that goes beyond the capabilities of TypeScript, as demonstrated in this video (there is another one on the website debugging the Microsoft Holometric Video PC project).

During the development of RTI it already helped me to open issues or make PR's for various issues (e.g. NaN bugs).

I believe that RTI will help improve productivity and efficiency for both engine developers and users, by catching errors early on and reporting them in detail.

Runtime Type Inspector is implemented using Babel AST traversal with custom JSDoc parsing (powered by TypeScript itself for highest compatibility), customized to PlayCanvas requirements. That means for example supporting polymorphing this types.

TLDR: It adds type assertions for runtime type checking, for example:

image

Being able to set breakpoints exactly where the errors are happening for the first time helps to track down problems that are otherwise difficult to pinpoint (and saves a lot of time :sweat_smile:).

Try for yourself: http://pc.runtimetypeinspector.org/

Just make sure to pick the right JS context - the exampleIframe:

image

I'm working on this for quite some time by now, a mix of fun and frustration - rewriting code on the AST level first seemed tricky, but given enough time, it was a rather straightforward way to automate type validations. I'm excited to read feedback and suggestions. :pray:

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

kungfooman avatar Nov 10 '23 17:11 kungfooman

This is pretty cool! A couple of questions:

  1. Is there a way to suppress reporting more than N errors if they are originating from the same place? Spam of errors on every application update - is not much of use.
  2. Is there a way to improve the stack within console log reporting so it points to the right place of the error? This will allow easier debugging with Dev Tools, but also provide a deep linking from Launcher to IDE.

Maksims avatar Nov 11 '23 15:11 Maksims

Hey @Maksims, thank you for the feedback!

  1. Is there a way to suppress reporting more than N errors if they are originating from the same place? Spam of errors on every application update - is not much of use.

Yep, for this there is this button:

image

Currently it supports a "warn once" or "spam all the time" mode, but do you mean even more fine-grained control with N + source?

(I just uploaded to latest HEAD + fixed a issue with the spam-button-state, now it saves/restores properly from localStorage)

2. Is there a way to improve the stack within console log reporting so it points to the right place of the error? This will allow easier debugging with Dev Tools, but also provide a deep linking from Launcher to IDE.

It's a console.error and if you click on it, the stack shows up:

image

Then you can click on the file location under assertType and you are at the "error source" and can just put a breakpoint:

image

So currently it takes two clicks to get to the place of the type error, but I'm not sure what you mean with deep linking from Launcher to IDE, could you elaborate a bit on that?

Thank you again!

kungfooman avatar Nov 11 '23 17:11 kungfooman

Yep, for this there is this button:

It did not work, unfortunately. I tried it, made no difference. Still, had errors spammed: image

It's a console.error and if you click on it, the stack shows up:

Ah, I did open dev tools after the errors were reported, it does not contain stack in such place, and all errors "originated" from the same place. Is there a way to preserve their original stack? E.g. if the exceptions happen in the user script, but it reports first playcanvas.rti.js:504, then it is less clear where the error happened. This is actually an issue already with the current engine, where some errors like providing wrong arguments, are reported from within the engine (that's where they happen), but it is not straight away apparent to the user that it was due to passing wrong arguments. Especially when these arguments are not directly applied, but this is slightly unrelated. It just highlights an issue: if an error is reported not from the user script, then they first think it is not their doing. So if an error happened somewhere within the user script or engine, the stack should specify errors from there, not from one single place.

Editor can try their best to filter through stack lines, and figure out if it can provide a deep link to a specific script in IDE, as it is really useful.

Maksims avatar Nov 11 '23 17:11 Maksims

It did not work, unfortunately. I tried it, made no difference. Still, had errors spammed:

Hm, maybe the intention of the button isn't clear, when "Spam type reports" is checked it means that every type error is calling console.error (and if it is unchecked, the type errors are only logged once). If that doesn't work, it is probably because I updated the file and then it needs a Ctrl+R with DevTools open to clear the cache :thinking:

Thank you for pointing out the stack trace issue when DevTools isn't open from the beginning - I wasn't aware of that, so I will save the stack traces aswell for better inspection (would be nice to see in UI too).

Editor can try their best to filter through stack lines, and figure out if it can provide a deep link to a specific script in IDE, as it is really useful.

Right, I made a little experiment what kind of information we can get out of the stack trace for Editor integration:

const info = () => {
  try {
    throw new Error(); // Dummy exception to retrieve stack trace
  } catch(e) {
    const {stack} = e;
    if (stack.toLowerCase().includes('script')) {
      const lines = stack.split('\n');
      const i = lines.findIndex(_ => _.includes('ScriptComponent'));
      let scriptLine = lines[i - 1];
      scriptLine = scriptLine.replace(/at/, '').trim().split(' ')[0];
      console.warn('Type error in script: ', scriptLine);
    }
  }
}
info(), false; // false so it doesn't breakpoint here

This is "breakpoint code" we can enter here:

image

(clicking on the function source)

image

Then you have integrated the "breakpoint code" into the code flow and now you can play around with stack traces:

image

Thinking about the editor, we could just add "quick links" to the actual scripts now. Thank you for the suggestion!

Edit: I think I finally understood it: Instead of "spam" and "once", it should have an option "never", so it doesn't spam anything in the console (I will work on that).

Edit 2: Added the type error reporting mode never now:

image

(this keeps DevTools completely "clean", only reporting in the UI)

kungfooman avatar Nov 12 '23 17:11 kungfooman

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
engine ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 18, 2024 10:44am

vercel[bot] avatar May 15 '24 10:05 vercel[bot]

Im unsure about directly including run time inspector specifics in the engine at best it can support loading a custom engine build but no custom code

kpal81xd avatar May 24 '24 15:05 kpal81xd

Hey @kpal81xd, can you elaborate more on what you mean by adding RTI specifics into the engine? Only a RTI build contains PC specific extra validations (e.g. checking that a Mat4 contains no NaN values)

Thank you for checking this out already!

kungfooman avatar May 24 '24 15:05 kungfooman

Its more just creating a custom build target from your rti package - Yes is it useful for debugging etc but since its purely for debugging purposes we do not want to bloat the engine. The user should be able to build the engine themselves from source if they wish to add custom packages but I do not believe we should officially support it

kpal81xd avatar May 28 '24 08:05 kpal81xd

I'm finally coming to a resolution here, it just feels so spammy for many examples still, even tho they run fine (simply based on wrong types in the engine). My main goal is "0 errors" - so developers can start editing examples and only see type errors they introduced themselves during their coding session.

If anyone has time, I would love to have these issues fixed:

  • http://pc.runtimetypeinspector.org/#/graphics/paint-mesh - scopeId seems to be a left over and has to be removed?
  • http://pc.runtimetypeinspector.org/#/user-interface/button-sprite - we are setting length of number arrays and move them around and calculating all kinds of stuff with them (leading to vectors filled with undefined) - useless calculations, so there is potential for a speedup aswell via small refactor
  • http://pc.runtimetypeinspector.org/#/graphics/lights-baked-a-o - filling of matrices with 'NaN' values
  • significantly reduce error counts, especially the ones that occur every frame

Any volunteers? :sweat_smile:

kungfooman avatar Jun 21 '24 11:06 kungfooman

A good cause 👍 I might tackle some of those next week.

LeXXik avatar Jun 21 '24 11:06 LeXXik