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

[FIX] Ensure consistent function return types #4153

Merged
merged 5 commits into from
Apr 1, 2022
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
4 changes: 2 additions & 2 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function spacesToTabs() {

return {
transform(code, id) {
if (!filter(id)) return;
if (!filter(id)) return undefined;
return {
code: code.replace(/ {2}/g, '\t'),
map: null
Expand All @@ -50,7 +50,7 @@ function shaderChunks(removeComments) {

return {
transform(code, id) {
if (!filter(id)) return;
if (!filter(id)) return undefined;

code = code.replace(/\/\* glsl \*\/\`((.|\r|\n)*)\`/, (match, glsl) => {

Expand Down
4 changes: 2 additions & 2 deletions src/anim/controller/anim-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,12 @@ class AnimController {

removeNodeAnimations(nodeName) {
if (ANIM_CONTROL_STATES.indexOf(nodeName) !== -1) {
return;
return false;
}
const state = this._findState(nodeName);
if (!state) {
Debug.error('Attempting to unassign animation tracks from a state that does not exist.');
return;
return false;
}

state.animations = [];
Expand Down
5 changes: 5 additions & 0 deletions src/anim/controller/anim-state.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Debug } from '../../core/debug.js';

import { AnimBlendTree1D } from './anim-blend-tree-1d.js';
import { AnimBlendTreeCartesian2D } from './anim-blend-tree-2d-cartesian.js';
import { AnimBlendTreeDirectional2D } from './anim-blend-tree-2d-directional.js';
Expand Down Expand Up @@ -65,6 +67,9 @@ class AnimState {
case ANIM_BLEND_DIRECT:
return new AnimBlendTreeDirect(state, parent, name, point, parameters, children, syncAnimations, createTree, findParameter);
}

Debug.error(`Invalid anim blend type: ${type}`);
return undefined;
}

_getNodeFromPath(path) {
Expand Down
4 changes: 2 additions & 2 deletions src/framework/app-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -1471,11 +1471,11 @@ class Application extends EventHandler {
* @returns {object} A object containing the values calculated to use as width and height.
*/
resizeCanvas(width, height) {
if (!this._allowResize) return; // prevent resizing (e.g. if presenting in VR HMD)
if (!this._allowResize) return undefined; // prevent resizing (e.g. if presenting in VR HMD)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these should be null instead of undefined .. as the return type is object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly. I'm mainly just trying to keep the functionality identical and satisfy ESLint.


// prevent resizing when in XR session
if (this.xr && this.xr.session)
return;
return undefined;

const windowWidth = window.innerWidth;
const windowHeight = window.innerHeight;
Expand Down
1 change: 1 addition & 0 deletions src/framework/components/anim/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,7 @@ class AnimComponent extends Component {
return param.value;
}
Debug.log(`Cannot get parameter value. No parameter found in anim controller named "${name}" of type "${type}"`);
return undefined;
}

setParameterValue(name, type, value) {
Expand Down
4 changes: 3 additions & 1 deletion src/framework/components/collision/system.js
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ class CollisionMeshSystemImpl extends CollisionSystemImpl {
}

createPhysicalShape(entity, data) {
if (typeof Ammo === 'undefined') return;
if (typeof Ammo === 'undefined') return undefined;

if (data.model || data.render) {

Expand All @@ -415,6 +415,8 @@ class CollisionMeshSystemImpl extends CollisionSystemImpl {

return shape;
}

return undefined;
}

recreatePhysicalShapes(component) {
Expand Down
2 changes: 1 addition & 1 deletion src/framework/components/script-legacy/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ class ScriptLegacyComponent extends Component {
if (fn) {
return fn.apply(instances[name].instance, args);
}

}
return undefined;
}

onEnable() {
Expand Down
8 changes: 6 additions & 2 deletions src/framework/components/scroll-view/component.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Debug } from '../../../core/debug.js';

import { math } from '../../../math/math.js';
import { Vec2 } from '../../../math/vec2.js';
import { Vec3 } from '../../../math/vec3.js';
Expand Down Expand Up @@ -441,7 +443,8 @@ class ScrollViewComponent extends Component {
return this.vertical;
}

console.warn('Unrecognized orientation: ' + orientation);
Debug.warn(`Unrecognized orientation: ${orientation}`);
return undefined;
}

_getScrollbarVisibility(orientation) {
Expand All @@ -451,7 +454,8 @@ class ScrollViewComponent extends Component {
return this.verticalScrollbarVisibility;
}

console.warn('Unrecognized orientation: ' + orientation);
Debug.warn(`Unrecognized orientation: ${orientation}`);
return undefined;
}

_getSign(orientation) {
Expand Down
2 changes: 1 addition & 1 deletion src/framework/components/sound/slot.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class SoundSlot extends EventHandler {
// If not loaded and doesn't have asset - then we cannot play it. Warn and exit.
if (!this.isLoaded && !this._hasAsset()) {
Debug.warn(`Trying to play SoundSlot ${this.name} but it is not loaded and doesn't have an asset.`);
return;
return undefined;
}

const instance = this._createInstance();
Expand Down
1 change: 1 addition & 0 deletions src/graphics/program-lib/programs/standard.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ const standard = {
}
return chan;
}
return undefined;
},

_setMapTransform: function (codes, name, id, uv) {
Expand Down
15 changes: 7 additions & 8 deletions src/graphics/reproject-texture.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
FILTER_NEAREST,
TEXTURETYPE_RGBM, TEXTURETYPE_RGBE,
PIXELFORMAT_RGB16F, PIXELFORMAT_RGB32F, PIXELFORMAT_RGBA16F, PIXELFORMAT_RGBA32F,
TEXTUREPROJECTION_OCTAHEDRAL, TEXTUREPROJECTION_EQUIRECT, TEXTUREPROJECTION_CUBE, TEXTUREPROJECTION_NONE
TEXTUREPROJECTION_OCTAHEDRAL, TEXTUREPROJECTION_CUBE
} from './constants.js';
import { Vec3 } from '../math/vec3.js';
import { random } from '../math/random.js';
Expand Down Expand Up @@ -39,14 +39,13 @@ const getCoding = (texture) => {
};

const getProjectionName = (projection) => {
// default to equirect if not specified
if (projection === TEXTUREPROJECTION_NONE) {
projection = TEXTUREPROJECTION_EQUIRECT;
}
switch (projection) {
case TEXTUREPROJECTION_CUBE: return "Cubemap";
case TEXTUREPROJECTION_EQUIRECT: return "Equirect";
case TEXTUREPROJECTION_OCTAHEDRAL: return "Octahedral";
case TEXTUREPROJECTION_CUBE:
return "Cubemap";
case TEXTUREPROJECTION_OCTAHEDRAL:
return "Octahedral";
default: // for anything else, assume equirect
return "Equirect";
}
};

Expand Down
6 changes: 3 additions & 3 deletions src/graphics/texture.js
Original file line number Diff line number Diff line change
Expand Up @@ -974,19 +974,19 @@ class Texture {
const mipSize = this._levels[idx].length;
if (!mipSize) {
Debug.error(`No byte array for mip ${idx}`);
return;
return undefined;
}
fsize += mipSize;
} else {
for (let face = 0; face < 6; face++) {
if (!this._levels[idx][face]) {
Debug.error(`No level data for mip ${idx}, face ${face}`);
return;
return undefined;
}
const mipSize = this._levels[idx][face].length;
if (!mipSize) {
Debug.error(`No byte array for mip ${idx}, face ${face}`);
return;
return undefined;
}
fsize += mipSize;
}
Expand Down
1 change: 1 addition & 0 deletions src/resources/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ class ResourceLoader {
if (this._cache[url + type]) {
return this._cache[url + type];
}
return undefined;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/resources/texture.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class TextureHandler {

open(url, data, asset) {
if (!url)
return;
return undefined;

let texture = this._getParser(url).open(url, data, this._device);

Expand Down
6 changes: 3 additions & 3 deletions src/scene/batching/batch-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ class BatchManager {
}

if (this._batchGroups[id]) {
Debug.error(`batch group with id ${id} already exists`);
return;
Debug.error(`Batch group with id ${id} already exists.`);
return undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it could return this._batchGroups[id] so that the code would still work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, it'd be changing functionality. I'd rather address that separately.

}

const group = new BatchGroup(id, name, dynamic, maxAabbSize, layers);
Expand All @@ -160,7 +160,7 @@ class BatchManager {
*/
removeGroup(id) {
if (!this._batchGroups[id]) {
Debug.error(`batch group with id ${id} doesn't exist`);
Debug.error(`Batch group with id ${id} doesn't exist.`);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/scene/materials/standard-material-options-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ class StandardMaterialOptionsBuilder {

const isOpacity = p === "opacity";
if (isOpacity && stdMat.blendType === BLEND_NONE && stdMat.alphaTest === 0.0 && !stdMat.alphaToCoverage)
return options;
return;

if (!minimalOptions || isOpacity) {
if (p !== "height" && stdMat[vname]) {
Expand Down
1 change: 1 addition & 0 deletions src/vr/vr-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ class VrDisplay extends EventHandler {
*/
getFrameData() {
if (this.display) return this._frameData;
return undefined;
}

/**
Expand Down