From 181dd066ed52f2b65615866578cd042bdeae7d8b Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Wed, 12 Aug 2026 17:21:54 +0100 Subject: [PATCH] Fix StandardMaterial property types in the generated .d.ts StandardMaterial defines its accessors dynamically, so tsc emits none of them. They are declared by hand from STANDARD_MAT_PROPS in the types fixup plugin, and each doc comment was looked up with contents.match(`@property {${type}} ${name}`) - passing the type through as a regex. That one line caused three distinct defects: - When the type in the list disagreed with the type in the class JSDoc, the lookup silently found nothing, so the accessor was emitted with no doc comment and with the (wrong) type from the list. occludeDirect was typed `number` despite defaulting to `false`, so `material.occludeDirect = true` needed a @ts-ignore from TypeScript. alphaFade had the same problem in reverse, typed `boolean` despite being _defineFloat('alphaFade', 1). - `Texture|null` was treated as a regex alternation, matching "@property {Texture" at its first occurrence in the file and then slicing from the wrong offset. Every texture property inherited a mangled fragment of diffuseMap's description, e.g. anisotropyMap documented as "e main (primary) diffuse map of the material (default is null)." - "@property {number} anisotropy" prefix-matched anisotropyIntensity, so the deprecated alias inherited its neighbour's documentation. Fixes: - occludeDirect is now `boolean` and alphaFade `number`, matching their runtime defaults. - opacityShadowDither's JSDoc said `boolean` for what is a dither mode string (DITHER_NONE etc.); the emitted declaration was already correct, so the JSDoc is corrected to `string`. - The doc lookup now parses the @property block into a map keyed by property name, rather than building a regex out of the type. - The build throws on a type disagreement or a missing @property tag, so this cannot silently regress into a dropped doc comment again. - Added the missing @property tag for sheenVertexColorChannel, and an explicit @deprecated doc for the anisotropy alias. Every StandardMaterial accessor now emits with its own correct doc comment. Fixes #9152 Co-Authored-By: Claude Opus 5 --- src/scene/materials/standard-material.js | 4 +- utils/plugins/rollup-types-fixup.mjs | 82 ++++++++++++++++++------ 2 files changed, 64 insertions(+), 22 deletions(-) diff --git a/src/scene/materials/standard-material.js b/src/scene/materials/standard-material.js index c8f725e914c..a2c11152eb7 100644 --- a/src/scene/materials/standard-material.js +++ b/src/scene/materials/standard-material.js @@ -338,6 +338,8 @@ const isBlack = (color) => { * "g", "b", "a", "rgb" or any swizzled combination. * @property {boolean} sheenVertexColor Use mesh vertex colors for sheen. If sheen map or * sheen tint are set, they'll be multiplied by vertex colors. + * @property {string} sheenVertexColorChannel Vertex color channels to use for sheen. Can be "r", + * "g", "b", "a", "rgb" or any swizzled combination. * @property {number} sheenGloss The glossiness of the sheen (fabric) microfiber structure. * This color value is a single value between 0 and 1. * @property {boolean} sheenGlossInvert Invert the sheen gloss component (default is false). @@ -389,7 +391,7 @@ const isBlack = (color) => { * - {@link DITHER_IGNNOISE}: Opacity is dithered using an interleaved gradient noise. * * Defaults to {@link DITHER_NONE}. - * @property {boolean} opacityShadowDither Used to specify whether shadow opacity is dithered, which + * @property {string} opacityShadowDither Used to specify whether shadow opacity is dithered, which * allows shadow transparency without alpha blending. Can be: * * - {@link DITHER_NONE}: Opacity dithering is disabled. diff --git a/utils/plugins/rollup-types-fixup.mjs b/utils/plugins/rollup-types-fixup.mjs index 31370f58d0f..e376d0ece2f 100644 --- a/utils/plugins/rollup-types-fixup.mjs +++ b/utils/plugins/rollup-types-fixup.mjs @@ -7,10 +7,15 @@ const REGULAR_OUT = '\x1b[22m'; const TYPES_PATH = './build/playcanvas/src'; +// StandardMaterial defines its accessors dynamically, so TypeScript emits none of them. They are +// declared by hand from this list, and their doc comments are taken from the matching +// `@property {type} name` tag in the class JSDoc - so the type here must match the type documented +// there, or the build fails. An optional third element supplies the doc comment for properties that +// have no `@property` tag (deprecated aliases, which are defined in src/deprecated/deprecated.js). const STANDARD_MAT_PROPS = [ - ['alphaFade', 'boolean'], + ['alphaFade', 'number'], ['ambient', 'Color'], - ['anisotropy', 'number'], + ['anisotropy', 'number', 'Defines amount of anisotropy. @deprecated Use {@link StandardMaterial#anisotropyIntensity} and {@link StandardMaterial#anisotropyRotation} instead.'], ['anisotropyIntensity', 'number'], ['anisotropyRotation', 'number'], ['anisotropyMap', 'Texture|null'], @@ -137,7 +142,7 @@ const STANDARD_MAT_PROPS = [ ['normalMapRotation', 'number'], ['normalMapTiling', 'Vec2'], ['normalMapUv', 'number'], - ['occludeDirect', 'number'], + ['occludeDirect', 'boolean'], ['occludeSpecular', 'number'], ['occludeSpecularIntensity', 'number'], ['opacity', 'number'], @@ -194,31 +199,66 @@ const STANDARD_MAT_PROPS = [ ['useSkybox', 'boolean'] ]; +/** + * Parses the `@property` tags of the StandardMaterial class JSDoc block. + * + * @param {string} contents - The contents of the generated standard-material.d.ts. + * @returns {Map} The documented properties, keyed by + * property name. + */ +const parseProperties = (contents) => { + const properties = new Map(); + + // Only consider the class JSDoc block, and split it on tag boundaries so that multi-line + // descriptions stay attached to the tag they belong to + const block = contents.slice(0, contents.indexOf('export class StandardMaterial')); + for (const tag of block.split('\n * @')) { + const match = /^property \{(.+?)\} (\w+)\s([\s\S]*)$/.exec(tag); + if (match) { + const [, type, name, description] = match; + properties.set(name, { + type, + description: description + .replace(/[\n\t*]/g, ' ') // remove newlines, tabs, and asterisks + .replace(/\s+/g, ' ') // collapse whitespace + .trim() + }); + } + } + + return properties; +}; + const REPLACEMENTS = [{ path: `${TYPES_PATH}/scene/materials/standard-material.d.ts`, replacement: { - guard: 'set alphaFade(arg: boolean);', + guard: 'set alphaFade(arg:', transformer: (contents) => { + const properties = parseProperties(contents); + const errors = []; - // Find the jsdoc block description using eg "@property {Type} {name}" - return contents.replace('reset(): void;', `reset(): void; - ${STANDARD_MAT_PROPS.map((prop) => { - const typeDefinition = `@property {${prop[1]}} ${prop[0]}`; - const typeDescriptionIndex = contents.match(typeDefinition); - const typeDescription = typeDescriptionIndex ? - contents.slice(typeDescriptionIndex.index + typeDefinition.length, contents.indexOf('\n * @property', typeDescriptionIndex.index + typeDefinition.length)) : - ''; + const accessors = STANDARD_MAT_PROPS.map(([name, type, doc]) => { + let description = doc; + if (description === undefined) { + const property = properties.get(name); + if (!property) { + errors.push(`${name}: declared as '${type}' but has no @property tag`); + } else if (property.type !== type) { + errors.push(`${name}: declared as '${type}' but documented as '${property.type}'`); + } + description = property?.description ?? ''; + } - // Strip newlines, asterisks, and tabs from the type description - const cleanTypeDescription = typeDescription - .trim() - .replace(/[\n\t*]/g, ' ') // remove newlines, tabs, and asterisks - .replace(/\s+/g, ' '); // collapse whitespace + const jsdoc = description ? `/** ${description} */` : ''; + return `\t${jsdoc}\n\tset ${name}(arg: ${type});\n\tget ${name}(): ${type};\n\n`; + }).join(''); - const jsdoc = cleanTypeDescription ? `/** ${cleanTypeDescription} */` : ''; - return `\t${jsdoc}\n\tset ${prop[0]}(arg: ${prop[1]});\n\tget ${prop[0]}(): ${prop[1]};\n\n`; - }).join('')}` - ); + if (errors.length) { + throw new Error(`StandardMaterial types disagree with its JSDoc - fix the @property tag in src/scene/materials/standard-material.js or the type in STANDARD_MAT_PROPS:\n ${errors.join('\n ')}`); + } + + return contents.replace('reset(): void;', `reset(): void; + ${accessors}`); }, footer: ` import { Color } from '../../core/math/color.js';