engine icon indicating copy to clipboard operation
engine copied to clipboard

Examples Overhaul

Open kpal81xd opened this issue 1 year ago • 5 comments

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
  • [x] 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.

kpal81xd avatar Feb 16 '24 16:02 kpal81xd

Any reflection on the trade-offs on examples/examples.config.mjs? Now all the files for every example are in one huge file and not per-example any longer - at least I feel uneasy about that. Having one file for each example allows for easier organization and modification of individual examples and one-big-file-for-all-examples feels like making it more:

  • difficult to maintain
  • switching file contexts for doing same-example work
  • git conflicts even tho people work on completely different examples
  • extra step that requires more mental overhead than just attaching files into the old static Example class

Turning the iframe functions into OOP feels a bit over-engineered, many changes like:

- setTimeout(iframeResize, 50);
+ setTimeout(() => iframe.fire('resize'), 50);

kungfooman avatar Feb 17 '24 12:02 kungfooman

Any reflection on the trade-offs on examples/examples.config.mjs? Now all the files for every example are in one huge file and

  • So actually I was thinking of converting that to put a config.mjs file in each of the example folders now along with controls.mjs and example.mjs so it would not all have to be in a single file

  • As for the iframe it was just adding scope to forward any events from the window over to the iframe in a more abstract context to make it easier to add new ones if needed

UPDATE: I have completed the conversion and it is a lot cleaner now

kpal81xd avatar Feb 17 '24 13:02 kpal81xd

Thank you for the quick changes, you work so fast :100:

What's up with all the file name changes? We have four conventions by now:

  • loader.js for ExampleLoader - filename only somewhat related to class name
  • ministats.js for MiniStats - lowercase of class name
  • We use kebab-case.js in normal engine that you converted to PascalCase.js in examples
  • PascalCase.js that you converted to in some places

(I don't care too much, just wondering)

I build it locally and build process worked like a charm so far, just need some time to test around here and there to get a picture

kungfooman avatar Feb 17 '24 15:02 kungfooman

I tested on my OnePlus 11 (Chrome/Android) and portrait mode has some problems:

Screenshot from PC (just scale devtools until examples window is going into portrait mode):

image

And on Android I have the opposite effect (being a bit too wide):

image

Not a big deal, just looks a bit strange:

image

Probably easier to fix in an extra PR, some examples also have so many options that I can't scroll to them...

otherwise - wow, I like the GLSL formatting, it seems to have some token issues here and there (mixed blue/white tokens):

image

Nothing that is a blocker here that I came across, good work :+1:

kungfooman avatar Feb 17 '24 16:02 kungfooman

Thank you for the quick changes, you work so fast 💯

What's up with all the file name changes? We have four conventions by now:

  • loader.js for ExampleLoader - filename only somewhat related to class name
  • ministats.js for MiniStats - lowercase of class name
  • We use kebab-case.js in normal engine that you converted to PascalCase.js in examples
  • PascalCase.js that you converted to in some places

(I don't care too much, just wondering)

I build it locally and build process worked like a charm so far, just need some time to test around here and there to get a picture

So all of it should be kebab case with the exception of React Components which I wanted to convert to PascalCase

kpal81xd avatar Feb 17 '24 18:02 kpal81xd

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

marklundin avatar Feb 19 '24 09:02 marklundin

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

kungfooman avatar Feb 19 '24 09:02 kungfooman

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.

mvaligursky avatar Feb 20 '24 14:02 mvaligursky