KeenSoftwareHouse / SpaceEngineers

2.93k stars 896 forks source link

Enable System.Threading.Tasks.Task support #525

Open ApocDev opened 8 years ago

ApocDev commented 8 years ago

Having proper Task support (available in .NET 4.5) would be a huge improvement over the currently clunky "MUST USE FSM" model of programmable block scripts.

I have a coroutine implementation based on the Task framework that will essentially "single-thread" the task worker so that it won't cause any threading issues, and ensures that the task is only resumed once per "run". However, I can't actually use it because (inexplicably to me) there is a whitelist of available .NET API, instead of a blacklist (which would be far shorter).

For example; a simple airlock script would be as follows with a proper coroutine implementation: https://gist.github.com/41d594e435fa00ed31912ff116789dcd

If you would like to see the implementation for the Coroutine system, let me know. (I'd prefer not to share it publicly, since it's also used for my day-job)

malware-dev commented 8 years ago

There is no way the Task framework will be enabled for programmable blocks because they need to have 100% control of when a script runs. They use a whitelist because of a blacklist for the same reason: Control. No accidental access to dangerous APIs.

kapitanov commented 8 years ago

Actually there is no need to use the Task framework, you can define your own await-able types so you won't run into any threading problems. I suppose the Coroutine system that @ApocDev has mentioned above uses similar approach.

malware-dev commented 8 years ago

I am aware of this. But in addition to my comment above, which I still believe applies, scripting is usually an entry point for inexperienced programmers, this is what is so great about scriptable games. Understanding async methods are not even close to being entry-level.

Obviously this is merely my opinion, but one I am rather confident Keen shares.

ApocDev commented 8 years ago

Well you're exposing a C# language interface with crippled API (which is fine, except not allowing the Enumerable extension class...), not a "script" interface. They are very different things.

If you want to talk entry-level, then why force FSM's on "entry-level" programmers who don't understand how they work? I'm not asking for async for entry-level people, that's obviously way over their head. There's little reason to constrict the "scripting" to entry-level when there are people who can do some amazing things that aren't actually entry-level.

The Tasks framework allows you to implement your own task runner, which can be single-threaded and single-stepped. (This is how we do it in some of our products that can't be multi-threaded, and need "coroutine" support in favor of things like FSMs and BTs). I'm willing to share some code to make it easier to implement a proper single-thread await-able system, so that's really a non-issue.

I take issue that I have to force people who play with me to use a mod that disables the ILChecker just so I can do something simple that should be allowed in the first place. These would be non-issues if there was a world option to allow any API in the programmable blocks. If you know what you're doing (and trust people), there's no reason not to allow it.

And yes, I understand the "security" reasons for it, but then you also have to do the same for mods. And if that happens, you'll pretty much alienate a lot of the userbase.

malware-dev commented 8 years ago

"I" am doing no such thing. I'm no Keen employee. I just have alot of experience with them because I've had a lot of my work merged, plus by now I probably have more hours behind me on the source of the programmable block than any single Keen employee. I'm the author of the programmable block argument feature, plus a whole lot of bugfixes to its ILInjector - although it's still not fully functional and I believe it needs to be completely replaced.

As to entry level: One wrong does not make another right. I feel like the PB was rushed and not completely thought through. But - just because there were some bad decisions made to begin with, does not mean it's OK to add more complications. However: If there is a way, as you say, to make this work without adding a performance overhead and with 100% control, then I'm all for it - and I would most definitely be interested in your solution for it. It should be possible to come up with a system that will allow for both entry points, one async and one not, without much trouble.

The protection system... I severely doubt you'll ever be able to convince them to release control of that. But I must say I agree that a world option would be nice, but I doubt we'll ever see it.

Finally: The scripting system is, like the rest of the game, not finished. I feel quite confident (and is of the adamant opinion) that it should and will be rewritten at some point, to the point that the currently existing scripts will be broken, each and every one of them. The current API is simply not good enough (I'm not going to argue the semantics on whether it is a "script" api or not... they're APIs for what is called ingame scripts. That's all I care about). However we scripters lie at the very bottom of the priority queue, given that we're so few in comparison with the rest of the players.

ApocDev commented 8 years ago

I didn't mean you specifically, apologies if it came out that way. (I know you're not a Keen employee, and I also know most of the ILChecker/Programmable Block stuff is coming from your PRs).

One thing I've learned from designing and building APIs from the ground up; avoid catering to "entry-level". There is rarely a reason to forcefully hurt yourself just to make sure someone with little knowledge of the material is able to do everything. I'm not saying "don't make it easy". The best APIs are those that are easy to use. But at the same time, alienating those who actually know what they're doing will only be a massive burden, because you get stuck catering to "entry-level", and are never able to evolve your API in any useful way.

As for performance overhead, of course there will be some, but nothing noticeable unless you have many thousands of the things running (which will probably cause the game itself to die way before any perf overhead from a single-threaded-task impl would). 100% control is the easy part, and the entire reason we even wrote it in the first place. We wanted full control over when the "task" could be "ticked", since we are doing synchronization with another process and we need to ensure that the states of both match during the entirety of the "tick" (or things get really wonky).

The API for the game isn't that bad, just some really odd decisions to make what should be method calls (or at least available extension methods), string arguments to an extension method.

I personally can't stand the "My" prefix on everything, it makes me feel like the game was developed using some tutorial for RPGMaker or something like that, but that's just me being nitpicky.

malware-dev commented 8 years ago

I'm not exactly without experience myself when it comes to writing APIs. And there's no denying that the API for this game is... strange, to say the least - and I cringe as much as you do at the My prefix...

What I mean by "entry level" is simply to make it easy enough to use for those with less (or no) experience, not to make it "too easy". Which means making sure entry level people can use it without being forced to learn the advanced stuff too soon, making them give up.

Also don't forget the APIs we're talking of here are for ingame scripting, which means that they must encourage performance based code and make sure there are no meta information leaked. Currently the last one is... lacking at best, and they're well aware of it. It will be fixed eventually but... priorities.

My opinion is that the game+modding and ingame APIs must be separated much further. They serve two different purposes.

I'm still interested in that task solution of yours...