KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
689 stars 228 forks source link

Support for 1.2 KerbNet Scanners #1848

Open GER-Space opened 7 years ago

GER-Space commented 7 years ago

while updating the SCANsat-Addon I was asking myself if kOS should restrict/provide access to the KerbNet scanners.

restrict: only accessable when in view of a scanner. geoposition:Terrainhight new: geoposition:Biome new: geoposition:Ore

I allready finished the neccesary functions, so porting that part to kOS should be trivial.

hvacengi commented 7 years ago

Interesting. I honestly haven't done more than a cursory glance at KerbNet. Is it essentially just like ScanSat? Does it expose any additional information besides what you noted above?

GER-Space commented 7 years ago

It is more a live version of SCANsat. You get a live image within a field-of-view of the scanner and you can use the mouse for selecting a point within.

You have a chance to detect anomalies on the map.

You can also create waypoints.

hvacengi commented 7 years ago

I know we want to integrate the new custom waypoints with kOS. But I don't think that has to be connected to the other KerbNet info.

So in your plan, we would limit that information to geopositions contained within the field of view of the KebNet scanner?

Are the KerbNet and SCANSat data sets similar enough that it might make sense for us to abstract the information to a common interface? It might also open the door for supporting additional mods in the future. That's what I'm in the middle of doing for the CommNet and RemoteTech interfaces. The only difference there is that the communication connectivity is a "choose one" option while KerbNet and SCANSat really need to be simultaneously supported.

Are you thinking of adding KerNet support to the base version of kOS, or keeping it as a separate addon?

If we introduce a KerbNet restriction, we should probably make it toggle-able via setting since a lot of scripts will be potentially broken by it. I know I leverage the ability to get the terrain height of any point heavily in landing scripts. You might want to look at PR #1843 to see how we are going to be leveraging settings with 1.2.

Dunbaratu commented 7 years ago

It raises the question what the values should return if the scanner can't work right now (i.e. doesn't exist or is disabled in some way). I really dislike the standard "HAS" prefix technique we've been using. If there is a value we can return that is obviously bogus and thus can be used to detect that it's not a real value, I'd rather use that then making the script crash if the suffix is attempted to be used when invalid. i.e. you could return a negative value for :ORE when it can't be scanned, since there's no such thing as negative ore for real.

Also, what about the difference between stock temporary data and mod'ed persisting data? In stock the data is gone when the satellite can't see it anymore, as opposed to the information persisting because it got scanned once upon a time in the past in SCANsat (I assume SCANsat does this. I've never used it but heard a lot about it). This matters because you're proposing adding the suffix to GEOPOSITION, not to the scanner mod itself, and it would be very strange if geoposition:ore worked with stock and failed with scansat. So it raises the question: do we try to keep track of where it's legal or not to call it - or can we defer that to whatever mods happen to be installed, as a "live" decision? (i.e. person calls geoposition:ore, then live our code diverts that request from the geoposition class into the other classes that may or may not exist in this installation and lets them answer the question).

hvacengi commented 7 years ago

I agree that adding has[suffix] identifiers for everything is a major pain. But in this case, I think we actually have a good application for a similar model. Instead of a bunch of has suffixes, the KerbNet implementation could simply have kerbnet:isinfov for "is in Field Of View". That idea gets more complicated with SCANSat however, since you perform independent scans of terrain, biome, and resources. Meaning that we'd be back to needing to query for each data type.

The other thing we really should think about is that there are more resources to scan for than just Ore. In that regard the has[suffix] method doesn't even work all that well, since the suffixes would be added dynamically. It may be more logical to return a lexicon of available scan data.

Or maybe we need to finally re-hash the concept of "null".

I'm OK with leaving GeoPosition's existing suffixes pointing to the mod implementation, but I'd be inclined to introduce a scanning specific structure to access the other data. Or maybe the scanning logic accepts a GeoPosition input and returns a structure value that inherits from GeoPosition but includes the scan specific suffixes (which I think was my recommendation to Addon developers to prevent issues with the global namespace).

GER-Space commented 7 years ago

Yes I would like to limit the information of geoposition to the maximum field of view.

I allready support both KerbNet and SCANsat simultaneously in my addon. Both scantypes are basicly a check if the coordinates had been scanned. The data are the same querys that kOS does. The difference is that SCANsat stores the information which coordinates hat been scanned with which scantype. (a bitmask array for each planet), while KerbNet onky allows a live view.

For altitude and resource I allreday return -1.0, when there was no scan. (It is really bad luck if you hit a spot that has a valid high of excatly -1.).

I think KerbNet should be added to base version of kOS, becasue it is a basic feature os KSP.

If you want support SCANsat, then the value returned should depend on the best possible answer. I only need two functions from SCANsat, that could be made version independent with a reflection call, so we don't need to link against scansat.

hvacengi commented 7 years ago

After I finish up my revisions for CommNet and RemoteTech I'll take a look at your existing code. (I should have that finished tonight)

Skippern commented 3 years ago

Hows are thing going with this issue? I would like my rover script to throw current waypoint to KerbNet as target for better visualization.