diff --git a/package-lock.json b/package-lock.json index a1d3dc6bd..1eb6cfb20 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "2.3.1", "license": "MIT", "dependencies": { - "@playcanvas/attribute-parser": "^1.10.0" + "@playcanvas/attribute-parser": "^1.10.1" }, "devDependencies": { "@jsquash/avif": "1.3.0", @@ -586,9 +586,9 @@ } }, "node_modules/@playcanvas/attribute-parser": { - "version": "1.10.0", - "resolved": "https://registry.npmjs.org/@playcanvas/attribute-parser/-/attribute-parser-1.10.0.tgz", - "integrity": "sha512-K5qffgp9eEgYL1nWvzZKVzxm7nwvbL6hHQJc2T8q/NExhJlKDwr7cIfol98C7HQZ6ay+oKUg+CCiD0WnQ4pexg==", + "version": "1.10.1", + "resolved": "https://registry.npmjs.org/@playcanvas/attribute-parser/-/attribute-parser-1.10.1.tgz", + "integrity": "sha512-lbjqE53mZWdpOHgGul8uaYacnXzrtIaWYDu+qIZzFEzgASakELcmSQSwQesM93qJXr+IrARVlDYr/Bc8iSqzmw==", "dependencies": { "@playcanvas/eslint-config": "^2.0.0", "@typescript/vfs": "^1.6.0", diff --git a/package.json b/package.json index 635b017e6..0d52994bb 100644 --- a/package.json +++ b/package.json @@ -89,6 +89,6 @@ "typescript": "5.6.3" }, "dependencies": { - "@playcanvas/attribute-parser": "^1.10.0" + "@playcanvas/attribute-parser": "^1.10.1" } } diff --git a/sass/editor/_editor-asset-script-inspector.scss b/sass/editor/_editor-asset-script-inspector.scss index 44c218670..e8609f4ca 100644 --- a/sass/editor/_editor-asset-script-inspector.scss +++ b/sass/editor/_editor-asset-script-inspector.scss @@ -1,3 +1,11 @@ +@use 'sass:color'; + +$bright-red: #e74c3c; +$bright-yellow: #f1c40f; +$status-info: #b1b8ba; +$status-error: $bright-red; +$status-warn: $bright-yellow; + .script-asset-inspector-container, .script-asset-inspector-error-container { display: flex; @@ -6,6 +14,49 @@ margin: 6px; } +.script-asset-inspector-attribute-error-container { + display: flex; + flex-direction: column; + border: none; + margin: 0; + background-color: #2c393c; + +} + +.pcui-label { + &.clickable-error { + cursor: pointer; + font-size: 11px; + + &:hover { + text-decoration: underline; + } + } + + &.pcui-error { // For simple errors + font-size: 11px; + } +} + +.script-asset-inspector-script.pcui-error.clickable-error { + cursor: pointer; + + &:hover { + text-decoration: underline; + } +} + +.script-asset-inspector-attribute-error-title, .script-asset-inspector-attribute-warning-title { + font-family: inconsolatamedium, Monaco, Menlo, "Ubuntu Mono", Consolas, source-code-pro, monospace; + font-weight: normal; + font-size: 12px; + cursor: pointer; +} + +.script-asset-inspector-attribute-error-title:hover, .script-asset-inspector-attribute-warning-title:hover { + text-decoration: underline; +} + .script-asset-inspector-error-container > .pcui-label { white-space: normal; } @@ -28,6 +79,7 @@ white-space: normal; } + .script-asset-inspector-attribute { flex-grow: 1; color: #b1b8ba; @@ -43,3 +95,48 @@ .script-asset-inspector-attribute:hover { background-color: #313e41; } + +.script-asset-inspector-warning { + color: $status-warn; +} + +.script-asset-inspector-attribute-warning { + &::before { + @extend .font-icon; + + content: '\E218'; + margin-right: 6px; + } +} + +.script-asset-inspector-attribute-warning.clickable-warning { + cursor: pointer; + + &:hover { + text-decoration: underline; + } +} + +.script-asset-inspector-attribute-error { + &::before { + @extend .font-icon; + + content: '\E368'; + margin-right: 6px; + } +} + +// Tooltip warnings styling +.warnings-title.script-asset-inspector-warning { + // Normal text color for the header +} + +.warning-item.script-asset-inspector-warning { + color: var(--pcui-warning-color); + font-size: 11px; +} + +.open-console { + text-decoration: underline; + cursor: pointer; +} \ No newline at end of file diff --git a/src/code-editor/editor.ts b/src/code-editor/editor.ts index f2fa16b6c..d508f78ea 100644 --- a/src/code-editor/editor.ts +++ b/src/code-editor/editor.ts @@ -13,9 +13,10 @@ class CodeEditor extends Editor { this.once('loaded', () => { this.emit('start'); - // Send an event to the opener - const openerOrigin = new URL(document.referrer).origin; - window.opener.postMessage('start', openerOrigin); + if (window.opener) { + const openerOrigin = new URL(document.referrer).origin; + window.opener.postMessage('start', openerOrigin); + } // if there is a merge in progress for our branch if (config.self.branch.merge && !config.self.branch.merge.conflict) { diff --git a/src/editor/assets/handle-script-parse.ts b/src/editor/assets/handle-script-parse.ts index 42bcf8390..e61820687 100644 --- a/src/editor/assets/handle-script-parse.ts +++ b/src/editor/assets/handle-script-parse.ts @@ -64,9 +64,6 @@ editor.once('load', () => { const errors = checkForErrors(res); if (errors) { editor.call('status:error', `There was an error while parsing script asset '${asset.get('name')}'`); - errors.forEach((error) => { - console.error('Error parsing script asset', error); - }); callback?.(null, res); return; } diff --git a/src/editor/console/console.ts b/src/editor/console/console.ts index 78559b4bc..179952bcf 100644 --- a/src/editor/console/console.ts +++ b/src/editor/console/console.ts @@ -109,7 +109,6 @@ editor.on('load', () => { return; } await addLog('error', msg, verboseMsg, onclick); - editor.call('status:error', msg); }); editor.method('console:history', () => { pendingHistory = true; diff --git a/src/editor/inspector/assets/script.ts b/src/editor/inspector/assets/script.ts index 044a612df..52c82d44f 100644 --- a/src/editor/inspector/assets/script.ts +++ b/src/editor/inspector/assets/script.ts @@ -1,13 +1,15 @@ import { Container, Label, Panel, Button } from '@playcanvas/pcui'; import { CLASS_ERROR } from '../../../common/pcui/constants.ts'; -import { tooltip, tooltipRefItem } from '../../../common/tooltips.ts'; +import { tooltip } from '../../../common/tooltips.ts'; const CLASS_ROOT = 'script-asset-inspector'; const CLASS_ERROR_CONTAINER = `${CLASS_ROOT}-error-container`; const CLASS_CONTAINER = `${CLASS_ROOT}-container`; const CLASS_SCRIPT = `${CLASS_ROOT}-script`; +const CLASS_WARNING = `${CLASS_ROOT}-warning`; const CLASS_ATTRIBUTE = `${CLASS_ROOT}-attribute`; +const CLASS_ATTRIBUTE_ERROR_CONTAINER = `${CLASS_ROOT}-attribute-error-container`; const DOM = parent => [ { @@ -42,42 +44,188 @@ class ScriptAssetInspector extends Panel { this.header.append(this._parseButton); } - _displayScriptAttributes() { + _displayScriptAttributes(scripts) { this._scriptAttributeContainer = new Container({ class: CLASS_CONTAINER }); - const scripts = this._asset.get('data.scripts'); let hasScripts = false; + + // Iterate over the scripts Object.keys(scripts).forEach((scriptName) => { hasScripts = true; this._scriptAttributeContainer[`_${scriptName}Container`] = new Container({ flex: true }); - this._scriptAttributeContainer[`_${scriptName}Container`]._scriptLabel = new Label({ text: scriptName, class: CLASS_SCRIPT }); + + const scriptData = scripts[scriptName]; + + // Get error and warning attributes for this script + const scriptErrors = scriptData.attributesInvalid ? + scriptData.attributesInvalid.filter(error => (error.severity ? error.severity === 8 : true)) : + []; + const scriptWarnings = scriptData.attributesInvalid ? + scriptData.attributesInvalid.filter(error => error.severity === 4) : + []; + + this._scriptAttributeContainer[`_${scriptName}Container`]._scriptLabel = new Label({ + text: scriptName, + class: [CLASS_SCRIPT] + }); + this._scriptAttributeContainer[`_${scriptName}Container`].append(this._scriptAttributeContainer[`_${scriptName}Container`]._scriptLabel); const hasCollision = editor.call('assets:scripts:collide', scriptName); if (hasCollision) { this._scriptAttributeContainer[`_${scriptName}Container`].append(new Label({ text: `script ${scriptName} is already defined in other asset`, class: [CLASS_SCRIPT, CLASS_ERROR] })); } - const attributes = this._asset.get('data.scripts')[scriptName]; + + const attributes = scriptData.attributes; this._tooltips.forEach(tooltip => tooltip.destroy()); this._tooltips = []; - attributes.attributesOrder.forEach((attributeName) => { - const attributeLabel = new Label({ text: attributeName, class: CLASS_ATTRIBUTE }); - const attributeData = attributes.attributes[attributeName]; - const item = tooltipRefItem({ - reference: { - title: attributeName, - subTitle: editor.call('assets:scripts:typeToSubTitle', attributeData), - description: (attributeData.description || attributeData.title || ''), - code: JSON.stringify(attributeData, null, 4) + const errorAttributeNames = scriptErrors.map(error => error.name); + const warningAttributeNames = scriptWarnings.map(warning => warning.name); + + // If there are invalid errors, show them in red below the script header + if (scriptErrors.length > 0) { + const errorContainer = new Container({ class: CLASS_ATTRIBUTE_ERROR_CONTAINER, flex: true }); + + // Always show the error header with icon + const errorHeader = new Label({ + class: [CLASS_ERROR, CLASS_SCRIPT, 'script-asset-inspector-attribute-error'], + text: 'This script contains invalid attributes:' + }); + errorContainer.append(errorHeader); + + // List all errors in the container + scriptErrors.forEach((error) => { + if (error.severity) { + // Rich error - show first sentence with line/column + const fileName = error.fileName || this._asset.get('name') || 'unknown'; + const location = `${fileName}:${error.startLineNumber}:${error.startColumn}`; + const firstSentence = error.message.split('.')[0]; + + const errorText = new Label({ + class: [CLASS_ERROR, 'clickable-error'], + text: `${location} - ${firstSentence}` + }); + + // Add click handler for rich errors + errorText.dom.addEventListener('click', () => { + editor.call('picker:codeeditor', this._asset, { + line: error.startLineNumber, + col: error.startColumn, + error: true + }); + }); + + errorContainer.append(errorText); + + // Log to console for rich errors + editor.call('console:error', `${location} - (${error.name}) ${error.message}`, () => { + editor.call('picker:codeeditor', this._asset, { + line: error.startLineNumber, + col: error.startColumn, + error: true + }); + }); + } else { + // Simple error - show as is + const errorText = new Label({ + class: [CLASS_ERROR], + text: error + }); + errorContainer.append(errorText); + + // Log to console for simple errors + const fileName = this._asset.get('name') || 'unknown'; + // editor.call('console:error', `${fileName} - ${error}`); } }); + + editor.call('status:error', `There was an error while parsing script asset '${this._asset.get('name')}'`); + this._scriptAttributeContainer[`_${scriptName}Container`].append(errorContainer); + } + + scriptData.attributesOrder.forEach((attributeName) => { + // Skip error attributes - they should not appear in the list + if (errorAttributeNames.includes(attributeName)) { + return; + } + + // Check if this attribute has a warning + const hasWarning = warningAttributeNames.includes(attributeName); + const attributeClasses = hasWarning ? [CLASS_ATTRIBUTE, CLASS_WARNING] : [CLASS_ATTRIBUTE]; + + const attributeLabel = new Label({ + text: attributeName, + class: attributeClasses + }); + + // Add click handler for warning attributes + if (hasWarning) { + const warning = scriptWarnings.find(w => w.name === attributeName); + if (warning) { + attributeLabel.class.add('clickable-warning'); + attributeLabel.dom.addEventListener('click', () => { + editor.call('picker:codeeditor', this._asset, { + line: warning.startLineNumber, + col: warning.startColumn + }); + }); + } + } + + const attributeData = attributes[attributeName]; + + const warningsForThisAttribute = scriptWarnings + .filter(w => w.name === attributeName) + .map(w => w.message); + + // Create tooltip content with reference info and warnings + const tooltipContainer = new Container({ + class: ['tooltip-reference'], + flex: true + }); + + // Add reference information + tooltipContainer.append(new Label({ + class: 'title', + text: attributeName + })); + tooltipContainer.append(new Label({ + class: 'subtitle', + text: editor.call('assets:scripts:typeToSubTitle', attributeData) + })); + tooltipContainer.append(new Label({ + class: 'desc', + text: (attributeData.description || attributeData.title || '') + })); + tooltipContainer.append(new Label({ + class: 'code', + text: JSON.stringify(attributeData, null, 4), + hidden: !attributeData + })); + + // Add warnings section if there are any + if (warningsForThisAttribute.length > 0) { + const warningsTitle = new Label({ + class: ['warnings-title', 'script-asset-inspector-warning'], + text: 'Warnings' + }); + tooltipContainer.append(warningsTitle); + + warningsForThisAttribute.forEach((warningText: string) => { + tooltipContainer.append(new Label({ + class: ['warning-item', 'script-asset-inspector-warning'], + text: warningText + })); + }); + } + tooltip().attach({ - container: item, + container: tooltipContainer, target: attributeLabel, horzAlignEl: this }); - this._tooltips.push(item); + this._tooltips.push(tooltipContainer); this._scriptAttributeContainer[`_${scriptName}Container`].append(attributeLabel); }); @@ -97,7 +245,6 @@ class ScriptAssetInspector extends Panel { this._scriptAttributeContainer.destroy(); } - this._displayScriptAttributes(); this._parseButton.disabled = false; if (error) { this._errorContainer.hidden = false; @@ -105,30 +252,67 @@ class ScriptAssetInspector extends Panel { return; } if (result.scriptsInvalid.length > 0) { - this._errorContainer.append(new Label({ text: 'Validation Errors: ', class: [CLASS_SCRIPT, CLASS_ERROR] })); + this._errorContainer.append(new Label({ text: 'This Script contains errors', class: [CLASS_SCRIPT, CLASS_ERROR] })); result.scriptsInvalid.forEach((invalidScript) => { - this._errorContainer.append(new Label({ text: invalidScript, class: [CLASS_SCRIPT, CLASS_ERROR] })); + if (typeof invalidScript === 'string') { + // Simple string error + this._errorContainer.append(new Label({ text: invalidScript, class: [CLASS_SCRIPT, CLASS_ERROR] })); + } else { + // Rich error object with file, line, column, message + const fileName = invalidScript.file || this._asset.get('name') || 'unknown'; + const location = `${fileName}:${invalidScript.line}:${invalidScript.column}`; + const errorText = `${location} - ${invalidScript.message}`; + + const errorLabel = new Label({ + text: errorText, + class: [CLASS_SCRIPT, CLASS_ERROR, 'clickable-error'] + }); + + // Add click handler to open code editor + errorLabel.dom.addEventListener('click', () => { + editor.call('picker:codeeditor', this._asset, { + line: invalidScript.line, + col: invalidScript.column + }); + }); + + this._errorContainer.append(errorLabel); + } }); this._errorContainer.hidden = false; return; } + + // Process attribute validation issues before displaying attributes for (const scriptName in result.scripts) { const attrInvalid = result.scripts[scriptName].attributesInvalid; - if (attrInvalid.length > 0) { - const label = new Label({ text: attrInvalid, class: [CLASS_ERROR, CLASS_SCRIPT] }); - const container = this._scriptAttributeContainer[`_${scriptName}Container`]; - if (container) { - container.appendAfter(label, container._scriptLabel); - } - } + + const warnings = attrInvalid.filter(error => error.severity === 4); + + // Log warnings to console with click-through to code editor at the warning location + warnings.forEach((warning) => { + const fileName = warning.fileName || this._asset.get('name') || 'unknown'; + const location = `${fileName}:${warning.startLineNumber}:${warning.startColumn}`; + editor.call('console:warn', `${location} - (${warning.name}) ${warning.message}`, () => { + editor.call('picker:codeeditor', this._asset, { + line: warning.startLineNumber, + col: warning.startColumn + }); + }); + }); + + // Do not add global error entries; errors are shown inline under each script title } + + // Now display the script attributes (warnings will be shown in yellow) + this._displayScriptAttributes(result.scripts); }); } link(assets) { this.unlink(); this._asset = assets[0]; - this._displayScriptAttributes(); + this._displayScriptAttributes(this._asset.get('data.scripts')); this._errorContainer.hidden = true; } diff --git a/src/editor/toolbar/toolbar-code-editor.ts b/src/editor/toolbar/toolbar-code-editor.ts index 013031d90..dddea6af7 100644 --- a/src/editor/toolbar/toolbar-code-editor.ts +++ b/src/editor/toolbar/toolbar-code-editor.ts @@ -17,10 +17,11 @@ editor.once('load', () => { editor.call('picker:codeeditor'); }); - editor.method('picker:codeeditor', (asset) => { + editor.method('picker:codeeditor', (asset, options) => { // open the new code editor - try to focus existing tab if it exists - let url = `/editor/code/${config.project.id}`; + const projectId = (config as any).project?.id; + let url = `/editor/code/${projectId}`; const query = []; const params = new URLSearchParams(location.search); @@ -43,15 +44,22 @@ editor.once('load', () => { url += `?${query.join('&')}`; } - const name = `codeeditor:${config.project.id}`; + const name = `codeeditor:${projectId}`; const wnd = window.open('', name); try { if (wnd.editor && wnd.editor.isCodeEditor) { if (asset) { - wnd.editor.call('integration:selectWhenReady', asset.get('id')); + wnd.editor.call('integration:selectWhenReady', asset.get('id'), options || {}); } } else { + const onCodeEditorReady = (event) => { + if (event.data === 'start') { + wnd.editor.call('integration:selectWhenReady', asset.get('id'), options || {}); + window.removeEventListener('message', onCodeEditorReady); + } + }; + window.addEventListener('message', onCodeEditorReady); wnd.location = url; } if (wnd) { diff --git a/src/workers/esm-script.worker.ts b/src/workers/esm-script.worker.ts index 0b830da3e..3c4540d58 100644 --- a/src/workers/esm-script.worker.ts +++ b/src/workers/esm-script.worker.ts @@ -63,11 +63,12 @@ const toSerializableError = (error) => { const code = error.type.startsWith('Invalid Type') ? PLAYCANVAS_ATTRIBUTE_DOCS_URL : null; return { - name: error.node.symbol?.getEscapedName() || 'Unknown', + name: error.attributeName, type: error.type, message: error.message, fix: error.fix, file: sourceFile.fileName, + fileName: sourceFile.fileName.split('/').pop() || sourceFile.fileName, // Extract just the filename startLineNumber: startLineChar.line + 1, startColumn: startLineChar.character + 1, endLineNumber: endLineChar.line + 1, @@ -91,21 +92,16 @@ workerServer.once('init', async (frontendURL) => { const script = attributes[key]; const attributesOrder = Object.keys(script.attributes); acc[key] = { - attributesInvalid: [], + attributesInvalid: script.errors.map(toSerializableError), attributesOrder, - attributes: script.attributes + attributes: script.attributes, + name: key }; return acc; }, {}); - const scriptsInvalid = errors.map((error) => { - if (!error.file) { - return error.message; - } - return `${error.file} (${error.line + 1},${error.column + 1}) [JavaScript]: ${error.message}`; - }); - workerServer.send('attributes:parse', guid, scripts, scriptsInvalid); + workerServer.send('attributes:parse', guid, scripts, errors); } catch (error) { const errorMessage = error instanceof Error ?