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

[RFC] Async/Lazy Component Systems #6986

Open
marklundin opened this issue Sep 24, 2024 · 15 comments
Open

[RFC] Async/Lazy Component Systems #6986

marklundin opened this issue Sep 24, 2024 · 15 comments
Labels

Comments

@marklundin
Copy link
Member

Before you can add a component to an entity, you currently need to register the component system with the app.

opts.componentSystems = [RenderComponentSystem]
const app = new AppBase(canvas);
app.init(opts);
app.start();

new Entity('cube').addComponent('render');

This has a couple of friction points:

  1. Creates unnecessary boilerplate. Engine users are required to manage a list of component system. This either means that systems are included when not needed, or worse, components are added without systems, causing errors and friction for developers
  2. In editor projects, every component systems is added by default, regardless of whether they're used or not.
  3. In both cases neither can be tree-shaken, as they're instantiated by the App

An optimal solution would;

  1. Only instantiate components systems when necessary
  2. Not require manually maintaining a list of component systems.
  3. Support tree-shaking

Proposal

Update the addComponent() method to resolve to a lazily loaded component system:

const createComponentSystem = async path => {
    const class = await import(path);
    return new class();
}

addComponent(type, data) {
  let system = this._app.systems[type];
  if (!system) {
    switch(type) {
      case 'camera' : await createComponentSystem('./CameraComponentSystem.js');
      case 'render' : await createComponentSystem('./RenderComponentSystem.js');
    }
  } 
  // ...
}

Benefits of this approach

  1. Maintains similar api addComponent('camera')
  2. Component Systems are lazily loaded.
  3. Simplifies engine only startup by deprecating the need for specifying systems array.
  4. Better supports tree-shaking.
  5. Editor projects only load necessary components.

Cons

Modifies the addComponent() to become async. Strictly speaking this is a breaking change as it requires await addComponent() which means a semantic major version bump.

Other options

An alternative non breaking change would be to use statically imported components which are not async

import CameraComponentSystem from './CameraComponentSystem'
addComponent(type) {
  // ...
  switch(type) {
      case 'camera' : CameraComponentSystem;
      ....
    } 
}

This still has the benefit of a not requiring specifying a component system array, but is harder to tree-shake.
@Maksims
Copy link
Contributor

Maksims commented Sep 24, 2024

An additional note is to ensure that the order of component systems and their update execution is deterministic and consistent between runs and through the development while using various number of systems.

And how that would work is no ESM?

@marklundin
Copy link
Member Author

marklundin commented Sep 24, 2024

I think the order of execution is something the engine would specify which would be independent of the order in which they're added.

UMD build would just inline those imports, which means you lose the benefit of tree-shaking, but engine only users can just handle this in their own build tool.

@LeXXik
Copy link
Contributor

LeXXik commented Sep 24, 2024

I like this.

@mvaligursky
Copy link
Contributor

3. In both cases neither can be tree-shaken, as they're instantiated by the App

Three-shaking works in the engine only project at the moment. The AppBase does not import any components, and so the components the user does not provide when the AppBase is created can be tree-shaken.

@mvaligursky
Copy link
Contributor

Modifies the addComponent() to become async.

This feels like it would be pretty inconvenient to use, for both the users and as well the engine developers, where a lot of functionality would need to be made async to support this?

@MAG-AdrianMeredith
Copy link
Contributor

I'm not convinced by the dynamic import (though its a really cool idea) but I like the idea of the auto system instanciation

@marklundin
Copy link
Member Author

An alternative solution is to extend parameters addComponent accepts to allow Component Classes too ie addComponent(CameraComponent). Then introduce a static system = CameraComponentSystem in the component which is used to instantiate and register the system.

class MySystem {}
class MyComponent { static system = MySystem }

entity.addComponent(MyComponent)

The string syntax addComponent('camera') would still be supported, so this is a non-breaking change. Strictly speaking this is a tight coupling between Component and System, but this already exist in the component lookup in the entity anyway.

@Maksims
Copy link
Contributor

Maksims commented Sep 26, 2024

An alternative solution is to extend parameters addComponent accepts to allow Component Classes too ie addComponent(CameraComponent). Then introduce a static system = CameraComponentSystem in the component which is used to instantiate and register the system.

class MySystem {}
class MyComponent { static system = MySystem }

entity.addComponent(MyComponent)

The string syntax addComponent('camera') would still be supported, so this is a non-breaking change. Strictly speaking this is a tight coupling between Component and System, but this already exist in the component lookup in the entity anyway.

That would require an additional import when using ESM?

@mvaligursky
Copy link
Contributor

The string syntax addComponent('camera') would still be supported

if the old way is still supported (and has to be for loading the scene from json), than this won't help with tree-shaking? What's the advantage then.

@marklundin
Copy link
Member Author

Editor project can be tree-shaken as we won't need to include the static list of every component systems in the start up script. Also for engine only it will simplify the boilerplate

@Maksims
Copy link
Contributor

Maksims commented Sep 26, 2024

Editor project can be tree-shaken as we won't need to include the static list of every component systems in the start up script. Also for engine only it will simplify the boilerplate

How you will ensure that the right component systems are in the engine, and avoid extra network file requests?

@marklundin
Copy link
Member Author

marklundin commented Sep 26, 2024 via email

@mvaligursky
Copy link
Contributor

but components can be added by string from scripts. Also the scene format has their names, and creates them by the string. How can you detect what is not used.

@Maksims
Copy link
Contributor

Maksims commented Sep 26, 2024

In order to do that for Editor there are few complexities:

  1. Identify all component systems that are used across all scenes.
  2. Identify all component systems that are used across all template assets which are not "excluded".
  3. Identify all component systems from scripts but only if ESM scripts are used, and only during bundling, as it wont work for string based addComponent.
  4. It has to do it for each Launch, otherwise the engine differences between launcher and published projects can lead to unintended differences and bugs.

The save from tree-shaking will be neglectable, complexity is huge, and if developers really need to save their tiny KBs, they can compile engine themselves excluding unnecessary component systems. Another branching in API with achieving same thing but a different way, leading to reduced learning curve long-term.

This will introduce complexity, branching API, more failure points, and potentially bad UX if things don't go smoothly in Editor, with extra delays when using Launcher. With seemingly tiny KBs savings, which are easily eclipsed by a single 512 PNG texture.

@mvaligursky
Copy link
Contributor

mvaligursky commented Sep 26, 2024

The original plan we had (and I don't see a better solution at the moment) is that somewhere in project settings, we'd have a list of components with checkboxes. By default all are ticked. User can untick some they don't need and the bundler will not import those components to tree-shake them. In a way equivalent to engine only project where we list those. Manual, but simple. Most people don't need to touch it, because as you said, the saving are minimal for majority of projects.

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

No branches or pull requests

5 participants