KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
697 stars 230 forks source link

Stopgap check for attempts to call functions from interpreter that were made in programs. #886

Closed Dunbaratu closed 9 years ago

Dunbaratu commented 9 years ago

This is a stopgap until #717 gets implemented later (if ever).

Add an ability to record which context a delegate function came from, and spit an error when it's the wrong context. (calling a program's function from inside the interpreter).

abenkovskii commented 9 years ago
  1. This time from the user's point of view there was a function in interpreter. screenshot1
  2. Can it be solved by using "$terminal-foo" instead of "$foo" when declaring / calling a function from the interpreter?
Dunbaratu commented 9 years ago

The cause of the entire problem is the really annoying fact that all files share global scope for variable names but not for ML code. So it remapped where $foo* pointed when bug.ks was run. One proper fix would be to go back in time and never have made all files share the same scope - make a proper file-level scope. But that can't be done now without breaking all the old prior-to-0.17.0 code that still uses "run" like a function call to communicate data in the global variable scope. (And that wasn't my design - that was like that since the beginning) There's going to be a LOT out there. There are some things I don't mind breaking, but that one will break everything. People will find code examples posted, try them, and find they don't work.

So the only real fix, going forward, is going to be having to eventually put all the code inside the same memory space just like the variables are, and making it so that programs that finish keep their functions around, and THAT is messy too (really messy) because the interpreter can't be running things in a two-part pass, scanning ahead of time to see what's going to be run later, like a program can. The code that leaves functions behind in memory is dependent on that behavior,

Your idea may work, but it would be tricky. In the long run I think making all functions be UserDelegates will probably work better. Then the UserDelegate can remember what its context is.

Dunbaratu commented 9 years ago

It occurs to me that I've been thinking through this problem all wrong. Until such a time as we actually do support calling functions from programs inside of the interpreter, why not just utterly disable functions in the interpreter altogether? So you can't even use a function made in the interpreter from the interpreter either.\

Basically, if in interpreter context, then ban the use BOTH of function calls, and declare function statements, with a helpful error message saying its not enabled yet.

Because, really, how useful is it to write a function out by hand at the interpreter prompt, and then call it at the interpreter prompt (which is the only use case that currently works that this change would break)?

I'd be perfectly happy just saying that functions simply don't work at all in the interpreter, as long as a reasonable error message explaining it was given, rather than the random spew of whatever error messages you get now.

One thing that will still make functions at the interpreter especially challenging, even if the problem with the two ML spaces is solved, is that the interpreter can't read ahead and do a two-pass compile. It cannot detect that you're going to call a function later, like the program compiler can. The compiler actually makes use of that ability in how it deals with functions now.

austinjames314 commented 9 years ago

Yeah that sounds sensible to me. My biggest frustration trying to get into using KOS, is all the times I try to do something, and I get a cryptic and vague error message. (Some of the error messages are good). Disable it and put in an error that explains it's not possible right now, and a new user can just go "Oh I guess I'll have to write a wrapper script" instead of "WTF?!?!? WHY DO YOU HATE ME!? WORK DAMMIT!!"

erendrake commented 9 years ago

discretion is the better part of valor. Lets do it :)

abenkovskii commented 9 years ago

Same works for locks: screenshot0 log: https://www.dropbox.com/s/2nzjjiejo08gck0/same_for_locks.txt?dl=0

Dunbaratu commented 9 years ago

Well crap. Locks never used to work at all for bridging between program and interpreter either, but their case won't be covered by denying function calls from the interpreter.

abenkovskii commented 9 years ago

Yeah. We can't completely remove locks from the interpreter because: 1) They was there for a long time and people got used to this feature 2) It will make piloting directly from the terminal impossible

abenkovskii commented 9 years ago

What about running a script that doesn't affect throttle after locking the throttle from terminal? I'm too lazy to try it but I guess it will break to. Is this behavior documented?

Dunbaratu commented 9 years ago

Actually wait... locks ARE UserDelegates, so it should be possible to record for them, whether they were defined in the interpreter or in a program. It isn't extendable to user functions, but it will work with locks to do that.

abenkovskii commented 9 years ago

Will a user delegate be restored after a program execution finishes execution?

Dunbaratu commented 9 years ago

Earlier @abenkovskii (I think it was you, I can't remember) claimed that doing unlock throttle. doesn't work in the terminal after having locked it in the terminal:

kOS Operating System
KerboScript v0.17.1

Proceed.
lock throttle to 1.
unlock throttle.

The above works just fine when I do it. It puts the throttle up to 100%, and then the unlock puts it back to its (stupid) default of 50% again.

abenkovskii commented 9 years ago

@Dunbaratu Issue #894. Can't reproduce it anymore. May be my testing of undefined behavior somehow affected it.

Dunbaratu commented 9 years ago

Addressed in PR #896

erendrake commented 9 years ago

@Dunbaratu ready to close?

abenkovskii commented 9 years ago

@Dunbaratu, now because all functions are user delegates adding function pointers should be easy. I think it can be done just by compiling delegate foo to push $foo*? Am I missing something?

Dunbaratu commented 9 years ago

missed this one earlier. it should be closed.