Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

Return more readonly components #233

Merged
merged 4 commits into from
Aug 4, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/Entity.d.ts
Original file line number Diff line number Diff line change
@@ -20,16 +20,16 @@ export class Entity {
* @param includeRemoved Whether a component that is staled to be removed should be also considered
*/
getComponent<C extends Component<any>>(
Component: ComponentConstructor<C>,
includeRemoved?: boolean
): C;
Component: ComponentConstructor<C>,
includeRemoved?: boolean
): Readonly<C> | undefined;

/**
* Get a component that is slated to be removed from this entity.
*/
getRemovedComponent<C extends Component<any>>(
Component: ComponentConstructor<C>
): C;
): Readonly<C> | undefined;

/**
* Get an object containing all the components on this entity, where the object keys are the component types.
@@ -52,7 +52,7 @@ export class Entity {
*/
getMutableComponent<C extends Component<any>>(
Component: ComponentConstructor<C>
): C;
): C | undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think this will be annoying? Should these functions throw an error (in development mode?) instead if the component does not exist?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's valuable to return undefined so you could check in your code if it exist or not instead of catching an error


/**
* Add a component to the entity.
11 changes: 10 additions & 1 deletion src/Entity.js
Original file line number Diff line number Diff line change
@@ -43,7 +43,11 @@ export class Entity {
}

getRemovedComponent(Component) {
return this._componentsToRemove[Component._typeId];
const component = this._componentsToRemove[Component._typeId];

return process.env.NODE_ENV !== "production"
? wrapImmutableComponent(Component, component)
: component;
}

getComponents() {
@@ -60,6 +64,11 @@ export class Entity {

getMutableComponent(Component) {
var component = this._components[Component._typeId];

if (!component) {
return;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No point going through queries if the component does not exist.

Copy link
Member

Choose a reason for hiding this comment

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

nice catch yep

for (var i = 0; i < this.queries.length; i++) {
var query = this.queries[i];
// @todo accelerate this check. Maybe having query._Components as an object
86 changes: 81 additions & 5 deletions test/unit/entity.test.js
Original file line number Diff line number Diff line change
@@ -193,7 +193,7 @@ test("remove entity", async t => {
t.is(world.entityManager.count(), 0);
});

test("get component includeRemoved", async t => {
test("get component development", async t => {
var world = new World();

world.registerComponent(FooComponent);
@@ -202,16 +202,92 @@ test("get component includeRemoved", async t => {
var entity = world.createEntity();
entity.addComponent(FooComponent);
const component = entity.getComponent(FooComponent);

t.throws(() => (component.variableFoo = 4));

entity.removeComponent(FooComponent);

t.is(entity.hasComponent(FooComponent), false);
t.is(entity.getComponent(FooComponent), undefined);

t.is(entity.hasRemovedComponent(FooComponent), true);
t.deepEqual(entity.getRemovedComponent(FooComponent), component);
const removedComponent = entity.getComponent(FooComponent, true);

t.throws(() => (removedComponent.variableFoo = 14));
});

test("get component production", async t => {
const oldNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = "production";
var world = new World();

world.registerComponent(FooComponent);

// Sync
var entity = world.createEntity();
entity.addComponent(FooComponent);
const component = entity.getComponent(FooComponent);

t.notThrows(() => (component.variableFoo = 4));

entity.removeComponent(FooComponent);

t.is(entity.hasComponent(FooComponent), false);
t.is(entity.getComponent(FooComponent), undefined);

const removedComponent = entity.getComponent(FooComponent, true);

t.notThrows(() => (removedComponent.variableFoo = 14));

process.env.NODE_ENV = oldNodeEnv;
});

test("get removed component development", async t => {
var world = new World();

world.registerComponent(FooComponent);

// Sync
var entity = world.createEntity();
entity.addComponent(FooComponent);
entity.removeComponent(FooComponent);

const component = entity.getRemovedComponent(FooComponent);

t.throws(() => (component.variableFoo = 4));
});

test("get removed component production", async t => {
const oldNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = "production";
var world = new World();

world.registerComponent(FooComponent);

// Sync
var entity = world.createEntity();
entity.addComponent(FooComponent);
entity.removeComponent(FooComponent);

const component = entity.getRemovedComponent(FooComponent);

t.notThrows(() => (component.variableFoo = 4));

process.env.NODE_ENV = oldNodeEnv;
});

test("get mutable component", async t => {
var world = new World();

world.registerComponent(FooComponent);

// Sync
var entity = world.createEntity();
entity.addComponent(FooComponent);
const component = entity.getMutableComponent(FooComponent);

t.notThrows(() => (component.variableFoo = 4));

t.is(entity.hasComponent(FooComponent, true), true);
t.deepEqual(entity.getComponent(FooComponent, true), component);
t.deepEqual(entity.getMutableComponent(BarComponent), undefined);
});

test("Delete entity from entitiesByNames", async t => {