-
Notifications
You must be signed in to change notification settings - Fork 28
Wasm csharp biding generator #117
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
base: main
Are you sure you want to change the base?
Conversation
…sed workerEventProcessor
…de, update event handling for compile and run
|
Oliver-Quail
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Daniel,
While I have been somewhat harsh, I think this work is amazing, it just needs some comments in the meantime to help with other people being able to work on it later down the line. I get this is a work in progress as well, so I don't think you need to improve your documentation immediately as I can see you've started writing it but in your next pull request please add more.
Finally I was unable to run your code on my system due to a syntax error in buildAndCopy.sh. I've included the details in a comment.
I will be happy to approve once these changes are made :)
|
|
||
| let hasErrors = false; | ||
|
|
||
| // If all good, then output the 'compiled' result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Daniel,
While I can see what is going on here. (Using the compiled binaries to compile a program), adding a comment here clarifying that the program is being complied not the binaries would help with clarity
| } | ||
| } | ||
|
|
||
| return compiled; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering if there is any documentation present on what data the compiled variable holds? If so please could you add it. I understand the priority right now is getting the program working, so please add a comment if you don't have time to document
| iframe.id="iframetest"; // this code is primordial... | ||
| if (language.needsSandbox) | ||
| iframe.sandbox = 'allow-scripts allow-modals'; | ||
| iframe.sandbox = 'allow-scripts allow-modals allow-same-origin'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Daniel,
While I can understand what is going on here with CORS, I think a comment clarifying what these three attributes do would be valuable, particularly for those working on this project in the future
| const loadDotNet = async () => { | ||
| const { setModuleImports, getAssemblyExports, getConfig } = await dotnet | ||
| .withDiagnosticTracing(false) | ||
| .withApplicationArgumentsFromQuery() | ||
| .create(); | ||
|
|
||
| setModuleImports("main.js", { | ||
| window: { | ||
| location: { | ||
| href: () => globalThis.window.location.href, | ||
| }, | ||
| }, | ||
| SplashKitBackendWASM: { | ||
| // TODO: Pass the rest of the SplashKit functions | ||
| write_line, | ||
| refresh_screen, | ||
| open_window, | ||
| fill_ellipse: () => { | ||
| // Research how to pass a JS object in WASM | ||
| fill_ellipse(color_black(), 260, 260, 200, 200); | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| const config = getConfig(); | ||
| const exports = await getAssemblyExports(config.mainAssemblyName); | ||
| return exports; | ||
| }; | ||
|
|
||
| const CompileAndRun = async (code, reportError) => { | ||
| try { | ||
| const exports = await loadDotNet(); | ||
| const result = await exports.CSharpCodeRunner.CompileAndRun(code); | ||
| if (result.includes("Compilation failed")) { | ||
| const errors = result.split(":"); | ||
| const errorLine = errors[1].split("Line"); | ||
|
|
||
| const indexCorrector = 1; | ||
| const filePath = "__USERCODE__/code/main.cs"; | ||
| reportError( | ||
| filePath, | ||
| result, | ||
| Number(errorLine[1]) + indexCorrector, | ||
| null, | ||
| true, | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| console.error("Error during code execution:", error); | ||
| } | ||
| }; | ||
|
|
||
| // This event will be trigger by the csharp compiler | ||
| document.addEventListener("compileAndRun", (ev) => { | ||
| CompileAndRun(ev.detail.program[0].source, ev.detail.reportError); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Daniel,
Don't fix it for this commit as I get that this is a work in progress but in your next pull request please can you add some documentation or comments to this code. That is if no documentation exists
| { | ||
| name: "CSharp", | ||
| userVisibleName: "C#", | ||
| aliases: ['C#', 'CSharp'], | ||
| sourceExtensions: ['cs'], | ||
| compilableExtensions: ['cs'], | ||
| defaultSourceExtension: "cs", | ||
| setups: [{ | ||
| name: "C#", | ||
| runtimeFiles: [ | ||
| { src: "moduleEventTarget.js", type: "text/javascript" }, | ||
| { src: "loadsplashkit.js", type: "text/javascript" }, | ||
| { src: "fsevents.js", type: "text/javascript" }, | ||
| { src: "CSharpWasm/main.js", type: "module" }, | ||
| { src: "runtimes/ExecutionEnvironmentInternal.js", type: "text/javascript" }, | ||
| { src: "runtimes/csharp/csharpRuntime.js", type: "text/javascript" }, | ||
| ], | ||
| runtimeDependencies: [ | ||
| "runtimes/javascript/bin/SplashKitBackendWASM.js", | ||
| "runtimes/javascript/bin/SplashKitBackendWASM.wasm", | ||
| ], | ||
| compilerFiles: [ | ||
| "compilers/csharp/csharpCompiler.js", | ||
| ], | ||
| runtimeSizeAprox: 20, | ||
| compilerSizeAprox: 150, | ||
| compilerName: "csharpCompiler", | ||
| supportHotReloading: false, | ||
| getDefaultProject: ()=>{return makeNewProject_CSharp;}, | ||
| persistentFilesystem: false, | ||
| compiled: true, | ||
| needsSandbox: false, | ||
| needsServiceWorker: false, | ||
| }] | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curios, is this process documented somewhere?
| var sb = new StringBuilder(); | ||
| sb.AppendLine("using System.Runtime.InteropServices.JavaScript;"); | ||
| sb.AppendLine(); | ||
| sb.AppendLine("namespace SplashKitSDK"); | ||
| sb.AppendLine("{"); | ||
| sb.AppendLine(" public partial class SplashKit"); | ||
| sb.AppendLine(" {"); | ||
|
|
||
| if (jsonData != null) | ||
| { | ||
| foreach (var module in jsonData.Values) | ||
| { | ||
| if (module.Functions == null) continue; | ||
|
|
||
| foreach (var function in module.Functions) | ||
| { | ||
| if (function.Signatures != null && | ||
| function.Signatures.TryGetValue("csharp", out var csharpSigs)) | ||
| { | ||
| foreach (var sig in csharpSigs) | ||
| { | ||
| if (sig.Contains(" SplashKit.")) | ||
| { | ||
| sb.AppendLine($" [JSImport(\"SplashKitBackendWASM.{function.Name}\", \"main.js\")]"); | ||
| sb.AppendLine($" {CleanSignature(sig)}"); | ||
| sb.AppendLine(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can you add a comment that specifies that you are binding C# to JavaScript just to make this clearer?
| # Build results | ||
| bin/ | ||
| obj/ | ||
| .DS_Store |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again please add these to the global .gitignore
| # CSharpWasm | ||
|
|
||
| This project enables building and running C# code in the browser using WebAssembly (Wasm). | ||
|
|
||
| ## Project Structure | ||
|
|
||
| - **SplashKitInterop.cs**: This file contains the logic for binding C# code with JavaScript functions, enabling the use of SplashKit functions within the C# environment. | ||
|
|
||
| - **CSharpCodeRunner.cs**: A class responsible for compiling C# code, providing the necessary functionality to run the code dynamically. | ||
|
|
||
| - **buildAndCopy.sh**: A shell script that builds the project and copies the necessary files into the `Browser_IDE/CSharpWasm` directory. This script helps automate the build process for easy integration with the browser. | ||
|
|
||
| ### Running the Build and Copy Script | ||
|
|
||
| To build the project and copy the necessary files into the `Browser_IDE/CSharpWasm` directory, run the following shell script: | ||
|
|
||
| ```bash | ||
| ./buildAndCopy.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get this is a work in progress but in your next pull request or when you get this to a working state please can you move this file in the documentation folder?
| "name": "browser", | ||
| "host": "browser" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking that this tells C# to run the wasm in the browser. If so please can you leave a comment noting this. It will mean that others in the future will be more easily able to work on this section of the project
| FRAMEWORK_DEST="../Browser_IDE/CSharpWasm/wwwroot/_framework" | ||
|
|
||
| # List of files to copy | ||
| FILES=( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting when I tried to run this, I got an error on this line for unexpected character "(". My system is Linux, I'm not sure if this helps but please can you look into this
Description
This is the first part of the bindings generation functionality. This initial PR covers generating bindings on the C# side. Additionally, this PR is a follow up of the C# WebAssembly support pull request.
Type of change
expected)
Note:
dotnet run.Next Steps:
buildAndCopy.shscript to run the BindingsGenerator project, create a new version of the bindings, copy them toCSharpCodeRunner, build the project, and copy and paste the output to the IDE folder.Question:
Do we need to generate types on the C# side?
Images:
dotnet run, it should generate a the bindinds in the same directory. SplashKit.Generated.cs