Skip to content

Refactor and improve browser versions (tic-html, export stubs, webapp) #1912

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 5, 2022
Merged

Refactor and improve browser versions (tic-html, export stubs, webapp) #1912

merged 1 commit into from
May 5, 2022

Conversation

tranxuanthang
Copy link
Contributor

@tranxuanthang tranxuanthang commented May 4, 2022 •
edited
Loading

This PR refactor & fix various problems in browser/HTML versions:

  • Build emscripten script as ES6 module, so that emscripten doesn't pollute the global namespace and can be easier to import to SPA projects (vue, react,...)
  • Change SDL_EMSCRIPTEN_KEYBOARD_ELEMENT to the canvas element so that emscripten won't block all keyboard input in the rest of the webpage (fixing the problem that make typing or backspace-ing comment content on tic80.com not possible after starting the game)
  • Add tabindex to canvas element so that canvas can be focused (and unfocused) and capture the keyboard input properly
  • Refactor html template to be more minimal, and show a simple "play" icon (instead of "click to play" text)
  • Small changes to html build in github action's script

This is my first PR so forgive me if I missed anything.

@tranxuanthang
Refactor and improve HTML builds
f96ef1a 
This commit refactor & fix various problems in HTML builds:

- Build emscripten script as ES6 module, so that it doesn't pollute the
global namespace and can be easier to import to SPA projects
(vue, react,...).
- Change `SDL_EMSCRIPTEN_KEYBOARD_ELEMENT` to the canvas element so that
emscripten won't block all keyboard input to the rest of the
webpage (fixing the problem that make typing comment content on
tic80.com is not possible after starting the game)
- Add tabindex to canvas element so that canvas can be focused (and
unfocused) and capture the keyboard input properly.
- Refactor html template of html, html export stub and webapp
build to be more minimal, and add a simple "play" icon instead of
"click to play" text
- Small changes to html build directories in github action's script
@tranxuanthang tranxuanthang changed the title Refactor and improve HTML builds (tic-html, export stubs, webapp) Refactor and improve browser versions (tic-html, export stubs, webapp) May 4, 2022
@joshgoebel
Copy link
Collaborator

joshgoebel commented May 4, 2022 •
edited
Loading

What is the goal of:

-s EXTRA_EXPORTED_RUNTIME_METHODS=['ENV']

Also, we should state clearly this is going to break older browsers that don't support ES6 modules.

How far bad do we want our browser support to extend? Do we have any stats from the website showing us the browser distribution (to perhaps use as a rough metric instead of just spitballing). I'm not super worried about this, but it's worth mentioning since if it was more than a few users I'm personally not sure the benefits listed here are worth breaking compatibility.

@joshgoebel
Copy link
Collaborator

Also if we're adding sub-directories now I'm not sure html and html-export fully clarify things (at all)... could we use more descriptive names that make it clear what we are building in the two different folders? (good naming is my pet peeve) :-)

@tranxuanthang
Copy link
Contributor Author

tranxuanthang commented May 5, 2022 •
edited
Loading

@joshgoebel I don't think there is any serious issues in compatibility, ES6 modules feature (import/export) is supported by most modern browsers, and has roughly same browser support to webassembly, so if browser does not support ES modules then it probably does not support webassembly anyway.

These are browser compatibility information of ES6 modules amd webassembly:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules#browser_support
https://developer.mozilla.org/en-US/docs/WebAssembly

And also it is possible to support even older browsers with javascript build tools and bundlers (webpack, esbuild,...) when necessary.

@tranxuanthang
Copy link
Contributor Author

tranxuanthang commented May 5, 2022 •
edited
Loading

The argument -s EXTRA_EXPORTED_RUNTIME_METHODS=['ENV'] is used so that we can dynamically define custom environment variables in preRun:
https://github.com/nesbox/TIC-80/pull/1912/files#diff-f1f3a943063f166d85dab81f0e9a1a1054fbb930f44c996018497c39534ed583R79-R83

It is not needed without -s MODULARIZE=1, because this way ENV is defined as a global variable and editable anywhere (https://emscripten.org/docs/porting/connecting_cpp_and_javascript/Interacting-with-code.html#environment-variables). But with -s MODULARIZE=1, ENV is not in global namespace anymore as everything is wrapped as a module, so we have to add -s EXTRA_EXPORTED_RUNTIME_METHODS=['ENV'] so that ENV is created as a property inside Module object and editable in preRun (emscripten-core/emscripten#9827).

@tranxuanthang
Copy link
Contributor Author

Also if we're adding sub-directories now I'm not sure html and html-export fully clarify things (at all)... could we use more descriptive names that make it clear what we are building in the two different folders? (good naming is my pet peeve) :-)

@joshgoebel I'm super bad at naming things, do you have any suggestions? ;-)

nesbox
nesbox approved these changes May 5, 2022
View reviewed changes
Copy link
Owner

@nesbox nesbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a web developer and I don't understand some things, but anyway let's merge it.
Thank you for the help. 👍

@nesbox nesbox merged commit aa4313d into nesbox:main May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nesbox nesbox nesbox approved these changes

Assignees

@tranxuanthang tranxuanthang

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tranxuanthang @joshgoebel @nesbox