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

refactor: enable an eslint rule, @typescript-eslint/strict-boolean-expressions #633

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
"no-console": ["error", { "allow": ["info", "warn", "error"] }], // do not leave console.log
"require-atomic-updates": ["off"], // the rule is kinda buggy
"@typescript-eslint/no-unnecessary-type-assertion": ["error"], // we don't need unnecessary type assertions
"@typescript-eslint/strict-boolean-expressions": ["error", { // ban dangerous boolean expressions
"allowAny": true // Three.js requires so many use of `any` s
}],
"@typescript-eslint/naming-convention": [
"error",
{
Expand Down
11 changes: 3 additions & 8 deletions packages/three-vrm/src/VRM.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,8 @@ export class VRM {
}

if (this.materials) {
this.materials.forEach((material: any) => {
if (material.updateVRMMaterials) {
material.updateVRMMaterials(delta);
}
this.materials.forEach((material) => {
(material as any).updateVRMMaterials?.(delta);
});
}
}
Expand All @@ -152,10 +150,7 @@ export class VRM {
* Dispose everything about the VRM instance.
*/
public dispose(): void {
const scene = this.scene;
if (scene) {
deepDispose(scene);
}
deepDispose(this.scene);

this.meta?.texture?.dispose();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/three-vrm/src/blendshape/VRMBlendShapeGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class VRMBlendShapeGroup extends THREE.Object3D {
// property has not been found
return;
}
value = args.defaultValue || value;
value = args.defaultValue ?? value;

let type: VRMBlendShapeMaterialValueType;
let defaultValue: number | THREE.Vector2 | THREE.Vector3 | THREE.Vector4 | THREE.Color;
Expand Down
6 changes: 3 additions & 3 deletions packages/three-vrm/src/blendshape/VRMBlendShapeImporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ export class VRMBlendShapeImporter {

let presetName: VRMSchema.BlendShapePresetName | undefined;
if (
schemaGroup.presetName &&
schemaGroup.presetName != null &&
schemaGroup.presetName !== VRMSchema.BlendShapePresetName.Unknown &&
!blendShapePresetMap[schemaGroup.presetName]
blendShapePresetMap[schemaGroup.presetName] == null
) {
presetName = schemaGroup.presetName;
blendShapePresetMap[schemaGroup.presetName] = name;
Expand All @@ -56,7 +56,7 @@ export class VRMBlendShapeImporter {
const group = new VRMBlendShapeGroup(name);
gltf.scene.add(group);

group.isBinary = schemaGroup.isBinary || false;
group.isBinary = schemaGroup.isBinary ?? false;

if (schemaGroup.binds) {
schemaGroup.binds.forEach(async (bind) => {
Expand Down
8 changes: 5 additions & 3 deletions packages/three-vrm/src/blendshape/VRMBlendShapeProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ export class VRMBlendShapeProxy {
*/
public getBlendShapeGroup(name: string | VRMSchema.BlendShapePresetName): VRMBlendShapeGroup | undefined {
const presetName = this._blendShapePresetMap[name as VRMSchema.BlendShapePresetName];
const controller = presetName ? this._blendShapeGroups[presetName] : this._blendShapeGroups[name];
if (!controller) {
const controller = (presetName != null ? this._blendShapeGroups[presetName] : this._blendShapeGroups[name]) as
| VRMBlendShapeGroup
| undefined;
if (controller == null) {
console.warn(`no blend shape found by ${name}`);
return undefined;
}
Expand All @@ -73,7 +75,7 @@ export class VRMBlendShapeProxy {
controller: VRMBlendShapeGroup,
): void {
this._blendShapeGroups[name] = controller;
if (presetName) {
if (presetName != null) {
this._blendShapePresetMap[presetName] = name;
} else {
this._unknownGroupNames.push(name);
Expand Down
4 changes: 2 additions & 2 deletions packages/three-vrm/src/debug/VRMDebug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ export class VRMDebug extends VRM {
super(params);

// Gizmoを展開
if (!debugOption.disableBoxHelper) {
if (!(debugOption.disableBoxHelper ?? false)) {
this.scene.add(new THREE.BoxHelper(this.scene));
}

if (!debugOption.disableSkeletonHelper) {
if (!(debugOption.disableSkeletonHelper ?? false)) {
this.scene.add(new THREE.SkeletonHelper(this.scene));
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/three-vrm/src/debug/VRMLookAtHeadDebug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export class VRMLookAtHeadDebug extends VRMLookAtHead {
private _faceDirectionHelper?: THREE.ArrowHelper;

public setupHelper(scene: THREE.Object3D, debugOption: VRMDebugOptions): void {
if (!debugOption.disableFaceDirectionHelper) {
if (!(debugOption.disableFaceDirectionHelper ?? false)) {
this._faceDirectionHelper = new THREE.ArrowHelper(
new THREE.Vector3(0, 0, -1),
new THREE.Vector3(0, 0, 0),
Expand Down
2 changes: 1 addition & 1 deletion packages/three-vrm/src/debug/VRMSpringBoneManagerDebug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export type VRMSpringBoneGroupDebug = VRMSpringBoneDebug[];

export class VRMSpringBoneManagerDebug extends VRMSpringBoneManager {
public setupHelper(scene: THREE.Object3D, debugOption: VRMDebugOptions): void {
if (debugOption.disableSpringBoneHelper) return;
if (debugOption.disableSpringBoneHelper ?? false) return;

this.springBoneGroupList.forEach((springBoneGroup) => {
springBoneGroup.forEach((springBone) => {
Expand Down
5 changes: 2 additions & 3 deletions packages/three-vrm/src/firstperson/VRMFirstPerson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,8 @@ export class VRMFirstPerson {
geometry.setIndex(newTriangle);

// mtoon material includes onBeforeRender. this is unsupported at SkinnedMesh#clone
if (src.onBeforeRender) {
dst.onBeforeRender = src.onBeforeRender;
}
dst.onBeforeRender = src.onBeforeRender;

dst.bind(new THREE.Skeleton(src.skeleton.bones, src.skeleton.boneInverses), new THREE.Matrix4());
return dst;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/three-vrm/src/humanoid/VRMHumanoid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class VRMHumanoid {
public getPose(): VRMPose {
const pose: VRMPose = {};
Object.keys(this.humanBones).forEach((vrmBoneName) => {
const node = this.getBoneNode(vrmBoneName as VRMSchema.HumanoidBoneName)!;
const node = this.getBoneNode(vrmBoneName as VRMSchema.HumanoidBoneName);

// Ignore when there are no bone on the VRMHumanoid
if (!node) {
Expand Down Expand Up @@ -143,7 +143,7 @@ export class VRMHumanoid {
* @param name Name of the bone you want
*/
public getBone(name: VRMSchema.HumanoidBoneName): VRMHumanBone | undefined {
return this.humanBones[name][0] || undefined;
return this.humanBones[name][0] ?? undefined;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/three-vrm/src/humanoid/VRMHumanoidImporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class VRMHumanoidImporter {
if (schemaHumanoid.humanBones) {
await Promise.all(
schemaHumanoid.humanBones.map(async (bone) => {
if (!bone.bone || bone.node == null) {
if (bone.bone == null || bone.node == null) {
return;
}

Expand Down
8 changes: 4 additions & 4 deletions packages/three-vrm/src/material/MToonMaterial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export class MToonMaterial extends THREE.ShaderMaterial {
constructor(parameters: MToonParameters = {}) {
super();

this.encoding = parameters.encoding || THREE.LinearEncoding;
this.encoding = parameters.encoding ?? THREE.LinearEncoding;
if (this.encoding !== THREE.LinearEncoding && this.encoding !== THREE.sRGBEncoding) {
console.warn(
'The specified color encoding does not work properly with MToonMaterial. You might want to use THREE.sRGBEncoding instead.',
Expand Down Expand Up @@ -231,9 +231,9 @@ export class MToonMaterial extends THREE.ShaderMaterial {
parameters.lights = true;
parameters.clipping = true;

parameters.skinning = parameters.skinning || false;
parameters.morphTargets = parameters.morphTargets || false;
parameters.morphNormals = parameters.morphNormals || false;
parameters.skinning = parameters.skinning ?? false;
parameters.morphTargets = parameters.morphTargets ?? false;
parameters.morphNormals = parameters.morphNormals ?? false;

// == uniforms =============================================================
parameters.uniforms = THREE.UniformsUtils.merge([
Expand Down
26 changes: 9 additions & 17 deletions packages/three-vrm/src/material/VRMMaterialImporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class VRMMaterialImporter {
* @param options Options of the VRMMaterialImporter
*/
constructor(options: VRMMaterialImporterOptions = {}) {
this._encoding = options.encoding || THREE.LinearEncoding;
this._encoding = options.encoding ?? THREE.LinearEncoding;
if (this._encoding !== THREE.LinearEncoding && this._encoding !== THREE.sRGBEncoding) {
console.warn(
'The specified color encoding might not work properly with VRMMaterialImporter. You might want to use THREE.sRGBEncoding instead.',
Expand All @@ -68,7 +68,7 @@ export class VRMMaterialImporter {
}

const nodePrimitivesMap = await gltfExtractPrimitivesFromNodes(gltf);
const materialList: { [vrmMaterialIndex: number]: { surface: THREE.Material; outline?: THREE.Material } } = {};
const materialIndexMaterialsMap = new Map<number, { surface: THREE.Material; outline?: THREE.Material }>();
const materials: THREE.Material[] = []; // result

await Promise.all(
Expand All @@ -80,15 +80,6 @@ export class VRMMaterialImporter {
primitives.map(async (primitive, primitiveIndex) => {
const schemaPrimitive = schemaMesh.primitives[primitiveIndex];

// some glTF might have both `node.mesh` and `node.children` at once
// and GLTFLoader handles both mesh primitives and "children" in glTF as "children" in THREE
// It seems GLTFLoader handles primitives first then handles "children" in glTF (it's lucky!)
// so we should ignore (primitives.length)th and following children of `mesh.children`
// TODO: sanitize this after GLTFLoader plugin system gets introduced : https://github.com/mrdoob/three.js/pull/18421
if (!schemaPrimitive) {
return;
}

const primitiveGeometry = primitive.geometry;
const primitiveVertices = primitiveGeometry.index
? primitiveGeometry.index.count
Expand All @@ -103,20 +94,21 @@ export class VRMMaterialImporter {
// create / push to cache (or pop from cache) vrm materials
const vrmMaterialIndex = schemaPrimitive.material!;

let props = materialProperties[vrmMaterialIndex];
if (!props) {
let props = materialProperties[vrmMaterialIndex] as VRMSchema.Material | undefined;
if (props == null) {
console.warn(
`VRMMaterialImporter: There are no material definition for material #${vrmMaterialIndex} on VRM extension.`,
);
props = { shader: 'VRM_USE_GLTFSHADER' }; // fallback
}

let vrmMaterials: { surface: THREE.Material; outline?: THREE.Material };
if (materialList[vrmMaterialIndex]) {
vrmMaterials = materialList[vrmMaterialIndex];
const vrmMaterialsAlreadyLoaded = materialIndexMaterialsMap.get(vrmMaterialIndex);
if (vrmMaterialsAlreadyLoaded != null) {
vrmMaterials = vrmMaterialsAlreadyLoaded;
} else {
vrmMaterials = await this.createVRMMaterials(primitive.material[0], props, gltf);
materialList[vrmMaterialIndex] = vrmMaterials;
materialIndexMaterialsMap.set(vrmMaterialIndex, vrmMaterials);

materials.push(vrmMaterials.surface);
if (vrmMaterials.outline) {
Expand All @@ -136,7 +128,7 @@ export class VRMMaterialImporter {
}

// render order
primitive.renderOrder = props.renderQueue || 2000;
primitive.renderOrder = props.renderQueue ?? 2000;

// outline ("2 pass shading using groups" trick here)
if (vrmMaterials.outline) {
Expand Down
6 changes: 3 additions & 3 deletions packages/three-vrm/src/material/VRMUnlitMaterial.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ export class VRMUnlitMaterial extends THREE.ShaderMaterial {
parameters.fog = true;
parameters.clipping = true;

parameters.skinning = parameters.skinning || false;
parameters.morphTargets = parameters.morphTargets || false;
parameters.morphNormals = parameters.morphNormals || false;
parameters.skinning = parameters.skinning ?? false;
parameters.morphTargets = parameters.morphTargets ?? false;
parameters.morphNormals = parameters.morphNormals ?? false;

// == uniforms =============================================================
parameters.uniforms = THREE.UniformsUtils.merge([
Expand Down
5 changes: 3 additions & 2 deletions packages/three-vrm/src/springbone/VRMSpringBoneImporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,11 @@ export class VRMSpringBoneImporter {
await Promise.all(
vrmBoneGroup.bones.map(async (nodeIndex) => {
// VRMの情報から「揺れモノ」ボーンのルートが取れる
const springRootBone: GLTFNode = await gltf.parser.getDependency('node', nodeIndex);
const springRootBone: GLTFNode | null = await gltf.parser.getDependency('node', nodeIndex);

const centerIndex = vrmBoneGroup.center === -1 ? null : vrmBoneGroup.center;
const center: GLTFNode =
vrmBoneGroup.center! !== -1 ? await gltf.parser.getDependency('node', vrmBoneGroup.center!) : null;
centerIndex != null ? await gltf.parser.getDependency('node', vrmBoneGroup.center!) : null;

// it's weird but there might be cases we can't find the root bone
if (!springRootBone) {
Expand Down
7 changes: 3 additions & 4 deletions packages/three-vrm/src/utils/disposer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
import * as THREE from 'three';

function disposeMaterial(material: THREE.Material): void {
Object.keys(material).forEach((propertyName) => {
const value = (material as any)[propertyName];
Object.values(material).forEach((value) => {
if (value?.isTexture) {
const texture = value as THREE.Texture;
texture.dispose();
Expand All @@ -20,11 +19,11 @@ function dispose(object3D: THREE.Object3D): void {
geometry.dispose();
}

const material: THREE.Material | THREE.Material[] = (object3D as any).material;
const material: THREE.Material | THREE.Material[] | undefined = (object3D as any).material;
if (material) {
if (Array.isArray(material)) {
material.forEach((material: THREE.Material) => disposeMaterial(material));
} else if (material) {
} else {
disposeMaterial(material);
}
}
Expand Down