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

engine: missing dep on / reference to pako library #53

Open
pkgw opened this issue Jul 15, 2020 · 4 comments
Open

engine: missing dep on / reference to pako library #53

pkgw opened this issue Jul 15, 2020 · 4 comments

Comments

@pkgw
Copy link
Contributor

pkgw commented Jul 15, 2020

engine/wwtlib/Layers/Layer.cs includes a literal reference to the pako decompression library. We don't include that code right now, so the engine crashes in certain usages.

Triggered by https://bit.ly/wwt-neowise2020 , which loads http://data1.wwtassets.org/packages/2020/07_neowise/Comet_NEOWISE_July_2020.wtt .

@Carifio24
Copy link
Member

I've looked into this a bit (since it should be relevant for tour export in glue). We had talked before about doing something like const pako = import('pako') to grab the pako library. The one issue that makes that a bit thorny is that when import is used dynamically like that, it returns a promise rather than the library object itself.

That being said, we can use import if we replace this line with something like

  Script.Literal("if (typeof pako == 'undefined') { import('pako').then((pako) => {");
  result = (string)Script.Literal("pako.inflate(e.target.result, { to: 'string' })");
  Script.Literal("}); } else { ");
  result = (string)Script.Literal("pako.inflate(e.target.result, { to: 'string' })}");

Up to some formatting, that makes the output JavaScript in that block be

try {
    if (typeof pako == 'undefined') {
      import('pako').then((pako) => {;
        result = pako.inflate(e.target.result, { to: 'string' });
      });
    } else { ;
      result = pako.inflate(e.target.result, { to: 'string' })
    };
  }

Not the most elegant fix, but it would work.

Worth noting that in a CommonJS setting (e.g. Node), we could use require, which is synchronous.

@pkgw
Copy link
Contributor Author

pkgw commented Jan 27, 2022

Hmm, will the import() branch work in this scheme? The result variable will be assigned asynchronously in the callback, which will run at some arbitrary time, maybe/probably after the main function exits. Either the rest of the processing will also need to happen in the callback, or some kind of await will be necessary.

But I'm sure that there's a solution here. It would be nice to figure out something scalable too — some things might become easier if the engine itself is able to take advantage of external JavaScript libraries. I know that ScriptSharp has some kind of system for creating stub C# classes that have magic annotations that say how they should translate into JS: they're used for the low-level WebGL calls at a minimum. If we could get to the point where an external dep like pako can be correctly loaded and invoked as if it were native C#, that would give us a really useful tool for going forward.

@Carifio24
Copy link
Member

Ah, yeah, as written this will almost certainly not work. It is good to know that we can get ScriptSharp to generate promise-y code with the assignment to result still being in C# (which works since the extraneous semicolons are just ignored).

Generating stub classes to translate to JS sounds interesting; I'll look into that.

@pkgw
Copy link
Contributor Author

pkgw commented Jan 27, 2022

Cool. Stubs classes won't help with the core issue of async imports here, but again, I'm sure there's a way to solve that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants