GameServer icon indicating copy to clipboard operation
GameServer copied to clipboard

Swap the CSharpScriptingEngine for the Nuget package

Open LeagueSandboxBot opened this issue 6 years ago • 4 comments

I put the scripting engine into its own Nuget package. We can use that as a Nuget library.


Beep, boop, I'm a bot! This issue was created by @Gan in #development.

LeagueSandboxBot avatar Aug 26 '18 00:08 LeagueSandboxBot

I'm a bit doubtful on this one, as the scripting engine is so closely coupled to the game server. Will have to discuss this more

MythicManiac avatar Aug 26 '18 11:08 MythicManiac

https://github.com/MatthewFrench/CSharpScriptingEngine/blob/master/CSharpScriptingEngine/CSharpScriptingEngine/CSharpScriptEngine.cs

Here’s the CSharpScriptEngine class. It doesn’t have any dependency on GameServer, making it completely decoupled in a code sense. Even though it was made with only GameServer in mind, it can easily be dropped in any project as a scripting engine. We can keep it in GameServer because it’s only a single file or we can remove it. It won’t have any serious impact.

MatthewFrench avatar Aug 26 '18 12:08 MatthewFrench

Hmm, my personal opinion is that we probably shouldn't extract it as far as a nuget package at this point at least.

This is because it's such a small piece of code that's not in the way of anything, and at least I can see it possibly being edited later on. If it's a NuGet package, it becomes a lot more difficult to make edits.

I do see the thinking behind this, and were the package itself bigger or guaranteed to basically never change, I'd think this is a good idea to do now.

I'll reconsider if this gets suport from more people, but for the time being I don't think this is the best choice.

MythicManiac avatar Aug 26 '18 13:08 MythicManiac

The scripting engine actually changed frequently for implementing more features in it. I think that a NuGet package is the way to go, but seems like that keeping it in the files make it much easier to update/add new features. I think that it shouldn't be in another NuGet in this case.

danil179 avatar May 07 '21 11:05 danil179