Issue #226 fix - made inklecate and tests project depend on ink-runtime-engine

[stackh34p]

Issue #226 Fix. Changes including:

• Moved ink-engine-runtime to its own csharp project
• Renamed tests project output to ink-tests.dll. Justification - tests is a too general name to allow internal member visibility to. Instead, ink-tests feels more confined to the ink project.
• Allowed ink-engine-runtime to expose internals to ink-tests and inklecate
• Updated inklecate to expose internals to ink-tests project
• Removed ink-engine-dll project and references in bundle scripts -- it is now completely substituted by the ink-engine-runtime.dll

All unit tests are passing

Due to being limited on working on an Ubuntu machine, I was not able to check whether the nuget packaging project works, so it should be a subject of attention when merging this PR.

[stackh34p]

Should not have missed that, but on Linux this file was initially hidden

[joethephish]

Thanks for this! Note that it might be a little while until I have a chance to review it - we're really busy on a milestone for our new project - but I'm definitely planning to review and merge when I have moment :)

[joethephish]

Hrm, unfortunately when loading up the solution in Xamarin on Mac, I get this error:

...and the runtime project fails to load:

Any ideas??

[stackh34p]

That is strange, I will check this

[stackh34p]

It did not load under Xamarin on Windows as well, but did not show an error. I removed the project and added it again, then added project references for ink-engine-runtime to the other two projects. It worked on MonoDevelop on Linux though. I hope it is all well now.

[joethephish]

Hmmm, so that loads okay now, thanks!

Right now I'm having problems running my release build script (build_release.command - designed to run on Mac but I don't think there's anything much that shouldn't run on linux?) Specifically this line is failing:

mkbundle ./inklecate/bin/Release/inklecate.exe --deps --static -o ./ReleaseBinary/inklecate

It's failing to find the ink-engine-runtime assembly in order to statically link. Not sure why it shouldn't be able to find it... any ideas?

(Sorry for the lack of investigation at my end, I'm really tight on time at the mo!)

[stackh34p]

I will get to the bottom of this. I think failures like this are somewhat expected now as the inklecate.exe now requires the ink-engine-runtime alongside in order to work, which is specific to this PR.

[stackh34p]

Figured it out - as I initially suspected, since inklecate now depends explicity on ink-engine-runtime, mkbundle commands needs to have the ink-engine-runtime.dll listed alongside inklecate.exe in order to create a valid package. Thus I got fixed the missing file error.

I am now getting an error "invalid option r" from the "cc -arch i386" options when running the build scripts on Ubuntu. Looks to me that this is some Mac-specific stuff, or is it not so? I have no Mac at my disposal otherwise I would have verified this myself, sorry for this.

If the ink-engine-runtime.dll was the only issue you should have the script working. Otherwise we will investigate further. Hope its all OK.

[joethephish]

Great, that worked!

So, I was almost ready for release, but hit one more snag :-( We like to include source for the runtime in the Unity plugin so that if it crashes in Unity, there's more information about where it went wrong. So we stopped using the runtime DLL there. However, if it's a requirement for inklecate.exe, then we'd need both - but I'd assume that Unity wouldn't be happy with that.

Possible (or non-) solutions:

• Stop including runtime source in the Unity plugin (I'd really rather not - at the very least we need it specifically on our own internal game project as we develop ink, even if we did this for the public version.)
• Is there a way to statically include the DLL in inklecate? But that would sort of defeat the point, and you may as well keep things the way they were.
• Unfortunately we can't include the entire source of inklecate too, since it uses a version of C# that's too new for Unity. This is something we want to do in future when Unity supports more language features though.
• A way to tell Unity not to use a DLL in game code even though it's included in the project purely for inklecate.exe? e.g. put both in some special folder that prevents Unity from treating it as an asset? In fact, maybe this would already be an improvement - perhaps we're accidentally including a copy of inklecate in every game right now...! (Thoughts, @tomkail?)
• Something else??

[stackh34p]

OK, if I understood the issue correctly, you want more information on ink-engine-runtime when it fails in a Unity game? And you are working this around by having the sources of the runtime as part of the unity plugin you've developed.

If that is the situation, my intuitive reaction would be to ship the unity plugin along with the runtime. and include the runtime .mdb file (this is mono's debug symbol file, it is generated upon build with mono and can be found in the Debug output directory.) As far as I know, Unity is able to read mdb files and uses them to display more accurate stack traces in the console.

I could assist you with this one as well, but I need to have a look at the unity plugin first, which I have not seen yet.

[stackh34p]

Hello Again.

From what I am initially seeing, it appears that you are keeping the inklecate.exe alongside the unity plugin so that you can compile the stories to json.

You have "a copy" of the ink-engine-runtime sources inside unity in order to play the stories within Unity itself. While certain reuse of the compiled ink-engine-runtime can be made, I do not see any reason for not keeping the sources within Unity as they are now.

Anyway, here are some important things to notice, and to consider: Unity uses its own version of the .NET framework. It is likely that .dlls will not work well (or even not at all) in Unity if they have been compiled against the default .NET runtime, or if they use higher version of the framework than .NET 3.5. Therefore, having the ink-engine-runtime.dll in the Assets folder is not likely to contribute to reusing its functionality (since it is compiled against a standard v3.5 profile, not for the Unity subset). A possible side effect would also be that Unity still detects the runtime dll, and complain about it not being complied in a compatible way. The latter should not be a blocker, could be seen as bad user experience for not-so-adept Unity users.

So, here are a few workarounds I believe would do the job without much changes:

• Just include the runtime dll where currently the inklecate.exe is located -- it should work as before. Only the exe will make use of the compiled dll while the Unity plugin will use the embedded sources of the runtime. We are definitely missing the point of re-using the runtime dll in Unity here, but as you read on, you might consider this to be the better case.

• Introduce pre-compiled versions of the runtime that are compatible to Unity specific subset modes. The bad thing about this is that these builds would be absolutely Unity-specific, rather than widely reusable, so you must still maintain the current builds for the other ink-related projects. Building for Unity is very tricky, as it has 4 different subsets of the .NET framework which it offers for compilation:

  • Unity Full v3.5 - still incompatible with the standard v3.5 framework builds, but the closest in features
  • Unity Subset v3.5 - the default for Unity. Some featured from the .NET frameworks are missing, like .NET configuration support and windows-specific stuff
  • Unity Micro v3.5
  • Unity Web v3.5 - the last two are even more limited.

  Furthermore, all four seem not to be very compatible with each other as well, therefore in order to properly support Unity, one should provide separate builds for each profile. That being said, it looks to me like too much effort for the effect we desire. Another side effect could be that inklecate.exe might not like working with the Unity-distribution of the runtime, and you may have to distribute an inklecate.exe version along which is compiled against the same profile as the runtime. This looks like an awfully complicated scenario.

  On the other hand, if we keep the runtime sources inside Unity, we are free of the above restrictions. Unity will properly compile them into the final game using what the end user has chosen as a platform (stripping us from supporting the various profiles).

• This option is quite similar to the first one. It introduces a small change, in case the presence of the current runtime dll causes Unity to complain for it being compiled for the standard .NET profile. This warning can be avoided by simply moving the inklecate.exe and the dll outside the Assets folder, and instruct the plugin to call inklecate from the new location.

I still have not played yet with these options but my experience with Unity and compiled dlls tells me we'd better not use dlls for Unity in this scenario. I will write again here if there is a more impact-less solution

[joethephish]

Okay, so Unity can have folders that aren't interpreted as assets, if their name is prepended by ~. Plugins on the Unity asset store have to be imported into the Assets folder anyway, and it's the convention to keep everything there. So I'll keep both inklecate and the DLL in a ~ folder, and then simultaneously have the source for the purpose of compiling into the game itself (my concern was over the existence of duplicate definitions).

Edit: To hide folders from Unity, they need to be appended by ~, not prepended.

[joethephish]

Anyway, everything's merged and working nicely - just need to do a new round of releases now. Thanks!

[stackh34p]

Nice, glad it worked out well. I am happy I've made a somewhat meaningful contribution
P.S.
As long as Unity itself does not load the ink-engine-runtime.dll there will not be any duplicate definitions, so you do not have to worry :)

[joethephish]

Well, that's the problem - it does load the DLL and give duplicate definitions, so we had to do somewhat crazy things when the Unity plugin is loaded to get around it. But at least it's working now...!