Ezriilc / HyperEdit

A plugin for Kerbal Space Program.
http://www.Kerbaltek.com/hyperedit
GNU General Public License v3.0
41 stars 30 forks source link

Lander is behaving as if coordinates are different now in KSP 1.3.1 #29

Closed Ezriilc closed 6 years ago

Ezriilc commented 6 years ago

The behavior is strange, and makes the Lander unusable. It tends to put the ship inside the planet and/or just makes it explode with very strange results in the map view.

This particular problem seems entirely new with KSP 1.3.1, and not at all like previous quirks of the Lander, which one could work around.

eberg86 commented 6 years ago

I am having this problem currently. Part of the reason I quit playing about 8 months ago was for this same reason. I have not been back to check however as to what the resolution might have been for that then.

I even went as far as trying to find a mod to freeze time because it seems if I set a marker on the ground then try to land a ship there the longer I wait the farther away from the mark the ship will end up.

fronbow commented 6 years ago

I've been trying to debug this problem for the past 2+ weeks and getting bogged down by ksp calculations/mechanics :-( After a seemingly productive evening, I think I may have found one of the culprits. The variable IntermAltitude in Lander.cs is behaving strangely! I'm currently stepping through the logic of FixedUpdate to find out why it seems to remain constant when I think it should decrease to get closer to the surface or at least nearer to the entered Altitude.

Ezriilc commented 6 years ago

I'm very glad that you are working on this, as I'm genuinely lost on this one - I simply don't know enough to even begin.

Please let me know whatever you may learn, and good luck. Thanks in advance!

fronbow commented 6 years ago

Don't worry, I'll update here if anything else jumps out at me! I have a sneaking suspicion that the altitude thing is what's causing the lat/lon shifts, though it'll take more stepping through the code tomorrow now that I've got a good Watch list set up!

eXigentCoder commented 6 years ago

Hey, if it helps with debugging this issue I noticed the following behaviour. While trying to get to a lander by the face on Duna from another vessel, I clicked the "Select" button and choose the target. When I warped to it, I was at the correct latitude but the wrong longitude. I kept manually entering in random longitudes until I was at roughly the correct position and landed. Later I entered the co-ordinates that I had manually worked out and again was at a completely different longitude but still the same latitude.

After going through a few cycles of keeping the coordinates the same and clicking "Land" then waiting a bit then "Drop (CAUTION!)" followed by another "Land", I'm starting to think that the planets rotation is having an effect on how the mod is determining the location coordinates.

This would account for why sometimes the ship will teleport inside the planet and blow up

Ezriilc commented 6 years ago

@eXigentCoder , thanks very much for your input. I don't personally know what to do with it, but all applicable information is welcome.

fronbow commented 6 years ago

Thanks for this @eXigentCoder I'm still slowly stepping through the source. I thought I'd solved it last week but the results seem to vary depending on the target body. So far I've managed to get it to teleport me closer to the surface (still cause for random exploding!) but the Longitude calculations don't seem to work at all as expected.

fronbow commented 6 years ago

Just a FYI! I'm still debugging this and trying to find what is going on. FWIW there's some interesting functions in kOS that I might have to borrow for the ground calculations. So far it seems that any ground calculations that are made above 10km get skewed as soon as the vessel gets within that 10km.

(Wish there was some early code documentation on what the functions here do!!)

Another line of thinking I have is that the number bases are getting mixed up. e.g. degrees, radians, and decimal. So we may need something similar to kOS' Geo-coordinates functions that 'normalises' lat and lon into a uniform base? Another thing that has me curious is that the named places for landing are more successful at getting you to the surface than random coords/altitude. (i.e. you are less likely to spawn under the terrain!!) If you want to play about with my stuff it's here: https://github.com/fronbow/HyperEdit/tree/testing (My VS rewrote the tabs/code layout to how I usually do stuff!!)

Ezriilc commented 6 years ago

It's all over my head, and so I'm very appreciative of your efforts.

chrisverwey commented 6 years ago

I agree with @eXigentCoder assessment. I did pretty much the same process of elimination. Yesterday I read somewhere that MechJeb had the same behaviour in KSP 1.3.1 but that it's now fixed in a recent developer release. I browsed through their commit history and I noticed 2 commits that sound like a possible fix. In commit #744 they changed method calls to GetRelSurfacePosition to GetWorldSurfacePosition. This apparently didn't work because in commit #747 they added "- body.position" to the end of the method call. I noticed that HyperEdit has 4 calls to GetRelSurfacePosition in Lander.cs and I wonder if this doesn't need to be updated? I have no background in coding mods, so I may be way off. Apologies if so.

Ezriilc commented 6 years ago

Thanks! I will take a look at it, just in case I find something simple to change.

fronbow commented 6 years ago

Thanks @chrisverwey I must've missed that; time for more playing!

fronbow commented 6 years ago

Ok, that worked. Though I was only using 0,0,100 for the lander data. @Ezriilc do you want to test my testing branch - I've upped a compiled dll to make it easier! See ~https://github.com/fronbow/HyperEdit/releases/tag/v1.5.4.2-alpha~ It probably needs more testing but I'm going to build a release version (it takes out all the debug stuff at compile time) and play about later without the overhead of having VS in the background!

Ezriilc commented 6 years ago

Yes! I will give that a try. If you think it's a good fix, then I can publish your build as a BETA on our page.

THANKS!

fronbow commented 6 years ago

I'm going to play around with it in my live build as I have more saved coords I can try (with vehicles so I can see if I get near!). If that's all good then we're good to go! I'm going to look into that freezing resources thing at some point too!

Ezriilc commented 6 years ago

All my tests work perfectly. Great job, @fronbow ! Can you send me a pull request?

EDIT: By putting up a BETA, we'll have the whole community testing for us.

fronbow commented 6 years ago

Cool, pull request is on its way. It's probably safe to keep it as a separate branch at the moment...unless you want to merge it? I'm not using any rogue debugs so they can stay in and will only compile in for debug releases!

Ezriilc commented 6 years ago

I want to make sure all credit is given where due, so I'll add your names to our page, if that's OK with ya'all. Please let me know if I've missed anyone, and thanks again to you all. @eXigentCoder @chrisverwey @fronbow

Ezriilc commented 6 years ago

@fronbow , I'm still a noob at GIT, so I'm glad to get advice. Is merging it the simpler way to go? I think I'm fine with that, or what do you recommend?

chrisverwey commented 6 years ago

Fine by me. Just glad I could help.

fronbow commented 6 years ago

@Ezriilc As far as I can see, it should be ok to merge. Ok with me, luckily this username is my username everywhere, even on the KSP forums!

chrisverwey commented 6 years ago

So I ran through a few tests this morning on Kerbin. The movement now all looks pretty neat. The only gripe I have is that the altitude seems to be around 1,000 too low on every jump. If you try to teleport to below 1,000 you get the message that you will be below the terrain (which is at least better than exploding). This even happens over the ocean. If I set my altitude to 1,100 then I end up at 100m.

fronbow commented 6 years ago

Hi @chrisverwey can you give me some planets+lander data to test this. I have a feeling I know where the below sea level thing is (I remember commenting the code!!)

chrisverwey commented 6 years ago

@fronbow I put in Lat 0, Long 0, Alt 1000 on Kerbin and hit Land and it positions me nicely over the water, but at altitude 158m. Funny enough if I load the coordinates for KSC runway (-0.04, 285.28, 20m) and hit land then I end up at 90m above the runway. If I adjust that to 2000m above the runway and hit land, then I go to exactly 2000m above the runway. If I put in Lat 0, Lat 0, Alt 2000 and hit land, then I arrive at 1158m above 0,0. Something is not happy.

fronbow commented 6 years ago

Cheers for this, I'll test it out later and see what's confusing the calculations!

Ezriilc commented 6 years ago

The altitude issue has been a thing for quite a while, actually. For instance, I believe the way it records the CURRENT position is different from how it sets it. If you click Current, then immediately land, I think it ends up higher than it was. @fronbow , if you're gonna look into this, then shall I wait to put up a BETA? I know peeps are eager for this to get done. Oh yea, and THANKS to everyone.

fronbow commented 6 years ago

I would keep the peoples happy and release a beta with the caveat that we're still ironing out some glitches - but it works better than the last released build!! I'll continue to look into this :) and start learning how the unity gui elements work when we've squished this!!

PS: At least we're landing at the correct coords now.

Ezriilc commented 6 years ago

Done. Since this is up now, there's no big hurry to get the remaining altitude thing fixed.

fronbow commented 6 years ago

Yes, for the moment it looks like if you want to land anywhere not near the surface, you can't. This could be a "hidden feature" in that you should really use the orbit tool if you don't want to land lol.

wile1411 commented 6 years ago

Height issue seems to be only when over water. Above land the height seems to get set accurately.

I'd say the height is being fixed to the planet surface, even when it's underwater. Is it possible to add some logic to set the height to be from the sea level if over water?

wile1411 commented 6 years ago

For the Loading of Co-ordinates, I don't think the co-ordinates used are fixed to the planet surface. If I load up a set of Co-ordinates over land, it works fine. If I time warp a day or so, the same co-ordinates will be somewhere else on the planet (possibly over water) as the planet rotates away from desired coordinate location.

I suspect there is some extra transform maths needed to get the desired location to be aligned with the planets surface rotation.

fronbow commented 6 years ago

@wile1411 one of the reasons I stepped in to try and help! was that in the previously released build the coords were seemingly random, and the altitude you ended up at invariably involved crashing below the surface!! From the digging we've been doing, Squad changed how they calculate stuff which changed how the api functions work. When I'm testing my builds, I'm using KSP Engineer to tell me the coords and ASL/AGL.

fronbow commented 6 years ago

So I decided to do some checking of @wile1411 's Height bug. I seemed to have fixed it in my builds so I'm releasing this beta for testing: http://www.Kerbaltek.com/hyperedit I've changed the versioning so that it stays in sync with the existing versions on kerbaltek! I would appreciate it if people test using this one please :)

Ezriilc commented 6 years ago

While all you awesome people are working on this, I've put our .version file back, in order to keep CKAN from thinking we're done with the full release.

chrisverwey commented 6 years ago

@fronbow that link you pasted above to BETA-3.dll doesn't work. Can we get a copy of the latest build plz for testing.

403 Forbidden! Sorry, no direct access.

chrisverwey commented 6 years ago

@fronbow Never mind. I managed to grab it from the kerbaltek.com downloads page. It seems you weren't "allowed" to just paste the link over here.

Ezriilc commented 6 years ago

"403 Forbidden! Sorry, no direct access." means that you're trying to hotlink to the file, or your browser is misbehaving - please turn off your plugins or try a different browser.

EDIT: I just noticed that fronbow had put a direct link - yea, that doesn't work. I'm a bit of a control freak, I guess.

I've put fronbow's latest build ("BETA-3") in our "Experimentals" section on our site: http://www.Kerbaltek.com/hyperedit

fronbow commented 6 years ago

Apologies, I wasn't aware kerbaltek had hotlinking disabled. (I'll remember that for the future!!)

Ezriilc commented 6 years ago

Lemme know if it becomes a problem, I have some other magic doings to do if we need. ;) EDIT: I fixed your comment.

chrisverwey commented 6 years ago

Thanks yes, I got it from the experimental page. I was doing some tests just now on Kerbin and the Mun. If I test jumping to say 0,0,2000m I end up at 2000m as expected over the water, but if I jump to 10,-100, 2000m I arrive at 2215m. I happened to glance at Flight Engineer which tells me that I'm exactly 2000m above terrain. I think that might be our problem with the altitude and it only shows up when you're not over water.

fronbow commented 6 years ago

@chrisverwey yeah it's a sneaky little bug this one! More testing ;) thanks for the input data. Is your Engineer coords matching up with ours? (on the HyperEdit Lander window)

wile1411 commented 6 years ago

didn't think to check the engineer Coords to the one in Hyper Edit. I'll download the latest version to test after work. The comment from chrisverwey sounds like the ALT part of the issue is fixed as I was kind of expecting the ALT to be over terrain and not a fixed ALT.

Having said that, I did notice that isn't not 100% perfect at preventing spawning below the surface as I managed to get a craft to explode once, but I'll need to recheck to see what I exactly did for reproduction steps.

What do you do when you detect someone having a Altitude that's below the surface? Do you add a fixed amount or do you set a Fixed safe if altitude?

Hoping that all that's left is the Coords issue. :)

wile1411 commented 6 years ago

Actually if it helps, when I first noticed the moving landing location issue, I figure the location had changed with a game update or something. Soooo.... I went and updated all my Default landing locations (KSC Launch pad, runway, etc...) What I noticed was that the LON value was all I really needed to correct to get the location that was intended.

Maybe the issue is just in the how the LON value is applied when spawning. LAT seemed to be always correct from what I could tell.

fronbow commented 6 years ago

@wile1411 I knew Engineer worked so I used that to test against! Yeah HyperEdit has always had a tendency to crash, as long as I've been using it! I'm calculating the actual terrain height underneath the vessel. Then after some checks, just add the entered Alt - though if you enter 0 you get a nice spawn thru terrain that corrects itself ;)

chrisverwey commented 6 years ago

@fronbow yes, the Lat/Long in Engineer matches just fine. I think that part of the problem is working now. As for the altitude, if the spec says "altitude above terrain" then I'm happy that it's working 100%. I never really noticed how the altitude worked before - I just assumed it was absolute altitude. Either way I'm happy. Exact lat/long is way more important than exact altitude.

wile1411 commented 6 years ago

@chrisverwey can you try land in a location (leave Landing screen open) wait for craft to actually land, time warp to next morning and hit LAND again? Does that put you in the same location with the new file?

fronbow commented 6 years ago

@chrisverwey As far as I know that's a problem with KSP's altitude dial - it always shows Above Sea Level regardless of what's underneath you! FWIW I've set my Engineer readout to show me both Altitude (Terrain) and Altitude (Sea Level). I'm currently at 10, -100, 20 - I spawned at 10, -100, 40ish. AGL = 362cm (Above Ground Level) ASL = 20m (Above Sea Level)

HTH

Ezriilc commented 6 years ago

var HyperEditsTendencyToCrash = true; Well, the plugin itself doesn't crash; however, the ships undergo an un-commanded rapid disassembly.

fronbow commented 6 years ago

The current version of the beta is 1.5.5.1 (Windows Explorer should tell you this) ;) It will resume normal ordering next compile which I'm thinking might be a release. Fingers crossed.

chrisverwey commented 6 years ago

@wile1411 I did the following steps to test

  1. Launch a plane on the runway
  2. Hit "current" button then adjust altitude to 100m
  3. Hit "land" button. The plane lands calmly
  4. Fast forward to the next day, but a different time of day to avoid being in exactly the same place
  5. Hit "land" button. The plane went to the exact same place and landed calmly.

@fronbow I noticed that hitting the "current" button places the asl altitude in the coordinates box, not the agl value. So if you hit "current" and then "land" you end up in a different spot (altitude-wise) from where you started.