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

Examples Overhaul #6060

Merged
merged 143 commits into from
Feb 21, 2024
Merged

Examples Overhaul #6060

merged 143 commits into from
Feb 21, 2024

Conversation

kpal81xd
Copy link
Contributor

@kpal81xd kpal81xd commented Feb 16, 2024

Refers #6030

This PR aims to align the examples experience to of using the engine in a standalone app.

Changes

  • Removed example function and arguments and put body into example script global scope (app is now accessed by using export { app }; at the bottom of the script)
  • Moved arguments from example into modules to load in for each script
  • Acquiring the canvas element is handled by the example rather than being passed in as an argument
  • Converted multiple paths (assetPath, glslPath etc) to a rootPath which serves all static files
  • Added support for imports of ES modules
  • Added syntax highlighting for shaders

Demo

This PR has a lot of changes it so I have provided a demo serving these changes.
https://kpal81xd.github.io/playcanvas-examples

Preview

image

ToDo

  • Convert observer to use module
  • Move config for each example to its own separate config.mjs from examples.config.mjs
  • Fix broken thumbnail generation

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

@@ -10,7 +10,7 @@
"build:metadata": "node ./scripts/metadata.mjs",
"build:sharing": "node ./scripts/sharing-html.mjs",
"build:standalone": "node ./scripts/standalone-html.mjs",
"build:thumbnails": "npm run build && node ./scripts/thumbnails.mjs",
"build:thumbnails": "node ./scripts/thumbnails.mjs",
Copy link
Collaborator

@kungfooman kungfooman Feb 18, 2024

Choose a reason for hiding this comment

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

We discussed this a bit here: #5555 (comment)

Basically it allows for a cold start - but I also agree that it takes quite some time before building thumbnails. @mvaligursky Do you have an opinion on this?

IMO in the best case, we could build the thumbnails while not wasting any time running rollup/node processes beforehand - it's technically doable, that's why I used index.js in the example dir which linked every example. By virtue of ESM you could get a list of every example viaexport * from ... the index.js files - acting as single-source-of-truth instead of possibly outdated generated metadata files. The importmap would resolve everything in node_modules and the only thing eventually required is a npm install. I still like that idea more than making us indebted to all kinds of build scripts.

(rollup still has its place of course for generating minified builds for publishing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well currently this would only be required when modifying or creating new examples now since thumbnails are included in the repo itself now.

Yea having the thumbnails resolved using the imports I don't like since it splits how the example names are generated from folder and file names to the names specified in the import index.js files. One workaround would be setting file path in the config to read after the import is resolved but that seems messy.

Additionally you don't need to add example to some index file when generating a new one you can simply add the file within the specified folder.

@marklundin
Copy link
Member

What is the build process for this in relation to the engine? Does it still rely on building the engine first, or can we just import directly?

npm install && npm build
cd ./examples
npm install && npm build

@kungfooman
Copy link
Collaborator

What is the build process for this in relation to the engine? Does it still rely on building the engine first, or can we just import directly?

npm install && npm build
cd ./examples
npm install && npm build

Yea, it still relies on playcanvas.d.ts from engine, so you still need the double build

@kpal81xd kpal81xd requested a review from mvaligursky February 19, 2024 12:31
@mvaligursky
Copy link
Contributor

mvaligursky commented Feb 20, 2024

I've noticed that if I reload the example (even without making a change to it), I get this:

Screenshot 2024-02-20 at 14 28 32

This also happens if I just change the active device.

Copy link
Contributor

@mvaligursky mvaligursky left a comment

Choose a reason for hiding this comment

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

Looks and works great!

@kpal81xd kpal81xd merged commit ea502fc into main Feb 21, 2024
7 checks passed
@kpal81xd kpal81xd deleted the examples-rework branch February 21, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants