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

Various changes made to support Σκοπός over the past few years #1

Merged
merged 13 commits into from
Jun 1, 2024

Conversation

eggrobin
Copy link
Collaborator

@eggrobin eggrobin commented May 5, 2024

No description provided.

Copy link

@DRVeyl DRVeyl left a comment

Choose a reason for hiding this comment

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

See 2 comments, everything looks probably-good.

=> (ant.CanTarget && ant.ToTarget != Vector3.zero) ? PointingLoss(Vector3.Angle(origin - ant.Position, ant.ToTarget), ant.Beamwidth) : 0;
=> (ant.CanTarget && !ant.IsTracking) ? PointingLoss(Vector3.Angle(origin - ant.Position, ant.ToTarget), ant.Beamwidth) : 0;
Copy link

Choose a reason for hiding this comment

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

Just be careful here, as the pointingloss for a poorly defined angle (ie direction to target is somehow near 0,0,0) will produce bad results. Unsure how much this particular library code is exercised now, especially with the conversion to jobs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, this won’t affect cases where ToTarget is near zero, only those where it is zero.
It seems unlikely that it would be zero in cases where it should be, since it is computed as difference in double precision where one of the terms is Position, which is PrecisePosition, which is a position in double precision (the other term of the difference, Target.transform.position, is regrettably in single precision, but the result, while perhaps garbage, shouldn’t come out to 0).

(The difference is then converted to single precision but this matters not. All that switching between precisions is neither a recipe for performance nor correctness, but let’s not get distracted.)

Copy link

Choose a reason for hiding this comment

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

Could be irrelevant if calculating garbage data anyway [what is ToTarget with a somehow broken/invalid target?]. Other source of potentially bad result is maybe no longer possible for reason you described [PrecisePosition is double]. Main indicator might be vessels & targets that are extremely far away from current origin [ie, focus on a vessel past Pluto, what are the calculations for vessels/targets in similar orbit near Earth?]

@@ -88,7 +90,7 @@ public struct CalcAntennaBodyNoise : IJobParallelForDefer
[WriteOnly] public NativeArray<float> bodyNoise;
public void Execute(int index)
{
bodyNoise[index] = rxHome[index] ? Precompute.NoiseFromOccluders(rxPos[index], rxGain[index], rxDir[index], rxFreq[index], rxBeamwidth[index], occluders) : rxPrecalcNoise[index];
bodyNoise[index] = rxTracking[index] ? Precompute.NoiseFromOccluders(rxPos[index], rxGain[index], rxDir[index], rxFreq[index], rxBeamwidth[index], occluders) : rxPrecalcNoise[index];
}
Copy link

Choose a reason for hiding this comment

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

I wonder about this particular change. The purpose of this job was to defer the noise calculation for antennas whose pointing was not determined earlier, specifically: the ground station antennas that are allowed to "simultaneously" point at any possible peer (as a gameplay simplification).

But isTracking is also changing, from:

        public virtual bool CanTarget => Shape != AntennaShape.Omni && (ParentNode == null || !ParentNode.isHome);

to:

        public bool IsTracking => Shape != AntennaShape.Omni && Target == null;

I don't know if those are the same, ie do only ground stations have Target == null?
But this is the only purpose of isTracking -- to indicate this non-physical idea of how a given antenna is pointing.

Copy link
Collaborator Author

@eggrobin eggrobin Jun 1, 2024

Choose a reason for hiding this comment

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

To your question

I don't know if those are the same, ie do only ground stations have Target == null?

I think the answer is yes, because this is how Target is initialized, where we see some of the logic that was in CanTarget:

            if (config.HasNode("TARGET"))
                Target = Targeting.AntennaTarget.LoadFromConfig(config.GetNode("TARGET"), this);
            else if (Shape != AntennaShape.Omni && (ParentNode == null || !ParentNode.isHome) && !(Target?.Validate() == true) && HighLogic.LoadedSceneHasPlanetarium)
                Target = Targeting.AntennaTarget.LoadFromConfig(SetDefaultTarget(), this);

The only intended change in behaviour in the changes surrounding the introduction of IsTracking is that you can explicitly configure an antenna to have this kind of track-everything-at-once behaviour using TARGET {}; Σκοπός uses that for its ground stations.

Those ground stations are not home, so that you cannot downlink your science to a random Орбита station (or maybe even someone’s TVRO dish!) using the entire C band.

@eggrobin eggrobin merged commit 55a940d into KSP-RO:master Jun 1, 2024
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