Improve legacy MPQ diagnostics and preview error reporting - #99
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 612bd96cb3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| vscode.commands.registerCommand('wurst.openWurstHome', () => openWurstHome()), | ||
| vscode.commands.registerCommand('wurst.copyDiagnostics', () => copyDiagnostics()), | ||
| vscode.commands.registerCommand('wurst.showLogs', () => showLanguageServerOutput()), | ||
| vscode.commands.registerCommand('wurst.showDiagnosticsActions', async () => { |
There was a problem hiding this comment.
Move these user-facing command registrations into src/features/commands.ts; placing four handlers in the language-server lifecycle module splits command ownership across modules and violates the repository's explicit command integration convention, making future command discovery and maintenance error-prone.
AGENTS.md reference: AGENTS.md:L26-L30
Useful? React with 👍 / 👎.
| @@ -0,0 +1,60 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
Relocate this new diagnostics implementation under src/features/ rather than adding another top-level src module. It implements a user-visible diagnostics feature shared by previewers and commands, while the repository explicitly reserves top-level files for the established architecture slices and directs new editor features to src/features/.
AGENTS.md reference: AGENTS.md:L41-L43
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| const detail = formatDiagnosticError(error); | ||
| const message = `${root.kind.toUpperCase()} read failed: ${gamePath}: ${detail}`; | ||
| log(message); | ||
| channelLog(message); |
There was a problem hiding this comment.
When an asset is found only through a later extension, locale, or HD fallback, findCascTexture and findCascAsset deliberately call gameReadDirect on several absent candidates first. This catch now records every normal miss, including a stack trace, and also invokes both the supplied logger and channelLog; common preview loads can therefore flood the output and evict the useful failure from the 100-line diagnostics buffer. Log only a terminal lookup failure or an error after the storage has confirmed that the entry exists.
Useful? React with 👍 / 👎.
| const text = fs.readFileSync(filePath, 'utf8'); | ||
| const lines = text.split(/\r?\n/); | ||
| while (lines.length > 0 && lines[lines.length - 1] === '') lines.pop(); | ||
| return lines.slice(-MAX_DIAGNOSTIC_LINES); |
There was a problem hiding this comment.
When ~/.wurst/logs/languageServer.log has grown large during a long-lived installation, Copy Diagnostics synchronously reads and splits the entire file on the extension-host thread even though only 100 lines are retained. This can freeze VS Code and temporarily allocate several times the log size; read a bounded chunk from the end of the file, preferably asynchronously.
Useful? React with 👍 / 👎.
| void vscode.window.showErrorMessage( | ||
| `Failed to extract ${name}: ${e instanceof Error ? e.message : String(e)}` | ||
| `Failed to extract ${name}: ${formatDiagnosticError(e)}` | ||
| ); |
There was a problem hiding this comment.
When opening an MPQ entry fails, formatDiagnosticError returns error.stack, so the user-facing error notification now contains the complete multiline stack rather than the concise message used previously. Preserve the stack in the newly added diagnostic log, but use e.message for the notification so routine extraction failures do not produce an oversized, implementation-heavy toast.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9db4792855
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| scripts/** | ||
| e2e/** | ||
| wc3data/** | ||
| resources/wc3-knowledge-base.json |
There was a problem hiding this comment.
Do not exclude this resource from the VSIX while loadCompilerKnowledgeBase() still uses it as the fallback when the installed compiler JAR is missing, busy, old, or lacks the knowledge-base entry. In a fresh/offline or standalone installation without usable WC3 game data, the packaged extension now returns undefined instead of loading the bundled metadata, leaving object-editor fields and object references without their schemas, labels, and catalog data.
Useful? React with 👍 / 👎.
| // editing so typing doesn't start by appending to that literal text. No real content existed to click | ||
| // into, so the captured range (if any) is meaningless here too. | ||
| if (!original) { body.innerHTML = ''; range = null; } | ||
| else body.innerHTML = renderWc3Colors(original); |
There was a problem hiding this comment.
When a user clicks inside any non-empty tooltip to edit it, the caret range captured above refers to nodes in the current preview, but this innerHTML assignment removes those nodes. The later addRange(range) therefore restores a range into the detached markup rather than the newly editable contents, so typing no longer reliably begins where the user clicked. Preserve/map the clicked text offset or capture a new range after rendering the raw value.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97838d3b68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // into, so the captured range (if any) is meaningless here too. | ||
| if (!original) { body.innerHTML = ''; range = null; } | ||
| if (!original) { body.innerHTML = ''; caretOffset = null; } | ||
| else body.innerHTML = renderWc3Colors(original); |
There was a problem hiding this comment.
When a tooltip starts with Unit / or Building /, the collapsed preview removes that prefix, so caretOffset is measured against shorter text; this assignment then restores the full original before applying the unchanged offset, placing the caret several characters before the location the user clicked. The new offset conversion is fresh evidence beyond the earlier detached-range issue: adjust the offset by the rendered hidden prefix length or map it against the full value.
Useful? React with 👍 / 👎.
| const message = formatDiagnosticError(error); | ||
| appendDiagnostic('VS Code extension', `Wurst language server failed to start: ${message}`); | ||
| vscode.window.showErrorMessage(`Wurst language server failed to start: ${message}`); |
There was a problem hiding this comment.
When the language client fails during start() or onReady(), formatDiagnosticError() normally returns the complete multiline stack, and this line places it directly in the user-facing VS Code notification. This produces an oversized implementation-heavy toast for routine startup failures; keep the stack in appendDiagnostic, but use error.message or another concise value for the notification.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83890fda14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (remaining <= length) { | ||
| const range = document.createRange(); | ||
| range.setStart(node, remaining); | ||
| range.collapse(true); |
There was a problem hiding this comment.
When the clicked offset falls exactly between a colored <span> and following plain text, remaining <= length restores the caret at the end of the preceding span rather than the start of the next text node. Typing there can therefore inherit the preceding color and serialize new text inside an unintended |cff...|r range. The new offset-based remapping is fresh evidence beyond the earlier detached-range issue; retain the clicked range's boundary affinity when mapping it into the replacement markup.
Useful? React with 👍 / 👎.
| export function tooltipPreviewText(v) { | ||
| return String(v == null ? '' : v).replace(/^(?:unit|building)\s*\/\s*/i, ''); |
There was a problem hiding this comment.
This helper is applied to every editable value selected by needsColorEditor and every read-only value containing color markup, not only tooltip fields. Consequently, a colored name, description, or other string legitimately beginning with Unit / or Building / is silently truncated in the collapsed preview even though the stored value still contains the text. Gate the cleanup on the field being a known tooltip/template field rather than on its text alone.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b64887e40b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| diagnosticsClient.outputChannel.show(); | ||
| return; | ||
| } | ||
| void vscode.commands.executeCommand('workbench.action.output.toggleOutput'); |
There was a problem hiding this comment.
When this command activates the extension in a window without a workspace, or after language-server startup fails, diagnosticsClient is never assigned. The fallback only toggles the Output panel and leaves whichever unrelated channel was previously selected, while the startup error exists only in the in-memory diagnostics buffer, so the new “Open Wurst output” troubleshooting action does not show any Wurst diagnostics precisely when startup is broken. Retain/show a dedicated Wurst output channel or make the client output channel available before startup completes.
Useful? React with 👍 / 👎.
1 participant
What changed
Root cause
Some MPQ files store a final raw sector at full physical sector size even when the remaining logical output is smaller. The reader incorrectly attempted PKWARE decompression for that sector, causing errors such as
PKExplode: invalid literal size byte 40. The correspondingcasc-tsreader fix is maintained in the sibling repository.Validation
casc-tsformat suite: 80 passed, 2 skipped