inkle / ink

inkle's open source scripting language for writing interactive narrative.
http://www.inklestudios.com/ink
MIT License
4.14k stars 494 forks source link

Retarget the runtime to netstandard #808

Closed paulloz closed 1 year ago

paulloz commented 1 year ago

Revert the change made in 5d92a5b3ba3c7a040892ff67649a58c4928adbc6. There's no reason to target net6, and doing so breaks compatibility with systems still targeting older frameworks.

I'm torn about whether we should retarget the compiler as well. It might be a good thing now that I think about it? Happy to hear your opinion about it.

joethephish commented 1 year ago

Hmmm, I think maybe the only reason I did this is because some tool auto-updated it for me and I assumed it knew what it was doing. I think it was maybe when I was trying to test through VSCode? Happy to accept whatever you'd suggest so long as it works with my workflow too!

joethephish commented 1 year ago

Just tried it with your change, these are the errors I get in VSCode:

/usr/local/share/dotnet/sdk/5.0.400/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.TargetFrameworkInference.targets(141,5): error NETSDK1045: The current .NET SDK does not support targeting .NET 6.0.  Either target .NET 5.0 or lower, or use a version of the .NET SDK that supports .NET 6.0. [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/usr/local/share/dotnet/sdk/5.0.400/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.TargetFrameworkInference.targets(141,5): error NETSDK1045: The current .NET SDK does not support targeting .NET 6.0.  Either target .NET 5.0 or lower, or use a version of the .NET SDK that supports .NET 6.0. [/Users/phish/inkle/projects/ink/InkTestBed/InkTestBed.csproj]
joethephish commented 1 year ago

After replacing some more instances of net6.0 with netstandard1.0;netstandard2.0, I now get the following errors:

/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(16,41): error CS0103: The name 'Directory' does not exist in the current context [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(18,52): error CS0117: 'Assembly' does not contain a definition for 'LoadFile' [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(18,66): error CS0117: 'Path' does not contain a definition for 'GetFullPath' [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(20,45): error CS1061: 'Type' does not contain a definition for 'IsAssignableFrom' and no accessible extension method 'IsAssignableFrom' accepting a first argument of type 'Type' could be found (are you missing a using directive or an assembly reference?) [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/FileHandler.cs(14,30): error CS0103: The name 'Directory' does not exist in the current context [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/FileHandler.cs(21,17): error CS0103: The name 'File' does not exist in the current context [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/StringParser/StringParser.cs(113,36): error CS1061: 'StringParser.ParseRule' does not contain a definition for 'Method' and no accessible extension method 'Method' accepting a first argument of type 'StringParser.ParseRule' could be found (are you missing a using directive or an assembly reference?) [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(35,58): error CS0103: The name 'BindingFlags' does not exist in the current context [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(35,33): error CS1061: 'Type' does not contain a definition for 'InvokeMember' and no accessible extension method 'InvokeMember' accepting a first argument of type 'Type' could be found (are you missing a using directive or an assembly reference?) [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(47,59): error CS0103: The name 'BindingFlags' does not exist in the current context [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(47,33): error CS1061: 'Type' does not contain a definition for 'InvokeMember' and no accessible extension method 'InvokeMember' accepting a first argument of type 'Type' could be found (are you missing a using directive or an assembly reference?) [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(59,60): error CS0103: The name 'BindingFlags' does not exist in the current context [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]
/Users/phish/inkle/projects/ink/compiler/Plugins/PluginManager.cs(59,33): error CS1061: 'Type' does not contain a definition for 'InvokeMember' and no accessible extension method 'InvokeMember' accepting a first argument of type 'Type' could be found (are you missing a using directive or an assembly reference?) [/Users/phish/inkle/projects/ink/compiler/ink_compiler.csproj]

...which seems... surprising??

paulloz commented 1 year ago

Your first comment is because you were trying to compile against net6 using the net5 SDK. I can dig a tad more after work this evening for the rest.

paulloz commented 1 year ago

@joethephish This should work fine.

joethephish commented 1 year ago

After doing those locally, I now get the following errors when attempting to run in VSCode:

A fatal error was encountered. The library 'libhostpolicy.dylib' required to execute the application was not found in '/Users/phish/inkle/projects/ink/InkTestBed/bin/Debug/netstandard2.0/'.
Failed to run as a self-contained app.
  - The application was run as a self-contained app because '/Users/phish/inkle/projects/ink/InkTestBed/bin/Debug/netstandard2.0/InkTestBed.runtimeconfig.json' was not found.
  - If this should be a framework-dependent app, add the '/Users/phish/inkle/projects/ink/InkTestBed/bin/Debug/netstandard2.0/InkTestBed.runtimeconfig.json' file and specify the appropriate framework.
paulloz commented 1 year ago

No, these projects should not target netstandard, it is not a target intended for that. I'm pretty sure you'd have the same issues compiling them with the current state on master. Given your earliest copy/paste you're using the 5.0 SDK and those projects were changed to target Net6 in 0fd4504a7a3a6703c142695b8ca98c1440ab35e8.

paulloz commented 1 year ago
  • There are a bunch of scripts that need a find-and-replace done that reference the build output folder (.vscode/launch.json, build_for_inky.command, build_release.command, profiler.command)

I'll replace the paths in the scripts, missed that. I'd suggest moving all of this into MSBuild at some point, it'd prevent from having to reference paths this way 😒

joethephish commented 1 year ago

So what's netstandard intended for?

Running the test bed in VSCode now works, hooray! But now when testing my build_for_inky.command build script I get:

/usr/local/share/dotnet/sdk/5.0.400/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(98,33): error MSB4030: "/p:PublishTrimmed=false" is an invalid value for the "SelfContained" parameter of the "ProcessFrameworkReferences" task. The "SelfContained" parameter is of type "System.Boolean". [/Users/phish/inkle/projects/ink/inklecate/inklecate.csproj]

Use MSBuild? I thought dotnet was meant to be the way forward nowadays?

paulloz commented 1 year ago

So what's netstandard intended for?

It's mostly (only? Somebody could correct me on that) for building libraries.

Use MSBuild? I thought dotnet was meant to be the way forward nowadays?

It is still the compilation engine.

But now when testing my build_for_inky.command build script I get:

Again, seems like an issue that would've been around since the commit I referenced earlier? I don't have the same issue on Windows, so I imagine it's a platform thing. Can you try replacing all the --self-contained with --self-contained true? Or replacing the /p with -p. Either should work.

paulloz commented 1 year ago

Did my last suggestion fix your build script issue?

joethephish commented 1 year ago

Yep that worked! Think that's perfect. I'll just merge now and apply my own fixes directly 👍