Skip to content

fix: Molang rewrite of math constants to avoid function conversion#487

Merged
DaanV2 merged 5 commits into
mainfrom
copilot/fix-molang-rewrite-math-constants
May 15, 2026
Merged

fix: Molang rewrite of math constants to avoid function conversion#487
DaanV2 merged 5 commits into
mainfrom
copilot/fix-molang-rewrite-math-constants

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 12, 2026

  • Investigate why packages/bedrock-diagnoser and ide/* hang during tests
  • Add forceExit: true to packages/bedrock-diagnoser/jest.config.ts
  • Add forceExit: true to root jest.config.ts (used by all ide/* packages)
  • Root cause found: getLastEndPosition in packages/molang has a while loop with no default: case — when it encounters a node with an unknown type (e.g. the empty {} operand produced by parsing !!v.temp), the loop spins forever since {} is never undefined
  • Fix: add default: return (node as ExpressionNode)?.position ?? 0; to the switch in getLastEndPosition
  • Rebuild packages/molang — all 649 tests pass
  • Run per-package tests with 60s limit — all 8 packages/* suites pass; ide/* finish within 60s (pre-existing 4-suite failure unrelated to this fix)

@github-actions github-actions Bot added dependencies Pull requests that update a dependency file release vscode ide packages labels May 13, 2026
…ng optimizations

- In `string.ts`: Don't append `()` for zero-argument FunctionCall nodes (e.g. `math.pi`)
  so that constants are serialized correctly without parentheses.
- In `nodes.ts`: Fix infinite loop in `getLastEndPosition` for FunctionCall with 0 args
  by returning early with the correct identifier length.
- In `nodes.ts`: Add `ExpressionNode.getStartPosition()` helper that finds the minimum
  (leftmost) source offset among all descendants of an expression node.
- In `registry.ts`: Use `OffsetWord` with the full expression range (from `getStartPosition`
  to `getLastEndPosition`) when adding optimization diagnostics, so code actions replace
  the correct text instead of just the operator and adjacent identifier.
- In `optimizations.test.ts`: Add regression test asserting that `5*3*math.pi`
  produces a replacement containing `math.pi` and not `math.pi()`.

Agent-Logs-Url: https://github.com/Blockception/minecraft-bedrock-language-server/sessions/c46da7a3-c138-4dfb-a355-ca41b9f597bc

Co-authored-by: DaanV2 <2393905+DaanV2@users.noreply.github.com>
@DaanV2 DaanV2 force-pushed the copilot/fix-molang-rewrite-math-constants branch from 9480ca5 to 3b9ddc2 Compare May 15, 2026 09:53
@DaanV2
Copy link
Copy Markdown
Contributor

DaanV2 commented May 15, 2026

@copilot I have no idea where, but some or a test is hanging / not timing out in the pipelines, can you fix that and then rebase it?

@DaanV2 DaanV2 marked this pull request as ready for review May 15, 2026 11:35
@DaanV2 DaanV2 changed the title [WIP] Fix Molang rewrite of math constants to avoid function conversion fix: Molang rewrite of math constants to avoid function conversion May 15, 2026
Copilot finished work on behalf of DaanV2 May 15, 2026 14:14
@DaanV2 DaanV2 merged commit f332136 into main May 15, 2026
4 checks passed
@DaanV2 DaanV2 deleted the copilot/fix-molang-rewrite-math-constants branch May 15, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file ide packages release vscode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Molang rewrite attempts to turn math constants into functions

2 participants