Nivekk / KOS

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

ENCOUNTER's inconsistet object type makes it imposssible to check against. #243

Open Dunbaratu opened 10 years ago

Dunbaratu commented 10 years ago

If you check the value of ENCOUNTER to find out if you have an encounter, you just can't seem to do it because whether you do or do not have an encounter will change the TYPE returned by ENCOUNTER, making KOSscript complain one way or the other.

So how do I test if there's an encounter?

This statement will give a runtime error if there IS an encounter: if ENCOUNTER = "None" { ..whatever.. }. and this one will give a runtime error if there is NOT an encounter: if ENCOUNTER:body = myIntendedBody { ...whatever... }.

Because in order to tell you that there's no encounter, ENCOUNTER returns a String containing "None" as its value, but if there is an encounter, then ENCOUNTER returns an OrbitInfo which is not allowed to be compared to a String.

KOSscript doesn't have reflection. You can't ask "what is the type of this object?' as a means of performing the test. You have to assume you know the type and write the code accordingly.

As far as I can tell, this is an impossible to work around problem at the moment. (If anyone has a workaround please let me know. I need this for my entry into ThrFoot's challenge contest.)

I suggest one of these two approaches: 1: Just like ENCOUNTER can pretend it's a string when it's the subject of a PRINT statement, also make it pretend it's a string (giving the value of the encounter body's name) when it's the subject of a comparator, so it will allow you to compare it to "None" to find out if it's a real encounter before then trying to access its suffix parts. (or) 2: Make a dummy OrbitInfo object to be returned when there's no encounter, so that ENCOUNTER still has the suffix parts BODY, PERIAPSIS, and APOAPSIS that you can compare against, but they contain dummy values.

baloan commented 10 years ago

This works fine for me (KSP 0.22, kOS 0.9.2). The trick is to use TARGET:NAME which returns a string.

set found to 0.
set lost to 0.
until lost {
    remove nd.
    set nd to node(time:seconds + eta + 1, 0, 0, dv).
    set eta to nd:eta.
    set n to n+1.
    add nd.
    print "eta:" + eta at (0,30).
    print "encounter:" + encounter at (0,31).
    if encounter = target:name { 
        // print "node:eta: " + round(eta) + ", periapsis: " + round(encounter:periapsis/1000) + "km".
        set found to 1.
        if encounter:periapsis < minper {
            set minper to encounter:periapsis.
            set mineta to time:seconds + eta.
        }
    }
    if encounter = "None" and found = 1 {
        set lost to 1.
    }
}
Dunbaratu commented 10 years ago

Ah. I did find one workaround but I still think this needs fixing.

The workaround is this: The following expression: "" + ENCOUNTER effectively forces KOSscript to cast ENCOUNTER to a string like it would if it was being printed.

so I can do this: if ("" + ENCOUNTER) = "None" { .... whatever ... }.

@boloan, I have no idea why that code works for you. For me if encounter = target:name complains that it can't compare a string to an orbitinfo.

a1270 commented 10 years ago

What about this, it seems to work:

set Trgt to "Mun".
if encounter/:body == Trgt { stuff }.

Why you can't compare ENCOUNTER(string) to "Mun"(string) still eludes me.

Dunbaratu commented 10 years ago

I assume the slash in encounter/:body was a typo. I've never seen that syntax construct before in KOS and don't know what it means.

I'll write the rest of my comment under the presumption that was a typo and you meant encounter:body. If this is not true, please disregard what I say.

The reason that doesn't work is this: Try your example code when there is no current encounter. What happens? The program stops with an error. It complains that there is no such suffix as 'body". Because when there is no encounter, the Object type of ENCOUNTER ''changes'' to a plain String instead of an OrbitInfo.

This is what I described above in the OP.

If an encounter exists:

If an encounter does not exist.

So to compare the encounter to a string to see if it's "Mun" or "None", and you don't know which it is yet, without crashing the script, you have to write a line of code that simultaneously does and does not contain the suffix ":body" on the word "encounter".

And that is the problem I am describing.

The only fix I found that works is that appending ENCOUNTER to an empty string causes it to run its to-string conversion, and thus I get an expression that's a string variable regardless of whether it originally was one or not. But that took a long time to figure out and it's based on knowing what the code behind the scenes is doing, and how to force it to cast Encoutner to a string.

a1270 commented 10 years ago

"/" is commonly used as an or+ operator. At least in the circles i am in. So encounter OR encounter plus :body. Including encounter:body was probably a mistake and I'll blame it on being distracted. Sorry about going on your rehashing your OP, i understood the issue but i believe comparing encounter to a sting variable is a valid work around. I am working on how to get encounter subelements to return defaults if no encounter.

Dunbaratu commented 10 years ago

@a1270 where you say: "So encounter OR encounter plus :body" highlights the problem that I didn't seem to be getting across to you. It's not "OR". It's actually AND. The way it works now, I have to write the expression both ways at the same time if I want it not to crash, and be able to to cover the case where I MIGHT have an encounter with "Mun" coming up or I MIGHT have an encounter with "None" coming up and I don't know which it is yet (which is why I'm running the expression - to find out which it is).

The expression ENCOUNTER:NAME caused the program to error out if ENCOUNTER was of type String.

The expression ENCOUNTER (without :NAME) caused the program to error out if ENCOUNTER was of type OrbitInfo.

You're acting as if I'm in a situation where I know ahead of time which version to write. But the point is that the SAME expression has to handle both cases.

Because I'm trying to do this:

Keep increasing the prograde delta-V until I detect that it will cause an encounter.

There was no way to write that UNTIL expression because the same expression has to handle both cases. It has to run successfully when ENCOUNTER is a string ("None") and also not crash when it suddenly changes to an OrbitInfo object later.

I'm glad it's being changed even if you don't get what the problem was.

a1270 commented 10 years ago

There is a miscommunication. "So encounter OR encounter plus :body" tried to convey:

set Trgt to "Mun".
if encounter/:body == Trgt { stuff }.

or

set Trgt to "Mun".
if encounter:body == Trgt { stuff }.

Not this:

set Trgt to "Mun".
if encounter or encounter:body == Trgt { stuff }.

247 should address the errors you're having with ENCOUNTER and ENCOUNTER:BODY. http://i.imgur.com/lKNOSsY.jpg There is an issue with the parsing which makes it not want to compare vars and bare strings.

if encounter == "Mun" { stuff }.

That is why the above code doesn't work.

Dunbaratu commented 10 years ago

So are you saying that a string literal like "Mun" and a string variable like someBodyName which happens to be set to "Mun" don't have the same effect on casting operands of == ? That one will force ENCOUNTER to be cast to a string for the sake of comparison but the other one wont? That's really, really weird..

If that's the case than perhaps the fix you're implementing may be too drastic a change. I'm wondering if it will screw up scripts people already wrote expecting the encounter of "None" to be just a string.

If the true error was that ENCOUNTER was meant to be casted to a string in the comparison but there's something wrong, in general, with all cases of string literal comparisons that they don't do that properly, then maybe fixing THAT is the answer, rather than this.

If it is true the ENCOUNTER does have the right method defined to cast itself into a string (and it seems it does or else you couldn't get the string from PRINTing it, and that this was meant to handle the problem I've been talking about but this other unrelated problem with comparisons to string literals is the culprit, them I'm sorry I made you make the change. Perhaps the real fix is to deal with the problem with string literal compares in general. That would probably solve more problems for more people in more generic cases than this will, At any rate I have a different workaround for now.

a1270 commented 10 years ago

247 doesn't change anything but the way it returns the string. The old code checked to see if there was an encounter, returning OrbitInfo(encounter), else returning "None". The new code does the same but instead returns a null OrbitInfo(). When null it'll return "None" on ENCOUNTER or default values on subelements(BODY = "None", etc). As for the parsing issue, i haven't had enough free time to fully understand the new one and don't want to poke around too much.