Skip to content
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

Use native compiler on Linux #10

Closed
wants to merge 2 commits into from
Closed

Use native compiler on Linux #10

wants to merge 2 commits into from

Conversation

mtijanic
Copy link

There is a native compiler available on Linux, no need to go through wine.

I used the binary from: https://github.com/silm/silm/blob/master/build/nwnnsscomp_lin
Sources available at: https://github.com/niv/nwn-tools/tree/master/nwnnsscomp

The command line is slightly different, but it does the same thing (based on the same code). There is also a native build for OSX, but I don't have the setup to test it.

Also updated the readme for linux to note the additional dependency.

- nwnnsscomp has a slightly different command line, but does the same thing
- Binary forked from github.com/silm/silm/blob/master/build/nwnnsscomp_lin
- Remove wine dependency from README.md

There is also a macOS native binary, but I can't test the setup with it.
@jakkn
Copy link
Owner

jakkn commented Sep 29, 2017

Hi @mtijanic, thank you for your contribution!

Everything that runs native is definitely an improvement, however I have a few concerns:

  • The output directory compiler argument (-b) was used to route .ncs output to cache/gff/ from the compiler. This is not strictly necessary, though it will have to be handled by the ruby script.
  • The module filename compiler argument (-m) is necessary for NWNScriptCompiler.exe to resolve hak includes and differ between modules in cases where there are more than one. Does nwnnsscomp handle this more intelligently?
  • I checked out your branch but was unable to compile any of my module scripts with nwnnsscomp.
$ bin/nwnnsscomp -go -v1.69 NWN src/nss/bop_switches.nss
NeverWinter Nights Script Compiler/Decompiler
Copyright 2002-2003, Edward T. Smith
Copyright 2003, The Open Knights Consortium

Errors occurred in compiling "src/nss/bop_switches.nss"

I tried several files but since they all fail I assume the compiler is missing some information.

  • nwnnsscomp and NWNScriptCompiler.exe may share a common ancestor but they have definitely diverged. As such, the two compilers may produce different output which can potentially result in code that compiles fine on Linux and errors out on Windows or the other way around.

As I was writing this I remembered having looked into nwnnsscomp a couple years ago and discussed the use case with niv. The feedback I got was that using different compilers within the same team is asking for trouble, and that using wine is an acceptable compromise for cross-platform support until someone, if ever, ports Skywing's compiler to POSIX.

I am definitely positive to using native compilers for both the improved execution time and the dependency reduction, but we have to address the above issues first of which I believe the potential variation in compiler output raises the highest concern.

@mtijanic
Copy link
Author

You're right on all accounts.

  • The -b argument is better handled by ruby, or best by outputting to stdout and piping it to tee
  • The -m argument I totally failed to see. Typically, nwnnsscomp would require the hak be unpacked to a certain directory, and then add that directory to the include path (similar to how cc works).
    -- However, it seems that support isn't merged to niv/nwn-tools, so it'd need this instead: https://github.com/meaglyn/nwn-tools
    -- Either way, the logic to add these to the include paths is completely missing from my branch, so it doesn't work. Again, I totally missed this usecase.

Finally, regarding different compilers, I'm kinda torn. There are benefits to compiling the same code with different compilers. But I guess for typical NWN use cases, it's not worth the hassle.

Let's kill this PR now then.

Is there an exact list of things NWNScriptCompiler does better? I'm thinking of taking a shot at porting it, if there's enough features to warrant it. Otherwise, might just be better to integrate the few features to nwnnsscomp and have some tests to make sure they generate compatible code.

@mtijanic mtijanic closed this Sep 29, 2017
@jakkn
Copy link
Owner

jakkn commented Sep 29, 2017

It's hard for me to say what the differences are. The output of -h provides some indication in terms of usability features, but bugfixes and tweaks is more difficult to tell. The last change to Skywing's repo was Dec 26, 2014 with the first on Dec 7, 2012, and I think the preceding history is locked away somewhere in a private SVN repo. Without the history it's hard to define an exact feature gap.

If you're interested in porting Skywing's compiler then I would definitely start by checking what niv has to say on the topic. I have had the same thought more than once, but my professional experience is in Java and web development and porting a C++ program to POSIX is too big of an undertaking for me. But if you're interested in committing the time and effort needed to get this done then I would be more than happy to help out and I should have enough time on my hands in the coming months to see it through.

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