Nivekk / KOS

Fully programmable autopilot mod for KSP.
Other
80 stars 30 forks source link

Trying to lock a value to an expression relative to itself makes flagrant error. #233

Open Dunbaratu opened 10 years ago

Dunbaratu commented 10 years ago

I was trying to adjust a throttle setting as follows:

set myth to 0. // to start with.
lock throttle to myth.
lock myth to myth + 0.02 * ( (desiredSpeed-mySpeed)/abs(desiredSpeed-mySpeed) ).

The idea being that the throttle variable "myth" will adjust itself to whatever it used to be plus or minus a small amount depending on if I'm going faster or slower than I'd like to be. I figured over time this will move the throttle by 0.02 each command execution tick until it hits the level where speed is where I want it.

This caused KSP to drop to an FPS rate of about 1, and it spewed lots of Flagrant Errors to the log.

I eventually figured out that ANY time you have a lock expression that references the variable being locked it does this. The following has a problem as well:

set x to 1.
lock x to x + 0.001.
print x.

I'd have expected this to increase x by 0.001 per KOS CPU clock tick.

But it appears to be trying to recurse infinitely, unable to figure out what the value of "x" is because it keeps trying to re-evaluate the expression on the fly right now instead of taking the previous value of x as the value to use within the expression.

i.e. it's expanding this: x + 0.001 to this: (x + 0.001) + 0.001. to this: ((x + 0.001) + 0.001) + 0.001). etc....

and so it never gets around to finishing the evaluation and you have to hurry up and use control-C before it bogs down all of KSP and kills it.

Here's the start of the exception that appears in the KSP.log. I've truncated it because it repeats the same message over and over as it keeps hitting the same exception during the recursion. The actual message is about 10000 lines long in my log, but you can get the idea from this bit:

[LOG 11:52:19.705] System.StackOverflowException: The requested operation caused a stack overflow.
  at kOS.ExecutionContext.FindVariable (System.String varName) [0x00000] in <filename unknown>:0 
  at kOS.Expression.AttemptGetVariableValue (System.String varName) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValueOfTerm (kOS.Term term) [0x00000] in <filename unknown>:0 
  at kOS.Expression.TryProcessMathStatement (kOS.Term input) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValueOfTerm (kOS.Term term) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValue () [0x00000] in <filename unknown>:0 
  at kOS.ExecutionContext.FindVariable (System.String varName) [0x00000] in <filename unknown>:0 
  at kOS.Expression.AttemptGetVariableValue (System.String varName) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValueOfTerm (kOS.Term term) [0x00000] in <filename unknown>:0 
  at kOS.Expression.TryProcessMathStatement (kOS.Term input) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValueOfTerm (kOS.Term term) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValue () [0x00000] in <filename unknown>:0 
  at kOS.ExecutionContext.FindVariable (System.String varName) [0x00000] in <filename unknown>:0 
  at kOS.Expression.AttemptGetVariableValue (System.String varName) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValueOfTerm (kOS.Term term) [0x00000] in <filename unknown>:0 
  at kOS.Expression.TryProcessMathStatement (kOS.Term input) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValueOfTerm (kOS.Term term) [0x00000] in <filename unknown>:0 
  at kOS.Expression.GetValue () [0x00000] in <filename unknown>:0 
JoCRaM commented 10 years ago

lock doesn't work as you imagine - it is more like a macro, that is evaluated whenever it is needed - which is every "cycle" for someting locked to steering or throttle, or in a when condition.

This is why it is possible to lock unassigned values - LOCK locked TO var. PRINT locked. SET var TO 3. PRINT locked.

Dunbaratu commented 10 years ago

Yeah but there's no reason it couldn't be implemented in a way that would make it work. The problem isn't that it is evaluated when needed. I knew that. The problem is that it forgets what it was set to last time it got evaluated, so it has no prev val to use and has to re-eval itself on the fly even inside its own expression.

If it was both a variable, and an expression that changes that variable whenever it's mentioned except it its own expression, it would work.

In other words, here's what it does now: ''I am being used in an expression. Run the expression and return that, and then forget what the answer was.''

What I'm talking about is making it do this:

Alter lock expressions's object so they contain two things: The expression to evalate (like it is now), and The most recent value that was calculated (this is the new thing I'm talking about).

Then it could behave like this instead: ''I am being used in an expression somewhere.''