Skip to content

Commit 0623abc

Browse files
authored
Fix bug in a-scene's setAttribute in case of uninitialized systems (#5482)
Co-authored-by: Noeri Huisman <mrxz@users.noreply.github.com>
1 parent c8e53ab commit 0623abc

File tree

2 files changed

+88
-3
lines changed

2 files changed

+88
-3
lines changed

‎src/core/scene/a-scene.js‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -515,10 +515,15 @@ class AScene extends AEntity {
515515
* setAttribute.
516516
*/
517517
setAttribute (attr, value, componentPropValue) {
518-
var system = this.systems[attr];
519-
if (system) {
518+
// Check if system exists (i.e. is registered).
519+
if (systems[attr]) {
520520
ANode.prototype.setAttribute.call(this, attr, value);
521-
system.updateProperties(value);
521+
522+
// Update system instance, if initialized on the scene.
523+
var system = this.systems[attr];
524+
if (system) {
525+
system.updateProperties(value);
526+
}
522527
return;
523528
}
524529
AEntity.prototype.setAttribute.call(this, attr, value, componentPropValue);

‎tests/core/scene/a-scene.test.js‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
/* global AFRAME, assert, CustomEvent, process, screen, sinon, setup, suite, teardown, test, THREE, EventTarget */
22
var AScene = require('core/scene/a-scene').AScene;
3+
var AEntity = require('core/a-entity').AEntity;
4+
var ANode = require('core/a-node').ANode;
35
var components = require('core/component').components;
46
var scenes = require('core/scene/scenes');
57
var setupCanvas = require('core/scene/a-scene').setupCanvas;
@@ -498,6 +500,84 @@ suite('a-scene (without renderer) - WebXR', function () {
498500
assert.notOk(setSizeSpy.called);
499501
});
500502
});
503+
504+
suite('setAttribute', function () {
505+
var sceneEl;
506+
setup(function (done) {
507+
sceneEl = this.el;
508+
sceneEl.addEventListener('loaded', function () {
509+
done();
510+
});
511+
});
512+
513+
test('can set a component with a string', function () {
514+
sceneEl.setAttribute('fog', 'type: exponential; density: 0.75');
515+
var fog = sceneEl.getAttribute('fog');
516+
assert.equal(fog.type, 'exponential');
517+
assert.equal(fog.density, 0.75);
518+
});
519+
520+
test('can set a component with an object', function () {
521+
var value = {type: 'exponential', density: 0.75};
522+
sceneEl.setAttribute('fog', value);
523+
var fog = sceneEl.getAttribute('fog');
524+
assert.equal(fog.type, 'exponential');
525+
assert.equal(fog.density, 0.75);
526+
});
527+
528+
test('can clobber component attributes with an object and flag', function () {
529+
sceneEl.setAttribute('fog', 'type: exponential; density: 0.75');
530+
sceneEl.setAttribute('fog', {type: 'exponential'}, true);
531+
var fog = sceneEl.getAttribute('fog');
532+
assert.equal(fog.type, 'exponential');
533+
assert.equal(fog.density, 0.00025);
534+
assert.equal(sceneEl.getDOMAttribute('fog').density, undefined);
535+
});
536+
537+
test('can set a single component via a single attribute', function () {
538+
sceneEl.setAttribute('fog', 'type', 'exponential');
539+
assert.equal(sceneEl.getAttribute('fog').type, 'exponential');
540+
});
541+
542+
test('can set system attribute with a string', function () {
543+
sceneEl.setAttribute('renderer', 'anisotropy: 4; toneMapping: ACESFilmic');
544+
assert.equal(sceneEl.getAttribute('renderer').anisotropy, 4);
545+
assert.equal(sceneEl.getAttribute('renderer').toneMapping, 'ACESFilmic');
546+
});
547+
548+
test('can set system attribute with an object', function () {
549+
sceneEl.setAttribute('renderer', {anisotropy: 4, toneMapping: 'ACESFilmic'});
550+
assert.equal(sceneEl.getAttribute('renderer').anisotropy, 4);
551+
assert.equal(sceneEl.getAttribute('renderer').toneMapping, 'ACESFilmic');
552+
});
553+
554+
test('can set system attribute value before system initializes', function () {
555+
delete sceneEl.systems['renderer'];
556+
sceneEl.setAttribute('renderer', 'anisotropy: 4');
557+
assert.equal(sceneEl.getAttribute('renderer'), 'anisotropy: 4');
558+
sceneEl.initSystem('renderer');
559+
assert.equal(sceneEl.getAttribute('renderer').anisotropy, 4);
560+
});
561+
562+
test('calls a-entity setAttribute for non-systems (component)', function () {
563+
var spy = this.sinon.spy(AEntity.prototype, 'setAttribute');
564+
sceneEl.setAttribute('fog', 'type', 'exponential');
565+
assert.ok(spy.calledOnce);
566+
});
567+
568+
test('calls a-entity setAttribute for non-systems (HTML attribute)', function () {
569+
var spy = this.sinon.spy(AEntity.prototype, 'setAttribute');
570+
sceneEl.setAttribute('data-custom-attr', 'value');
571+
assert.ok(spy.calledOnce);
572+
});
573+
574+
test('calls a-node setAttribute for systems', function () {
575+
var spy = this.sinon.spy(ANode.prototype, 'setAttribute');
576+
sceneEl.setAttribute('renderer', 'anisotropy: 4');
577+
assert.ok(spy.calledOnce);
578+
assert.equal(sceneEl.getAttribute('renderer').anisotropy, 4);
579+
});
580+
});
501581
});
502582

503583
/**

0 commit comments

Comments
 (0)