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

#1665 clarify Orbit:MEANANOMALYATEPOCH docs #1696

Closed
wants to merge 1 commit into from
Closed

#1665 clarify Orbit:MEANANOMALYATEPOCH docs #1696

wants to merge 1 commit into from

Conversation

sshilovsky
Copy link

for the sake of other unaware rocket scientists

for the sake of other unaware rocket scientists
traditionally done in radians, in keeping with the kOS standard
of making everything into degrees, they are given as degrees by
kOS.
kOS. Orbit epoch is a complex term, refer to `#1665<https://github.com/KSP-KOS/KOS/issues/1665>`_ for details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I like the idea of pointing people at the github issue about the problem, since it will also contain confusing information about what the mod used to be like...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. It would be better if the documentation itself explained the concept of epoch. I'm not sure that we've actually fixed anything related to this though. @Dunbaratu were you going to add a suffix to handle the epoch issue in the review of #1660?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's just better than nothing. Might be reasonable to just notice that orbit epoch is not the same that the universe epoch. A link to KSP docs could be also helpful and better than to github issue.

I think something has to be done here. I'd lost a couple hours trying to catch a bug related to this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is probably no reason to describe the concept of epoch right now, but let #1665 fix to address that.

This PR is just to save others efforts before #1665 is fixed.

@Dunbaratu
Copy link
Member

@sshilovsky : The problem with the notion of putting in a temporary change to the docs until such a time as the #1665 is fixed.. is that the docs get published officially at the same time as the full releases do. So if we assume #1665 is in the next release, there won't be any window of time during which the edit you talked about would help.

The edit we make should be the edit that makes the docs look like what we want them to look like after issue #1665 gets finished and closed. By the time the documentation change becomes public, that's what the situation will be.

@hvacengi : I'm stalled on #1665 because @dewiniaid last said he was still going to mess with it a bit more, and I didn't want to go any further on it until I heard more about that. If he's not planning on doing it, then I may take over testing the problem and seeing what can be done about it. I do think we need something to make "find current position on orbit" more sane. At the moment the "mean anomaly at epoch" value is useless without knowing the epoch timestamp, which keeps on changing a LOT as the game progresses. I haven't figured out yet when and why it changes, bit it changes a lot. The game seems to very frequently recalc the Euclidian parameters for the orbit object, and each time it does so it seems to move the epoch again.

@Dunbaratu
Copy link
Member

Oh, I didn't see that #1665 was moved to v1.0.1 rather than v1.0.0. In that case there might be a need for a temp fix in the docs?

@dewiniaid
Copy link

@Dunbaratu err, I'm not an actual kOS developer and am not working on #1665, just a highly opinionated and technical user. Apologies if anything implied otherwise.

@Dunbaratu
Copy link
Member

I misspoke. I mean the person working on #1660 - @lamont-granquist

@Dunbaratu Dunbaratu added this to the v1.1.0 milestone May 3, 2017
@Dunbaratu
Copy link
Member

Added to Milestone v1.1.0 because the issue this is connected to is in milestone V1.1.0

@Dunbaratu Dunbaratu removed this from the v1.1.0 milestone Jun 12, 2017
@hvacengi
Copy link
Member

hvacengi commented Aug 3, 2017

It looks like we added the epoch suffix in #2047 including documentation. As such, I'd like to close this PR as the stop-gap documentation is no longer needed.

@hvacengi hvacengi closed this Aug 3, 2017
@sshilovsky sshilovsky deleted the patch-1 branch August 5, 2017 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants