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

Use Vector3d instead of Vector3 #328

Closed
wants to merge 2 commits into from
Closed

Conversation

sswelm
Copy link
Contributor

@sswelm sswelm commented Oct 26, 2018

by using vector3d instead of Vector3, the calculated distance is much more accurate, resulting in much less instability in the final solar power

by using vector3d instead of Vector3, the calculated distance is much more accurate, resulting in much less instability in the final solar power
@@ -155,10 +155,10 @@ public static Double Flux(ModularFlightIntegrator fi, KopernicusStar star)
// Get sunVector
RaycastHit raycastHit;
Boolean directSunlight = false;
Vector3d scaledSpace = ScaledSpace.LocalToScaledSpace(fi.IntegratorTransform.position);
Vector3d scaledSpace = ScaledSpace.LocalToScaledSpace(fi.Vessel.CoMD);
Copy link
Member

Choose a reason for hiding this comment

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

Is CoMD actually the same as the vessel position in world space? I am not entirely sure, and can't check it at the moment.

Copy link
Contributor Author

@sswelm sswelm Oct 28, 2018

Choose a reason for hiding this comment

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

Yes it is, it gives the Vector3d position of the vessel, which is 64 bit instead 32 bit transform

Sorry, something went wrong.

Double scale = Math.Max((star.sun.scaledBody.transform.position - scaledSpace).magnitude, 1);
Vector3 sunVector = (star.sun.scaledBody.transform.position - scaledSpace) / scale;
Ray ray = new Ray(ScaledSpace.LocalToScaledSpace(fi.IntegratorTransform.position), sunVector);
Vector3d sunVector = (star.sun.position - scaledSpace) / scale;
Copy link
Member

Choose a reason for hiding this comment

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

This change would have serious implications on the calculation, because you are replacing a scaled space position with a local space one (scaled space = local space / 6000).

Sorry, something went wrong.

Copy link
Contributor Author

@sswelm sswelm Oct 28, 2018

Choose a reason for hiding this comment

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

Possibly, but perhaps we should only use scale space position when it is actually relevant, by default you realy don't need it. My gut feeling sais that if it is relevant, you are better of calculating it manually by multiplying COMD by the scale but I'm not sure. Either-way should aim for maximum precision at all times

@StollD StollD closed this Dec 30, 2018
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.

None yet

2 participants