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

TR Orbit and Impact velocity additions #1

Closed
wants to merge 4 commits into from

Conversation

PBscoots
Copy link

I think I figured out how to point this at you @CalebJ2. Hopefully you can merge this to your branch and then merge it to the main KOS branch from there before your pull request KSP-KOS#1603 gets closed. I modified the wrapper, the addon, and the documentation. I think that is everything. I am not really familiar with C# so I copied all of your conventions on class naming/variable and function declaration and so on. I am like that "I have no idea what I'm doing" dog meme.
Also, I am unsure if the spaceorbit really needs to say in the KOSexception that you should always check hasimpact in the addon file because it could be possible to have a modified orbit due to aerobreaking but not have an impact. I don't know what else to do for an exception there though because idk if those patch instances will always have orbits in them or not.

I added the space orbit and impact velocity to the wrapper portion.
I added the necessary bits to the addon file for the space Orbit and impace velocity terms to be pulled through.
I added documentation for the impact velocity and space orbit suffixes
I had something misnamed in the impact velocity so I changed it quick
@PBscoots
Copy link
Author

Sorry to inundate you with posts and comments. But do we need an AssemblyInfo.cs file? its called for in the kos addon readme but I don't see any of the other addons using one. Except for the KOS main developer who is making some kind of camera addon
https://github.com/hvacengi/KOS-StockCamera/tree/master/src/kOS.Addons.StockCamera/Properties

@hvacengi
Copy link

hvacengi commented Sep 20, 2016

@PBscoots I'm in the process of reviewing and updating @CalebJ2's pull request on the main kOS repository. Part of what I'm doing is actually a rebase, because the number of things updated between submission and review make it really tough to isolate the changes for the PR itself, while still being able to run test. When I get that pull request merged, could you submit your revisions to the main repository instead of here?

I will warn you I will also be heavily modifying the documentation and some suffix identifiers.

(There is no need for a separate AssemblyInfo.cs file in this instance, as the addon will be included in the kOS assembly itself. That readme applies to addons external to kOS.)

@PBscoots
Copy link
Author

@hvacengi Yeah sure. I thought I should do the commits here because Caleb had put the addon together and I thought he would want to check the few changes I made and then commit them on up the line.
Its not a problem if you are heavily modifying the documentation. I didn't make many changes all together so if I need to redo them its not a huge deal.

@PBscoots PBscoots closed this Sep 20, 2016
@PBscoots
Copy link
Author

@hvacengi my pull request for the Traj API was merged, so whenever you finish the rebase, I will add my changes to the KOS side and when that is merged and then I can help you test.

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.

2 participants