Skip to content

Conversation

@roginvs
Copy link
Contributor

@roginvs roginvs commented May 21, 2025

Description

This PR embeds a WebAssembly-compiled compile.exe into this extension. It is under options flag and it is disabled by default.

Having this compiler as WebAssembly build allow using this extension on non-intel processors, for example on Mac M1 or others.

image

image

Notes

Currently it uses compiled files from my repo but I will change it into sfall repo as soon as improvements PRs are merged.

I have not tested this on Windows but likely it will also work there

@burner1024
Copy link
Member

Interesting, I haven't considered M1. Does it not run wine? I thought there was some compatilibility layer.

@roginvs
Copy link
Contributor Author

roginvs commented May 22, 2025

Actually I have no idea, but i had an impression that it was not possible to run x86/x64 apps on M1 Macbooks.

Anyways, even if it is possible then it still requires some setup. Having a embedded cross-platform compiler should be better user experience in my opinion. Maybe later we can enable it by default

@roginvs
Copy link
Contributor Author

roginvs commented May 22, 2025

CI fails on found npm access token: npm_i_save_dev_types_Slash but it also fails on almost empty PR roginvs#2

@burner1024
Copy link
Member

microsoft/vscode-vsce#1153 maybe

@roginvs
Copy link
Contributor Author

roginvs commented May 23, 2025

Yes, looks like it was a bug in vscode-vsce. Right now all checks passed.

I changed installation script, now it will check hash of downloaded release file. Just more security. With hash check it is not a big difference where this file was downloaded from

@burner1024
Copy link
Member

I'll look it over soon.

@burner1024
Copy link
Member

@copilot review this

@roginvs
Copy link
Contributor Author

roginvs commented Jun 11, 2025

I'd assign Copilot to review this PR if I had permission to add reviewers. :)

@burner1024 burner1024 requested a review from Copilot June 12, 2025 05:58

This comment was marked as outdated.

@roginvs roginvs requested a review from Copilot June 12, 2025 11:14

This comment was marked as outdated.

@burner1024
Copy link
Member

@copilot review this

I'd assign Copilot to review this PR if I had permission to add reviewers. :)

Copilot itself advised me to make that comment to request a review, apparently that didn't work :), so assigned it in GUI.

@roginvs
Copy link
Contributor Author

roginvs commented Jun 13, 2025

Updated to use sslc from https://github.com/sfall-team/sslc

@roginvs roginvs requested a review from Copilot June 13, 2025 21:07

This comment was marked as outdated.

@burner1024
Copy link
Member

Looks like it has trouble working with symlinks.
On my system, Fallout2_Restoration_Project/scripts_src/headers/sfall is a symlink to sfall headers. External compiler works with that, while built-in one doesn't. Also leaves cruft behind.

Screenshot_20250614_132921
Screenshot_20250614_132846

Linux build works fine too.

@roginvs
Copy link
Contributor Author

roginvs commented Jun 14, 2025

@burner1024 Thanks, I will take a look. I have Linux and symlink too but it is relative symlink. I tested now with absolute symlink and I confirm that it needs fixing.

I will try to use NODERAWFS again, very likely it fails to find path because of absolute symlink and because host FS is mounted into subfolder.

About leftover *.tmp: it also happens on Linux build, probably because of https://github.com/sfall-team/sslc/blob/master/compile.c#L239 and https://github.com/sfall-team/sslc/blob/master/compile.c#L261 . I will take a look on this too

@roginvs
Copy link
Contributor Author

roginvs commented Jun 15, 2025

@burner1024

  • Leftover .tmp files are fixed. Merged in my repo, waiting to be merged in sslc repo
  • I changed filesystem back to NODERAWFS because we need to mount host FS into the root inside guest. Mounting host into sub-folder caused absolute symlinks to fail. Because NODERAWFS does not work good on multiple runs we have to spawn new process each time, similar to using compile.exe

@burner1024 burner1024 requested a review from Copilot June 20, 2025 10:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a built-in WebAssembly-compiled compiler for Fallout2 .ssl files, enabling cross-platform support when the external compiler is unavailable. Key changes include:

  • Adding a new module (ssl_compiler.ts) to integrate the built-in compiler.
  • Updating settings and documentation to include a new flag (useBuiltInCompiler) and related configuration.
  • Modifying the compile function in fallout.ts to conditionally use the built-in compiler.

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server/src/sslc/ssl_compiler.ts New module for invoking the built-in WebAssembly SSL compiler.
server/src/settings.ts Added new boolean flag for using the built-in compiler.
server/src/fallout.ts Updated compile logic to check for and fallback to the built-in compiler.
server/src/compile.ts Updated call to compile to await asynchronous execution.
server/package.json Version bump and dependency addition for the built-in compiler.
package.json Version bump and updated settings schema for using the built-in compiler.
docs/settings.md Documentation update to reflect the new built-in compiler setting.
docs/changelog.md Changelog entry for the new built-in compiler feature.
README.md Update to highlight the embedded cross-platform .ssl compiler.
.vscodeignore Excludes built-in compiler dependency from being ignored.
Files not reviewed (1)
  • server/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)

server/src/fallout.ts:598

  • The variable name 'successfullCompilerPath' is misspelled; consider renaming it to 'successfulCompilerPath' for clarity.
let successfullCompilerPath: string | null = null;

server/src/fallout.ts:674

  • The message 'Succesfully compiled' is misspelled; please change it to 'Successfully compiled'.
        } else {


cmdArgs.push(opts.inputFileName, "-o", opts.outputFileName);

const p = fork(
Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

Consider handling the 'error' event on the forked child process to reject the promise in case of early failures.

Copilot uses AI. Check for mistakes.
@burner1024 burner1024 merged commit 88181cf into BGforgeNet:master Jun 22, 2025
1 check passed
@burner1024
Copy link
Member

OK, merged and released, thanks.

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.

2 participants