fritzy / ape-ecs Goto Github PK

I was accidentally calling world.removeEntity and then entity.destroy in sequence.

function removeEntity(ecs: World, entity: Entity) {
    const graphicData = entity.getOne(GraphicsComponent);
    // delete the sprite from memory here

    ecs.removeEntity(entity);
    entity.destroy();
}

I noticed that doing this led to the entity's components not being cleared before it was entered into the entity pool. As I was stepping through the code I could clearly see that my entities were in the pool and all of its components were still defined.

Later on in the code, I called createEntity and the latest destroyed entity would be popped from the pool, with all of its original components. So the two component definitions would be overlaid on top of each other.

This is very surprising. I would expect the second call would be a no-op or would throw an error saying this entity is already destroyed.

"My use case for many-to-many relationships is tracking, say, visibility between multiple entities. There's a bunch of entities with Sight and a partially overlapping set with Presence, and I want to update a Perception relation when entities get added to or removed from these queries, or when their position changes. Plus easily access related perception results from either end (the relation is not necessarily symmetric)."

Hi,

I tried running the example from the readme, but I get this.

 /myproject/node_modules/ape-ecs/src/world.js:5:21: Cannot resolve dependency 'uuid/v1'
  3 |  * @type {class}
  4 |  */
> 5 | const UUID = require('uuid/v1');
    |                     ^
  6 | const Entity = require('./entity');
  7 | const Query = require('./query');
  8 | const Component = require('./component');

The function isn't used in the file so removing the line solves the issue.

Due to things like World.prototype.registerComponent mutating the given klass.prototype, makes it not appealing to work with multiple worlds.

What's your thoughts on how to do that sort of multi-world/shard/chunk thing?

I'm not sure if this is a documentation problem, or a problem with my understanding.

The example code provided in the project's README does not work. I copy/pasted the code in the README, but instead of using frame.time.update({ in the update method, I needed to use frame.c.time.update({ as I got the error that frame.time was undefined.

Similarly, when modifying the example Gravity system, I noticed that:

const vector = entity.getOne('Vector')
vector.my += 9.807 * frame.time.deltaTime * .01
vector.update()

Seems to have two separate errors:

1. frame should be frameInfo
2. frame.time seems like it should be frameInfo.c.time

Similarly, the vector.my line in the above code looks like it should actually be vector.vector.my as the vector variable seems to be a wrapper. There is no my property on vector.

I just started exploring this library today, but these issues are extremely confusing. Any help would be greatly appreciated.

What's the recommended way to clean up the world once you're done with it in a way that avoids memory leaks?

Currently I'm doing:

  for (const entity of world.entities.values()) {
    entity.destroy();
  }
  world.entities.clear();

I'm not fond of hard coded string usage--especially in reference to code components because editors don't link the two. And code helpers like autocomplete, find references etc don't work w/ the string references.

I'm finding myself using the following to pass component references:

  const game = world.createEntity({
    id: 'game',
    components: [
      {type: Components.GameInfo.type()}
    ]
  });

Which I can make do with, but obviously I'd prefer if the library accepted either form (string or class) and got the type itself. Ie,

  const entity = world.createEntity({
    id: 'game',
    components: [
      {type: Components.GameInfo},
      {type: 'OtherComponent'}
    ]
  });

const gameInfo = entity.getOne(Components.GameInfo); // optionally support class reference
const otherComp = entity.getOne('OtherComponent'); // still support string name

If you're writing a Component in TS, you essentially have to define each property twice.

Internally, ES class properties use Object.defineProperty every time the class is initialized, which is slow. This is why this was avoided in the first place.

If no static properties object has been defined, we can check for class properties at registration and build it.

Clarify what that object is, how it is defined, and how property constructors work.

This is not currently allowed in the typescript declarations.

class Position extends Component {
    static properties = {
        x: 0, y: 0
    }
}

class ListenToPositions extends System {
    init() {
        this.subscribe(Position);
    }
}

Gives error

Argument of type 'typeof Position' is not assignable to parameter of type 'string'

This is already done for almost all the other functions in the API.

Currently the switch in components.js is case sensitive. Right now if the user writes <component> instead of <Component> there is no warning. Maybe the default case should warn if the string starts and ends with <> but no match was made? Thoughts?

I'm not positive, but it seems like a "CoreProperties collision" is only detected at "run" time, but this error could be thrown much earlier when registerComponent() is called.

The error I get is:

component.js:52 Uncaught Error: Cannot override property in Component definition: id

(I will just have to live with the fact that I can't use id)

I'm trying to get TypeScript to be aware of the properties of a Component, but because Components' properties are static, it's not possible to create a generic Component type from those properties.

I put together a TypedComponent definition that gets what I'm after, but it's pretty messy and I'm not sure it's an accepted pattern in TypeScript.

Let me know your thoughts, I'm going to continue developing with it and see how it feels, but so far it's been a useful type.

// types
declare type Constructor<T> = new (...args: any[]) => T;
export function TypedComponent<TProperties extends {}, TBase extends Component>(
  props: TProperties
): typeof Component &
  Constructor<TBase & TProperties> & {
    properties: TProperties;
  };

// implementation
function TypedComponent(props) {
    const newClass = class TypedComponent extends Component {};
    newClass.properties = {...props};
    return newClass;
}

Then in my component definition, then in a system:

// usage:
class Position extends TypedComponent({x: 0, y: 0}) {}
assert(typeof Position.properties.x === 'number');
// ...
const position: Position = entity.getOne(Position);
assert(typeof position.x === 'number');

Object, Set, Array proxy types or extensions?

I am adding the ability to load components from a json configuration in my game. I think that ape-ecs should export componentReserved so that I can check against this list in my json configs.

I'm trying to use the Component change subscriptions system described here and I'm having trouble subscribing to change events.

I've got my Position component on which I've set

this.changeEvents = true;

in the constructor. I only change the position's data by calling its update method here.

Then I subscribe to the events in my OnMoveEventHandler system.

I'm successfully seeing add events but I'm not seeing any change events.

Is this a bug or am I missing something in configuring this?

Scenario:

• using Webpack 5
• using PIXI.js as Dependency

Problem Description:

When Webpack bundles all packages together conflicting declaration names of functions/classes are renamed to solve this issue without requiring the developer to act. The applied resolution is: one of the declaration names remains as it is and the other ones will be renamed to something like ${declaration name}__${declaration name}.

Since Pixi is used as a dependency, it is inserted before my code in the final bundle. Therefore my DisplayObject component class is renamed to DisplayObject_DisplayObject which is then of course registered as DisplyObject_DisplayObject inside the worlds component registry.

This leads to the error ... DisplayObject is not registered ... while creating entities.

Note:
This error first came up while running the project in production mode without modification.
Therefore I suggest the tests should include testing of minified and bundled code.

I am currently working on a solution, but I'd like to test a bit further.
Branch | commit of working solution

Hi,
I would like to suggest a small change in the System creation process to have a more flexible solution for Systems with custom parameters.

Example for a system with custom parameters:

const world = new EcsApe.World()
world.registerSystem('render', new SystemWithCustomParameters(world, 1, 2, 3) )

where:

class SystemWithCustomParameters extends ApeECS.System {
  constructor (world, param1, param2, param3) {
    super(world)
    this.customInit(param1, param2, param3)
  }

  customInit(parameters) {
      // do stuff with parameters here
  }
}

The init() that ApeECS.System is offering cannot be used here. My suggestion would be to pass through additional parameters from the constructor, such as:

class System {

  constructor(world, ...args) {
    this.world = world
    // ..
    this.init(...args)
  }  
 }

This would allow to have all additional parameters available in the init() function, if needed. Please have a look also my js-fiddle I created here: https://jsfiddle.net/cv85mezq/106/

If we change the world.registerSystem as well, like so:

registerSystem(group, system, ...args) {

    if (typeof system === 'function') {
      system = new system(this, ...args);
    }
    if (!this.systems.has(group)) {
      this.systems.set(group, new Set());
    }
    this.systems.get(group).add(system);
    return system;
  }

we could even let the World create the System with custom parameters like this:

world.registerSystem('render', SystemWithCustomParameters, 1, 2, 3)

What do you think of this?

Hi there,

Webpack 5 has the auto-polyfill functionality disabled and because utils/IdGenerator is making use of the nodejs crypto library and Buffer class, I have to include many browserify-polyfills to get the project to compile without errors. Running the compiled script in the browser leads to the next issues regarding the missing process object, which is currently not resolved.

Since Buffer and crypto are only used for ID generation, maybe it is a good idea to use a different approach and make it browser compatible without the use of polyfills.

Cheers,

Martin

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }'
	- install 'crypto-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "crypto": false }
 @ ../../node_modules/ape-ecs/src/entity.js 2:20-49
 @ ../../node_modules/ape-ecs/src/index.js 6:10-29
 @ ./src/ape.ts 1:0-32 18:16-21

Implementing the solution webpack recommends { "crypto": require.resolve("crypto-browserify") }' results in this:

ERROR in ../../node_modules/safer-buffer/safer.js 5:13-30
Module not found: Error: Can't resolve 'buffer' in '/Users/martinemmert/Work/03_projects/games/last-man-standing/node_modules/safer-buffer'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "buffer": require.resolve("buffer/") }'
	- install 'buffer'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "buffer": false }
 @ ../../node_modules/asn1.js/lib/asn1/decoders/pem.js 4:15-45
 @ ../../node_modules/asn1.js/lib/asn1/decoders/index.js 6:15-31
 @ ../../node_modules/asn1.js/lib/asn1.js 10:16-42
 @ ../../node_modules/parse-asn1/asn1.js 5:11-29
 @ ../../node_modules/parse-asn1/index.js 1:11-28
 @ ../../node_modules/browserify-sign/browser/sign.js 7:16-37
 @ ../../node_modules/browserify-sign/browser/index.js 5:11-28
 @ ../../node_modules/crypto-browserify/index.js 39:11-37
 @ ../../node_modules/ape-ecs/src/util.js 1:15-32
 @ ../../node_modules/ape-ecs/src/entity.js 2:20-49
 @ ../../node_modules/ape-ecs/src/index.js 6:10-29
 @ ./src/ape.ts 1:0-32 18:16-21

Implementing the solution webpack recommends { "buffer": require.resolve("buffer/") }' results in this:

ERROR in ../../node_modules/cipher-base/index.js 2:16-43
Module not found: Error: Can't resolve 'stream' in '/Users/martinemmert/Work/03_projects/games/last-man-standing/node_modules/cipher-base'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
	- install 'stream-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "stream": false }
 @ ../../node_modules/create-hmac/browser.js 4:11-33
 @ ../../node_modules/crypto-browserify/index.js 5:21-58
 @ ../../node_modules/ape-ecs/src/util.js 1:15-32
 @ ../../node_modules/ape-ecs/src/entity.js 2:20-49
 @ ../../node_modules/ape-ecs/src/index.js 6:10-29
 @ ./src/ape.ts 1:0-32 18:16-21

Which resolves any compiling issues but rises new js runtime errors

Uncaught ReferenceError: process is not defined
    at eval (default-encoding.js:3)
    at Object.../../node_modules/pbkdf2/lib/default-encoding.js (main.bundle.js:1669)
    at __webpack_require__ (runtime~main.bundle.js:33)
    at fn (runtime~main.bundle.js:239)
    at eval (async.js:4)
    at Object.../../node_modules/pbkdf2/lib/async.js (main.bundle.js:1662)
    at __webpack_require__ (runtime~main.bundle.js:33)
    at fn (runtime~main.bundle.js:239)
    at eval (browser.js:1)
    at Object.../../node_modules/pbkdf2/browser.js (main.bundle.js:1655)

tl;dr; The reason why this behavior is occurring is currently not known and I still investigate the issue.
It can be resolved by deactivating the useDefineForClassFields compiler option of Typescript.

Both this.view and this.displayObjectQuery are undefined when accessing them inside the update method and are undefined if the code is paused by the debugger. At the end of init both are defined.

I need to have a better understanding of the System class to go on with my investigations, but if by chance anyone else has an idea - I would be glad if we can resolve this.

Note: if placed some comments into the code as hints

import { Query, System, World } from 'ape-ecs';
import type { View } from '../components/View';

export class RenderSystem extends System {
  public view?: View;
  public displayObjectQuery!: Query;
 
  init() {
    this.view = this.world.getEntity('RenderView');
    this.displayObjectQuery = this.createQuery().fromAll('DisplayObject').persist();
    console.log(this.view) // output of the RenderView Entity object
  }

  update() {
    if (!this.view || !this.view.stage || !this.view.renderer) return;
    // is never executed beacue this.view is undefined
    this.displayObjectQuery.execute();
    this.view.renderer.render(this.view.stage);
  }
}

Typescript compiler compiles that to:

import { System } from 'ape-ecs';
export class RenderSystem extends System {
    view;
    displayObjectQuery;
    init() {
        this.view = this.world.getEntity('RenderView');
        this.displayObjectQuery = this.createQuery().fromAll('DisplayObject').persist();
        console.log(this.view); // output of the RenderView Entity object
    }
    update() {
        if (!this.view || !this.view.stage || !this.view.renderer)
            return;
        // is never executed beacue this.view is undefined
        this.displayObjectQuery.execute();
        this.view.renderer.render(this.view.stage);
    }
}

Babel transpiles that to:

function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; }

import { System } from 'ape-ecs';
export class RenderSystem extends System {
  constructor(...args) {
    super(...args);

    // if I remove this by hand after transpiling, this.view is defined during update
    _defineProperty(this, "view", void 0);  
    // if I remove this by hand after transpiling, this.displayObjectQuery is defined during update
    _defineProperty(this, "displayObjectQuery", void 0);
  }

  init() {
    this.view = this.world.getEntity('RenderView');
    this.displayObjectQuery = this.createQuery().fromAll('DisplayObject').persist();
    console.log(this.view); // output of the RenderView Entity object
  }

  update() {
    if (!this.view || !this.view.stage || !this.view.renderer) return; // is never executed beacue this.view is undefined

    this.displayObjectQuery.execute();
    this.view.renderer.render(this.view.stage);
  }

}

Documentation needs to be updated to show that methods that type a type-name string can also take the type constructor itself, directly.

from* methods only add results to the query, while "not" and "only" filter only remove results that were added by from* methods.

This is a bit of an interesting problem. Let's say I have 5 systems running in order. If I save the world within the 3rd system, then shut everything down, the next time I load the world, I want to somehow jump right back into the system stack at the 3rd system, so that the first two systems don't get processed twice within the same tick.

What do you think of adding a variation on runSystems for this purpose?

world.runSystemsFromSystem("default", ThirdSystem);

So docs/World.md says "👀 See " the system docs for details on registering and running.

Which docs/System.md section says "👀 See " the world docs.

I've been had! Guess I'll have to make do with reading the source for now ;-)

Well, kind of a bummer but - this method doesn't change the prefix currently.

The documentation of Query.persist and Query.execute might be vague about their use cases.

I understood from the docs that calling execute is used to run the actual query and gets all matching Entities as a result and If I want to track added and removed entities, I use persist.

The following code shows my implementation of how I thought the queries should be used.
I always received the same results from calling execute and saw an endless stream of [RenderSystem]: added new child to stage in my console. Adding .persist() to the query creation solves the issue.

import { Query, System } from 'ape-ecs';
import type { DisplayObject } from '../components/DisplayObject';
import type { View } from '../components/View';

export class RenderSystem extends System {
  private view?: View;
  private newDisplayObjects!: Query;

  init() {
    this.view = this.world.getEntity('RenderView')?.c.view;
    this.newDisplayObjects = this.createQuery().fromAll('DisplayObject', 'New');
    if (process.env.NODE_ENV === 'development') {
      if (!this.view || !this.view.stage || !this.view.renderer) {
        if (!this.view) console.warn(`[${this.constructor.name}]: the view is undefined`);
        if (!this.view?.stage)
          console.warn(`[${this.constructor.name}]: the stage of the view is undefined`);
        if (!this.view?.renderer)
          console.warn(`[${this.constructor.name}]: the renderer of the view is undefined`);
      }
    }
  }

  update() {
    if (!this.view || !this.view.stage || !this.view.renderer) return;

    this.newDisplayObjects.execute().forEach((entity) => {
      const displayObjects: Set<DisplayObject> = entity.getComponents('DisplayObject');
      displayObjects.forEach((displayObject) => {
        if (displayObject.value) {
          this.view?.stage?.addChild(displayObject.value);
          console.log('[%s]: added new child to stage', this.constructor.name);
        }
      });
      
      entity.removeTag('New');
    });

    // update the view
    this.view.renderer.render(this.view.stage);
  }
}

From what I understand after reading through the code, the update function is a key part of understanding the queries, and the persist function is required if the components of an entity will change.
It might be more clear how a Query works if this is described.

💭 If you persist a LOT of Queries, it can have a performance from creating Entities, or adding/removing Components or Tags.

This comment made me think that I should use persist to track added and removed entities only when required.

if a property default is an object, then it should be recursively set as sub-properties on this._values

Currently queries can trackAdded, trackRemoved. We could also have trackChanged, trackChangedValues, trackChangedComponents.

Hi everyone,

I'm trying to modify the source to add some Query logic like "any" in addition to "has".

While testing, I got error message like this:

TypeError: hasSet[0] is not iterable

Which is reported at

QueryCache.initial() {
    if (hasSet && hasSet.length > 0) {
        // an undefined been pushed into hasSet, length == 1
        results = new Set([...hasSet[0]]); 
    }
}

This is because i had misspelled the component's name. Since the message is confusing, I think it's better to check and throw error. But the ECS.entityComponents is a JS Map.

So is wrapping this field as a class/structure the correct way to do this?

It's not wise to check every time as this only happens the first time.

BTW, if the logic extending is helpful, i'm every happy to fork and file a pull request.

split README into README and API docs

If two systems happen to have the same criteria for a query, is there currently a way to share the query?

this.world.systems.get("OtherSystem").myPersistedQuery.execute()?

Seems like a bad idea to have the systems know about one another, but it would be nice to be able to reuse the same underlying query somehow since it seems like adding a lot of persisted queries is not great for performance. Maybe queries could be given an id to access with world.getQuery(id)?

The docs state that if both are declared, a query should not match unless all components in fromAll are satisfied in addition to having one or more components from fromAny. In practice, it seems like including a fromAny will ignore the fromAll requirement, at least when checking for a tag that gets added/removed.

Not sure if it's necessarily an issue, but you can't just JSON.stringify the result of an entity.getObject because it might have circular references.

This is easy to do if you create a value on a component which is an array of entities.

Currently, my default code formatting settings diverge from the ones used in the project.
It might be helpful to add a tool like prettier with your preferences to keep the code consistent.

In fact ape should catch things like that whenever an id, component, or entity is passed.

It seems like default values of components are actually the same (Warning I'm on an old release 609c511)

I have this component:

export class UnitBuildQueue extends Component {
static properties = {
    queue: [],
    frameAdded: 0,
  };
};

I have this code:

let e0, e1; // two entities initialized elsewhere
e0.addComponent({
  type: 'UnitBuildQueue',
  key: 'q',
  frameAdded: 10,
});


e1.addComponent({
  type: 'UnitBuildQueue',
  key: 'q',
  frameAdded: 20,
});

e0.c.q.queue.push('foo');

console.log(e1.c.q.queue); // THIS HAS FOO

This is fixed if I instead do this:

e0.addComponent({
  type: 'UnitBuildQueue',
  key: 'q',
  frameAdded: 10,
  queue: [],  // THE FIX
});

Any thoughts? Did this ever get fixed?