Skip to content

Add built-in compiler for Fallout2 .ssl files #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

roginvs
Copy link

@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 sfall-team/sslc#14 is merged

Lots of any because I have not found a good typescript definitions for emscripten module (@types/emscripten have no .FS property on the instance)

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
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
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
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

So you aren't using an M1? What prompted you to work on this then?

@roginvs
Copy link
Author

roginvs commented May 24, 2025

@burner1024
I was making a PR with test scripts into fallout2-ce. In the README file I added some instructions about how to compile https://github.com/fallout2-ce/fallout2-ce/pull/140/files#diff-7e0064f2b1902c3c7083e171e90a0a91df55a38157bf30de762952196392b179 but then I though that having an additional step of installing compile.exe can be annoying.

I am simple user, I want a button which makes everything for me. As user I want to install extension and to have a ready-to-use IDE.

@burner1024
Copy link
Member

I am simple user, I want a button which makes everything for me. As user I want to install extension and to have a ready-to-use IDE.

Well, the simplest way for that would be to just bundle compile.exe, I guess.
But having native cross-platform builds paves the way for future improvements, so that's cool too.

Let's see when the upstream PR gets merged.

@roginvs
Copy link
Author

roginvs commented Jun 3, 2025

Looks like PR into sslc got stuck for a while. Let's hope it will be merged at some point.

I think there is no blockers right now for merging this PR. I tested it locally and it works for me. There is a hash of the package in package-lock.json so npm/pnpm will not allow to install something else which do not have the same hash

@burner1024
Copy link
Member

I'll look it over soon.

@burner1024
Copy link
Member

@copilot review this

@roginvs
Copy link
Author

roginvs commented Jun 11, 2025

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

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