webidl
webidl copied to clipboard
ncannasse
Split source / output
Hi guys.. I am very impressed with the work you have done here; I wish I had known about it sooner. Once I got up and running (thanks to @zicklag) I thought I might be able to help out by contributing a few small changes.
- Moved the hard coded webidl.Options to webidl.json in the project folder.
- Added an optional field to webidl.Options for 'out' so we can specify the output folder.
- Added an OUT field to the Makefile (ideall we want this all in one place but since we are using make I thought I would just follow the existing approach)
- Modified Generate.hx to use the opts.out field to put the generated output into a target folder.. in my example it's ./webRoot.
- Added a make clean to remove the generated output from webRoot
It should be possible to simply execute "make js" and have everything dump into webRoot. If you run something like live-server or the python simple server in there you should get the running wasm.
I have tested this on Linux Mint which seems entirely compatible with Ubuntu based on the tested / help that @zicklag provided. I don't have access to OSX / MacOS or Windows unfortunately.
If you are happy with this great, if there are changes you would like, I am happy to make them, if you don't like any of this then that's ok too ;-) I just wanted to see if I could help out. Thanks for your consideration. Kind Regards Wolfie
I haven't tested running it, but I think its a good idea. :+1:
My only thought from looking at it is that we could maybe merge the splitout sample with the other one, or at least put them both in a samples folder.
@zicklag Excellent.. I would be more than happy to merge it into the existing sample.. I just didn't want to go ahead and do that unless I had some kind of indication from you guys that it would be acceptable to do so. I'll manually make the changes and let you know when that's done.. thanks.
Also you might want to wait to hear from @ncannasse. Its his repo, I've just contributed. :smiley:
@zicklag ... cool, @ncannasse with your permission I would like to do as @zicklag suggested :-)
@zicklag ... while I have you here maybe you can answer something for me? What is the plan with regards to handling bindings for multiple c++ classes in the same project. The idl file in the sample is called point.idl presumably because the only thing it handles presently is the Point class but if I wanted to add a class (different header and implementation file) would I just append it to the same .idl file? And if so I suspect I will have to handle it somehow in the makefile? Perhaps make is a bit light for a larger workload if that's the case? Perhaps the work presently being done by make should be handled by a webidl executable? I'm not sure what the long term goals are for this project but it looks like something that I would like to make more industrial use of than just a few samples...
@zicklag well that was easier than I expected. I've gone ahead and updated my fork with the changes necessary to compile multiple c++ targets and generate the bindings. For now I updated the splitout example.
- Handling parsing of #includes (Generalte line 92.. Haxe is refreshingly Functional these days.. )in generator so it is no longer necessary to supply them manually in the webidl.json file. I haven't tested cases where there might be duplicate includes so I have a note to test that and deduplicate the array if that happens (I suspect it will).
- Added a Array<String> field for sourceFiles in webidl.json and removed the hard coded source files in the sample project
From a user perspective:
- All you have to do is ensure the sourceFiles in webidl.json are supplied in the array; it is no longer necessary to call out the includes specifically, it should parse / generate them dynamically now.
- Update the Makefile supplying the cpp files on the libpoint.hdll and (libpoint_win lines if on Windows)
The makefile isn't terrible but I think it could still become troublesome having to make changes in two places.. I'll put some more thought into that. If you guys could test that and let me know what you think I would appreciate it. After you run make.js and launch a webserver in ./webRoot you should see the following output on the js console in your browser (tested on Chrome latest).
splitout/Splitout.hx:17: Result = 11,13 len=17.029386365926403
libpoint.js:8 This is Context!
libpoint.js:8 This is test!
Hey there ! I'm glad there's interest in this project. ATM I have already many (=too much) repositories to monitor so i'm not sure I can invest a lot of time on webidl.
So if @zicklag wants to be added as a contributor and keep developing it, I will gladly give the corresponding rights :) I'm planning to use webidl for bullet HL support, as long as it doesn't break things here it's fine for me.
@ncannasse .. Hi Nick.. very good news, congratulations @zicklag you have been promoted!. I'll go ahead and make the changes you suggested. Thank you both.
Hi @zicklag .. as per our discussion I have gone ahead and merged the samples as follows:
- Created a top level samples folder in the project root
- Renamed the existing sample to simple
- The simple sample is an aggregation of the original and the changes described above in this PR.
I've additionally taken the liberty of changing the naming a little based on our discussions:
- The Module name reflects the folderName+Module.hx --> SimpleModule.hx
- The Entry point reflects the folder name as Upper Case -> Simple.hx
- The IDL file reflects the folderName since the idl file is more related to the module than it's contents. -> simple.idl
When generating output the following naming is used:
- We prefix the folder name with lib_ (snake case) following the examples by the emscripten guys since this is the generated js.
- The custom js bindings are held in folder.js -> simple.js
I guess you are the new maintainer? If so could you please test at your convenience and let me know if any changes are needed or not. If not I am happy for you to merge this as it is. I will have further enhancements to contribute later this week.
@ncannasse .. Nick, you said something important above "so long as we don't break anything".. can you be specific about what that means? What can I test against to ensure that nothing is broken and / or if there are breaking changes what can I use to document against to keep you current?
Also I plan to update the readme to reflect these changes but since it includes a URL which is not yet merged to master I haven't pushed that yet.
Thanks Kind Regards /W
@WolfieWerewolf I'll check it out as soon as I can. :slightly_smiling_face:
As far as what we can't break, it's bullet bindings for HashLink.
Edit: Also about the Readme URL, I think you can just put that in there, and when the PR is merged it will be valid right?
@zicklag ... excellent thanks.. I'll test against that shortly. There will likely need to be some changes to those bindings to make use of this PR.. if so I'll create a related PR for that project. Re the URL... I don't think so.. the url is hard coded in the MD and will point to my fork, not the active master... I mean I could point it to what it would be once merged but I'll have to test that anyway..
@zicklag .. mate.. I haven't tried to compile this yet but it looks to me like the Haxe Bullet bindings include a fork of the webidl that it was using at the time and doesn't reference this project? https://github.com/armory3d/haxebullet/tree/master/Sources/webidl And the webidl files look to be about 3 months old. I would suspect that various changes, not just the ones in this PR, could have had an impact on it. Has anyone tested this recently?
That's not the repo I was talking about here. The one I'm talking about is the one that HashLink itself comes with: https://github.com/HaxeFoundation/hashlink/tree/master/libs/bullet.
BTW I've got a PR for Armory3D's Haxebullet to update it. That is part of the Kha integration I mentioned in the other issue. ;) https://github.com/armory3d/haxebullet/pull/27
oops.. my bad. I'm sorry I'm not following there's a lot going on. Do you still want me to verify against the bullet stuff in hashlink (using the correct URL this time)... sorry for my confusion.. I am just getting caught up with all the dependencies etc..
No problem. You could test it if you want. It should still work fine because I think it pretty much just uses the generated bullet.cpp file from WebIDL and our changes have been mostly to the JavaScript generation and the export locations, which aren't used by that repo directly anyway. Still could be good to test. I think the bullet.cpp file should actually end up identical to the one already there if you generate bindings with the included WebIDL file.
Ok good info thanks.. I will test just to be sure.. I did make some changes to the way the configuration is handled though.. I moved the static config out of the Module to a json file so that might need some tweaking.. thanks for the info
No problem. That was actually the first time I've ever reviewed a PR to a repo that I maintain.
Nice.. I do appreciate it because no matter how much I look at code I'll always miss something.. like the .vscode thing... I'll go through this again with our comments, make the necessary changes and push that up.. likely tomorrow mate.. thanks again.