Skip to content

Wasm#39

Open
o-murphy wants to merge 9 commits intomasterfrom
wasm
Open

Wasm#39
o-murphy wants to merge 9 commits intomasterfrom
wasm

Conversation

@o-murphy
Copy link
Copy Markdown
Owner

No description provided.

o-murphy and others added 7 commits January 13, 2026 11:23
Major architectural changes:
- Implement WASM bindings for core ballistic calculations using Emscripten
- Add C++ exception handling with proper JS error type conversion
- Integrate py-ballisticcalc C++ engine via WASM

Exception handling:
- Add C++ exception support via Emscripten (-fexceptions, DISABLE_EXCEPTION_CATCHING=0)
- Implement universal wrapExceptions template for automatic C++ to JS error conversion
- Add specific converters for OutOfRangeError, ZeroFindingError, InterceptionError
- Register exception classes in globalThis for proper instanceof checks
- Add comprehensive exception handling tests

HitResult API improvements:
- Implement getAt() method with WASM interpolation for trajectory queries
- Add flag() and _checkFlag() methods for TrajFlag validation
- Add toWasmTrajectoryData() converter for TrajectoryData to WASM format
- Add props getter (alias for shot) for Python API compatibility
- Add error field for storing RangeError

Code cleanup and deprecations:
- Remove deprecated dangerSpace() method and DangerSpace class
- Remove TypeScript atmospheric calculation methods (now in WASM)
- Remove unused engines (euler, rk4) and generics
- Remove src/helpers.ts
- Convert drag_tables.js to TypeScript

Unit system improvements:
- Add footPound getter to Energy class
- Add pound getter to Weight class

Build and configuration:
- Add Makefile for WASM compilation
- Add jest.config.js with ESM support
- Update ts-jest config to modern format (remove deprecated globals)
- Add .editorconfig, .eslintignore, .prettierignore
- Add tsup.config.ts for bundling

Bug fixes:
- Fix maximumDrop and minimumAltitude to be negative (matches Python behavior)
- Fix zero-finding test to use await for async setWeaponZero()
- Align incomplete-shot tests with Python test parameters

Test results: 376 passed, 6 failed (expected for incomplete-shot edge cases)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the ballistic trajectory calculations to a WebAssembly-based engine, integrating the bclibc library. Key changes include the introduction of a WasmManager for module handling, refactoring core domain models such as Calculator, Shot, Atmo, and TrajectoryData to utilize WASM bindings, and updating the test suite for asynchronous operations. The PR also establishes project-wide standards with new configuration files, a Makefile for build orchestration, and a formatting guide. Feedback identifies a hardcoded local path in the VSCode settings, identical 'before/after' examples in the formatting documentation, and the use of non-English text in the Makefile and source code comments.

Comment thread .vscode/settings.json Outdated
} No newline at end of file
"editor.parameterHints.enabled": true,
"editor.suggest.snippetsPreventQuickSuggestions": false,
"cmake.sourceDirectory": "/home/murphy/jsproj/js-ballistics/lib/bclibc"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The cmake.sourceDirectory is set to a hardcoded absolute path. This path is specific to your local machine and will not work for other contributors. It should be replaced with a workspace-relative path using ${workspaceFolder}.

Suggested change
"cmake.sourceDirectory": "/home/murphy/jsproj/js-ballistics/lib/bclibc"
"cmake.sourceDirectory": "${workspaceFolder}/lib/bclibc"

Comment thread FORMATTING.md
Comment on lines +92 to +108
**Before formatting:**

```javascript
function test(a, b, c) {
const result = a + b + c;
return result;
}
```

**After formatting:**

```javascript
function test(a, b, c) {
const result = a + b + c;
return result;
}
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The 'Before formatting' and 'After formatting' code examples are identical. Furthermore, the 'After' example is inconsistent with your .prettierrc configuration, which specifies "semi": true. The example should demonstrate a code block being formatted correctly by Prettier.

Here's a suggestion for how it could look:

Before formatting:

function test(a,b,c){
    const result=a+b+c
    return result
}

After formatting:

function test(a, b, c) {
    const result = a + b + c;
    return result;
}

Comment thread Makefile
@echo ""

update-emsdk:
@echo "🔄 Оновлення Emscripten SDK..."
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This message is in Ukrainian, while most other messages in this file and the project are in English. For consistency and to make it accessible to a wider audience, it's recommended to use English for all user-facing output. This also applies to other messages in this file (e.g., lines 124, 126, 146).

	@echo "🔄 Updating Emscripten SDK..."

Comment thread src/conditions.ts Outdated
`Invalid temperature: ${fahrenheit}°F. Adjusted to (${cLowestTempF}°F).`,
);
console.warn(`Invalid temperature: ${fahrenheit}°F. Adjusted to (${cLowestTempF}°F).`);
fahrenheit = cLowestTempF; // ← ДОДАНО: коригування
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This comment is in Ukrainian. To maintain consistency across the codebase, please write all comments in English.

Suggested change
fahrenheit = cLowestTempF; // ← ДОДАНО: коригування
fahrenheit = cLowestTempF; // Clamp to the minimum temperature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant