Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

discuss: NULL and NaN check ability? #11

Open
Dunbaratu opened this issue Mar 15, 2014 · 15 comments
Open

discuss: NULL and NaN check ability? #11

Dunbaratu opened this issue Mar 15, 2014 · 15 comments
Labels
discussion Not necessarily a problem - Using a Github issue to have a public discussion kerboscript For changes to the script language itself.

Comments

@Dunbaratu
Copy link
Member

I think the ability to do the following would be useful:

if isNULL( expr ) { 
  // stuff
}
if isNaN( expr ) {
  // stuff
}

Only if every place in the kOS C# code that could potentially return NaN or NULL to the script manages to check for every case where that could happen and trap it and turn it into a different result would such functions be useless. (And even then I would argue that doing that trap is the wrong solution as it hides relevant data from the script programmer. There is a big difference between a valid number versus a NaN and a big difference between a valid object with zeroed-out data versus a NULL).

For example, you try to look at what your next encounter is. But there is no next encounter. NULL is the sensible way to report that, but currently returning a NULL to the script leaves the script programmer helpless to do anything to detect it and just means their code will crash when they try using the result.

There is also a difference between "your fuel tanks are empty" versus "you have no fuel tanks", so returning zero for cases where a resource is invalid and missing might not actually be the correct answer (see #5).

@erendrake
Copy link
Member

I believe that isNull(expr) or expr = null would be fine, Null will be a valid state sometimes (inactive sensors, body with no atmo, ect) so a null check seems useful

@Dunbaratu
Copy link
Member Author

Here's another example of the relevant difference between a Double object who's value is zero and a null Double object that doesn't exist, and why returning them in a way the script programmer can tell apart from each other could be useful:

In addition to inactive sensors you could have an entirely missing sensor. What if you try to query the current air pressure but there's no barometers installed on the vessel? That should also return NULL, not zero, because zero actually means something useful. Zero means "kOS is telling you that you are in a vacuum right now."

I can easily imagine writing a launching script that, if its attached to a vessel that can measure air pressure, uses that information, but if it can't detect air pressure it needs to know that and disable the air pressure-related code so it doesn't behave as if it was in a vacuum right away on the launchpad.

@Dunbaratu
Copy link
Member Author

It may also be useful to have an IF NULL operator, similar to some SQL implementations:

ifnull( obj, val )

would return obj, unless obj is null, in which case it returns val instead.
For example:

print "The current pressure is " + ifnull( SHIP:SENSORS:PRES, "[cannot read it]" ).

It's basically like what a lot of people use the trinary "?" operator for in C/C++/C#/Java - to put a nullable variable inside a larger expression in a way that protects the null case without having to use a lot of extra verbiage to do it.

@Dunbaratu
Copy link
Member Author

marionapp just made it deliberately crash the script whenever any value ever becomes NaN, so the isNaN functionality is moot. You won't be able to ask if a value is NaN because the script will crash as soon as NaN is the result of any expression, before getting to the line that does the check.

But the isNull idea is still feasable.

@xyztdev
Copy link
Contributor

xyztdev commented Apr 5, 2014

If possible, an isDefined(var) function would also be nice. Or, even better would be to set all undefined variables to null.

@Dunbaratu
Copy link
Member Author

I hope you meant "a way to return null when attempting to read all undefined variables".

To actually set them to null would mean making all possible combinations of N characters taken from the identifier alphabet set come into existence, where N is the max identifier length. You'd run out of memory.

@xyztdev
Copy link
Contributor

xyztdev commented Apr 6, 2014

Well, yes of course. However, from the users POV, it would appear that all of them would be set to null; which is why I phrased it that way.

However, technically you could set all of them to null if you had a system that accepted regular expressions to set variables. (This is of course irrelevant, but I am attempting to defend my initial statement).

@erendrake erendrake added this to the v12.2 Adding features at last. milestone May 10, 2014
@erendrake erendrake removed this from the v12.2 Adding features at last. milestone Oct 11, 2014
@Dunbaratu Dunbaratu removed this from the v12.2 Adding features at last. milestone Oct 11, 2014
@erendrake erendrake added the kerboscript For changes to the script language itself. label Feb 21, 2015
@jenden0
Copy link

jenden0 commented Apr 9, 2015

Assuming this has not been done already and I just haven't found it, would you guys be open to a pull request that implements isDef and isNull? I needed isDef for a script I was writing so I implemented it in my local copy of the source, but I'd be happy to clean it up and add the related isNull so that other people can use them.

@Dunbaratu
Copy link
Member Author

Be aware that we are in the middle of a massive rewrite to handle local variables, and that affects what isDef would have to do (i.e. search the local vars, then the nextmost scope, and so on up the chain to global.) How long ago did you get your develop copy ? If it's more than a few weeks old, chances are it will be operating from the old global-only code and not quite work with the new design.

That being said, I'd be happy to look it over and see what you have.

@jenden0
Copy link

jenden0 commented Apr 9, 2015

Yea, its the last stable release so its well before those changes. I knew that there was a lot of work going on related to functions and didn't think about the impact that would have on scoping.
That being said, if the scoping code at head is close enough to done that its safe to work off of I'd be happy to rewrite my change.
What I've got right now for isDef is pretty trivial though not exactly elegant. I just created a global function that tries to access a variable then returns true if there's no exception. In the case of an exception due to undefined variable it returns false. Presumably as long as there is still some mechanism in the code for retrieving the value of a variable this would probably still work, though I'd kind of like to rewrite it without intentionally causing exceptions.

@jenden0
Copy link

jenden0 commented Apr 9, 2015

I haven't actually checked to see if it works, but this is what the code I have right now for isDef looks like after tweaking it to compile at head. Pretty much identical code works fine at the latest release version.
664deb7

erendrake pushed a commit to erendrake/KOS that referenced this issue Apr 13, 2015
Pulling development branch updates
@jenden0
Copy link

jenden0 commented Apr 23, 2015

Just to follow up on this, my primary use case was to create optional parameters to scripts, but with the new functions, variable scoping, and argument length checking this is no longer helpful for me. Feel free to use my code above if you want, but I won't be pursing it myself at this time.

@WazWaz
Copy link
Contributor

WazWaz commented May 8, 2015

Did this change make it in? The v0.17.0 changelog seems to suggest so, but I can't find it. My use case is simpler: isdef(target) as a way to tell if the player has targeting something yet.

@Dunbaratu
Copy link
Member Author

I don't think it did make it in.

@Dunbaratu
Copy link
Member Author

I am considering changing kerboscripts boolean checks to use proper short-circuiting, which they currently don't. If they do, then using that with an ifdef or ifnull operator would be a good marriage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Not necessarily a problem - Using a Github issue to have a public discussion kerboscript For changes to the script language itself.
Projects
None yet
Development

No branches or pull requests

6 participants