untoldwind / KontrolSystem2

Autopilot scripting system for KSP2
Other
54 stars 14 forks source link

Distances calculated from orbit.global_position() are incorrect #116

Closed appenz closed 5 months ago

appenz commented 5 months ago

I am calculating distances between two targetables x1 and x2 at a future time t by:

const p1 = x1.orbit.global_position(t)
const p2 = x2.orbit.global_position(t) 
const distance = p1.distance(p2)

If I understand correctly, this should give the correct distance for any future time t as long as both orbits are regular (i.e. have Apoapsis, no SOI change, no maneuver node). This is not the case.

As an example distance from Kerbin to Mun should be constant as Mun's orbit is circular. If measured like this, distance increases by about 800,000 km per day (which is much larger than Mun's orbit).

Sample code on GitHub to reproduce is in this repo.

The error appears for both two targetables orbiting the same body as well as for targetables orbiting different bodies. It also appears wrong for both vessels and bodies.

untoldwind commented 5 months ago

This should be as intended in the 0.5.2.4 release. The orbit.global_velocity method might need a bit more testing.

For compatibility I also added orbit.global_relative_position(ut) == orbit.global_position(ut) - orbit.reference_body.global_position(ut)

appenz commented 5 months ago

Thank you, I'll verify this week.

untoldwind commented 5 months ago

After some further testing I discovered that the global_velocity is using the wrong frame of reference. Will be fixed in the next patch.

appenz commented 5 months ago

Looks like this was fixed and the unit test I had written now shows correct results. You can probably close it.

I can't fully test this as part of my overall maneuvers as I am currently blocked on the global_velocity bug.

untoldwind commented 5 months ago

I checked with 0.5.2.5: This here returns a consistent result:

use { Vessel } from ksp::vessel
use { CONSOLE } from ksp::console
use { galactic_origin, Orbit } from ksp::orbit
use { current_time } from ksp::game

pub fn main_flight(vessel: Vessel) -> Result<Unit, string> = {
    CONSOLE.clear()

    const ut = current_time()
    const current_local = vessel.position
    const current = vessel.global_position

    CONSOLE.print_line("CurrentL: " + current_local.to_fixed(1))
    CONSOLE.print_line("CurrentG: " + current.to_local(vessel.main_body.celestial_frame).to_fixed(1))
    CONSOLE.print_line("CurrentB: " + current.to_local(galactic_origin()).to_fixed(1))

    CONSOLE.print_line("VCurrentL: " + vessel.orbital_velocity.to_fixed(1))
    CONSOLE.print_line("VCurrentG: " + vessel.global_velocity.to_local(vessel.main_body.celestial_frame).to_fixed(1))
    CONSOLE.print_line("VCurrentB: " + vessel.global_velocity.to_local(galactic_origin()).to_fixed(1))

    check_tree_p(ut, vessel.orbit)
    check_tree_v(ut, vessel.orbit)

    for(i in 0..10) {
        const time = i * 100.0
        check_tree_p(ut + time, vessel.orbit)
        check_tree_v(ut + time, vessel.orbit)
    }
}

fn check_tree_p(ut: float, orbit: Orbit) -> Unit = {
    CONSOLE.print_line("OB: " + orbit.global_position(ut).to_local(galactic_origin()).to_fixed(1))

    let p = orbit.relative_position(ut)
    let body = Some(orbit.reference_body)
    while(Some(current) = body) {
        p += current.orbit.relative_position(ut)
        body = current.parent_body
    }
    CONSOLE.print_line("P : " + p.to_fixed(1))
}

fn check_tree_v(ut: float, orbit: Orbit) -> Unit = {
    CONSOLE.print_line("VB: " + orbit.global_velocity(ut).to_local(galactic_origin()).to_fixed(1))

    let v = orbit.orbital_velocity(ut)
    let body = Some(orbit.reference_body)
    while(Some(current) = body) {
        v += current.orbit.orbital_velocity(ut)
        body = current.parent_body
    }
    CONSOLE.print_line("V : " + v.to_fixed(1))
}

I.e. the global_position/_velocity in the galatic_origin frame is the the sum of all the relative vectors of the orbit-hierarchy - of course assuming that Kerbol is not moving at (0,0,0), which might change when/if other solar systems are added.

DimiBD commented 5 months ago

I just test the maneuvers for auto Minmus, it's working fine !

appenz commented 5 months ago

Ran a few test cases and this bug appears to be fixed.

I tested: