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

mod: Component and SceneModule .add .remove #377

Merged
merged 1 commit into from
Apr 25, 2018
Merged
Show file tree
Hide file tree
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
1,273 changes: 78 additions & 1,195 deletions build/whs.js

Large diffs are not rendered by default.

1,273 changes: 78 additions & 1,195 deletions build/whs.module.js

Large diffs are not rendered by default.

23 changes: 11 additions & 12 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions src/components/__tests__/components.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ app.start();
category.forEach(component => {
test(component, () => {
const element = new WHS[component]();
expect.assertions(1);

app.add(element).then(() => {
expect(app.children).toContain(element);
});
Expand Down
46 changes: 25 additions & 21 deletions src/core/Component.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {extend, transformData} from '../utils/index';
import {ModuleSystem} from './ModuleSystem';
import {ModuleManager} from './ModuleManager';
import {ManagerError} from './errors';
import {ManagerError, CompositionError} from './errors';

/**
* @class Component
Expand Down Expand Up @@ -146,41 +146,45 @@ class Component extends ModuleSystem {
* @return {Promise} Resolved when action is done.
* @memberof module:core.Component
*/
add(object) {
object.parent = this;

return new Promise((resolve, reject) => {
this.defer(() => {
object.defer(() => {
const {native} = object;
if (!native) reject();
async add(object) {
if (object.parent) await object.parent.remove(object);

const addPromise = this.applyBridge({onAdd: object}).onAdd;
await this.wait();
await object.wait();

const resolver = () => {
this.native.add(native);
this.children.push(object);
if (!object.native) {
throw new CompositionError(
'Component',
'there is no object.native',
this
);
}

resolve(object);
};
object.parent = this;
await this.applyBridge({onAdd: object}).onAdd;
this.native.add(object.native);
this.children.push(object);

if (addPromise instanceof Promise) addPromise.then(resolver);
else resolver();
});
});
});
return object;
}

/**
* @method remove
* @instance
* @description Remove a child `Component`.
* @param {Component} object - Component that should be a **child** of this Component.
* @return {Promise} Resolved when action is done.
* @memberof module:core.Component
*/
remove(object) {
async remove(object) {
if (object.parent !== this) return;

await this.wait();
await object.wait();

object.parent = null;
this.native.remove(object.native);
this.children.splice(this.children.indexOf(object), 1);
}

/**
Expand Down
14 changes: 14 additions & 0 deletions src/core/__tests__/Component.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,17 @@ test('.applyModule()', () => {
test('.applyBridge()', () => {
expect(component.applyBridge({hello: 'world'}).hello).toBe('world');
});

test('.add() & .remove()', async () => {
const box = new WHS.Box();
const group = new WHS.Group();
expect(group.children.length).toBe(0);
await group.add(box);
expect(group.children.length).toBe(1);
await group.add(box);
expect(group.children.length).toBe(1);
await group.remove(box);
expect(group.children.length).toBe(0);
await group.remove(box);
expect(group.children.length).toBe(0);
});
41 changes: 41 additions & 0 deletions src/modules/__tests__/modules.spec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import '../../polyfill';
import * as WHS from '../../index';
import {
Scene,
} from 'three';
import {FogModule} from '../app/FogModule';
import gl from 'gl';
import { Box } from '../../index';

const app = new WHS.App();
const modules = {};
Expand All @@ -16,6 +20,43 @@ test('SceneModule', () => {
modules.scene = new WHS.SceneModule();
});

test('SceneModule .add() & .remove()', async () => {
const a = new WHS.App([
new WHS.SceneModule(),
]);
const b = new WHS.Box();
const g = new WHS.Group();

expect(a.children.length).toBe(0);
await a.add(g);
expect(a.children.length).toBe(1);
await a.add(g);
expect(a.children.length).toBe(1);
await a.add(b);
expect(a.children.length).toBe(2);
await g.add(b);
expect(a.children.length).toBe(1);
expect(g.children.length).toBe(1);
await a.remove(g);
expect(a.children.length).toBe(0);
});

test('SceneModule .setScene() & .getScene()', async () => {
const a = new WHS.App([
new WHS.SceneModule(),
]);
const b = new WHS.Box();
expect(a.children.length).toBe(0);
await a.add(b);
expect(a.children.length).toBe(1);
const oldS = a.getScene();
const s = new Scene();
a.setScene(s);
expect(a.children.length).toBe(0);
a.setScene(oldS);
expect(a.children.length).toBe(1);
})

test('DefineModule', () => {
modules.camera = new WHS.DefineModule('camera', new WHS.PerspectiveCamera());
});
Expand Down
85 changes: 49 additions & 36 deletions src/modules/app/SceneModule.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {
Scene
Scene,
} from 'three';

const SYMBOL_CHILDREN_FOR_SCENE = Symbol('SYMBOL_CHILDREN_FOR_SCENE')

/**
* @class SceneModule
* @category modules/app
Expand All @@ -18,40 +20,51 @@ export class SceneModule {
}

integrate(self) {
this.children = [];

this.add = function (object) {
object.parent = this;

return new Promise((resolve, reject) => {
object.defer(() => {
const {native} = object;
if (!native) reject();

const addPromise = this.applyBridge({onAdd: object}).onAdd;

const resolver = () => {
self.scene.add(native);
this.children.push(object);

resolve(object);
};

if (addPromise instanceof Promise)
addPromise.then(resolver);
else resolver();
});
});
};

this.remove = function (object) {
object.parent = null;
self.scene.remove(object.native);
};

this.setScene = function (scene) {
self.scene = scene;
this.manager.set('scene', scene);
};
Object.assign(
this,
{
async add(object) {
if (object.parent) await object.parent.remove(object);

await object.wait();

if (!object.native) {
throw new CompositionError(
'SceneModule',
'there is no object.native',
this
);
}

object.parent = this;
await this.applyBridge({onAdd: object}).onAdd;
self.scene.add(object.native);
this.children.push(object);

return object;
},
async remove(object) {
if (object.parent !== this) return;

await object.wait();

object.parent = null;
self.scene.remove(object.native);
this.children.splice(this.children.indexOf(object), 1);
},
_setScene(scene) {
this.children = scene[SYMBOL_CHILDREN_FOR_SCENE] = scene[SYMBOL_CHILDREN_FOR_SCENE] || []
Copy link
Member

Choose a reason for hiding this comment

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

Hey, thanks for the PR! Everything seems good. Can you explain me why do we use Symbol here? And I'll merge it 😀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The scene is a THREE.Scene object. We need store this.children in the object. If we use scene._varName = this.children, there is a risk that the three.js team may use _varName in THREE.Scene later. But in general, Symbol is safe.

Copy link
Member

Choose a reason for hiding this comment

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

@BrookShuihuaLee Ok, thanks for explanation! Also, could you contact me in discord for further cooperation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sasha240100 ok, i've sent you a friend request 😄

self.scene = scene;
},
setScene(scene) {
this._setScene(scene);
this.manager.set('scene', scene);
},
getScene() {
return self.scene;
},
},
);
if (self.scene) this._setScene(self.scene);
}
}