Skip to content

isHome =/= is a ground station, add eventvoid hook to signal when the network updates#70

Merged
siimav merged 14 commits into
KSP-RO:masterfrom
periodically-makes-puns:even-more-skopos-fixes
May 6, 2026
Merged

isHome =/= is a ground station, add eventvoid hook to signal when the network updates#70
siimav merged 14 commits into
KSP-RO:masterfrom
periodically-makes-puns:even-more-skopos-fixes

Conversation

@periodically-makes-puns
Copy link
Copy Markdown

Fixes for Skopos, and the hook needed for mockingbirdnest/Skopos#70.

This formally distinguishes between "ground stations" and "homes". Checks that appear to apply to all ground stations (and not just "homes") now check the isGroundStation flag, which checks if ParentBody is not null.

Two non-trivial usages of CommNode.isHome are currently left unmodified:

  • RealAntenna.LoadFromConfigNode: I am not confident that the ParentNode is fully initialised when we call this function.
  • RATools.HighestGainCompatibleDSNAntenna: The function name appears to imply it is intended for "home" antennas, though it is currently unused in the code (?).

I don't think replacing the isHome usages in Physics.cs actually changed any calculations? When I checked a link with low antenna elevation and high beamwidth (GEO sat to Bering Sea ship), it had no impact on the calculated noise temperature.

Comment thread src/RealAntennasProject/PlannerGUI.cs Outdated
Comment thread src/RealAntennasProject/Physics.cs Outdated
// Home Stations are directional, but treated as always pointing towards the peer.
float allbody = rx.ParentNode.isHome ? AllBodyTemps(rx, origin - rx.Position) : AllBodyTemps(rx, rx.ToTarget);
// Tracking antennas always point towards the peer.
float allbody = rx.IsTracking ? AllBodyTemps(rx, origin - rx.Position) : AllBodyTemps(rx, rx.ToTarget);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please ensure that all standard (non-Skopos) ground stations will always have isTracking == true

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, it'd be false for the ones that are weak enough to be considered Omni antennas. For all the non-omni antennas associated with ground stations, though, either isHome is defined with no target (so it is considered isTracking), or it's a Skopos station manually defining an empty TARGET {}.

@periodically-makes-puns
Copy link
Copy Markdown
Author

The isTracking condition is just that the antenna isn't Omni and the Target is null. Is it correct to assume that omni antennas "point" towards their targets? I will admit I am not too familiar with the physics behind omni antenna.s

Comment thread src/RealAntennasProject/RealAntenna.cs Outdated
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)
else if (ParentNode is RACommNode RANode && RANode.isGroundStation) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we move this condition to SetDefaultTarget instead (and make the default target TARGET{} for ground stations)?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think ParentNode being null is a sign something is broken. Can't participate in the network if you ...don't have a node in the network. Yeah, a validity check probably goes in SetDefaultTarget.

Comment thread GameData/RealAntennas/RealAntennasCommNetParams.cfg Outdated
Comment thread GameData/RealAntennas/RealAntennasCommNetParams.cfg Outdated
Comment thread src/RealAntennasProject/RealAntenna.cs Outdated
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)
else if (ParentNode is RACommNode RANode && RANode.isGroundStation) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think ParentNode being null is a sign something is broken. Can't participate in the network if you ...don't have a node in the network. Yeah, a validity check probably goes in SetDefaultTarget.

Comment thread src/RealAntennasProject/Physics.cs Outdated
Comment thread GameData/RealAntennas/RealAntennasCommNetParams.cfg Outdated
@DRVeyl
Copy link
Copy Markdown

DRVeyl commented May 2, 2026

RealAntenna.LoadFromConfigNode: I am not confident that the ParentNode is fully initialised when we call this function.

Need to figure this first one out.

RATools.HighestGainCompatibleDSNAntenna: The function name appears to imply it is intended for "home" antennas, though it is currently unused in the code (?).

It's for sorting the planner window IIRC. Change this one to isGroundStation, there's no reason it should necessarily compute for a science destination. (It's a different sort of awkward to make a super strong Skopos station that won't accept science comms, but that's a distinction for another time.)

@DRVeyl
Copy link
Copy Markdown

DRVeyl commented May 2, 2026

Please review all Physics uses of ParentBody != null eg (rx.ParentNode is RACommNode rxNode && rxNode.ParentBody != null) as these are equivalent to isGroundStation and should be changed?

@periodically-makes-puns
Copy link
Copy Markdown
Author

Antenna planning is broken. Looking into it.

@periodically-makes-puns periodically-makes-puns marked this pull request as draft May 3, 2026 04:24
@periodically-makes-puns periodically-makes-puns marked this pull request as ready for review May 3, 2026 05:32
Comment thread src/RealAntennasProject/RealAntenna.cs Outdated
Comment on lines +167 to +168
if (!(ParentNode is RACommNode))
Debug.Log($"{ModTag} {ParentNode?.displayName} is not an RA comm node but has a RealAntenna! Defaulting target for {Name} to null");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this should refactor slightly. If you somehow hit this condition, scream loudly and ... set Target to null?

if (!(ParentNode is RACommNode raNode)) 
  Target = null;
else if (raNode.isGroundStation)
  Target = null;
else {
  ...
}

Copy link
Copy Markdown

@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.

With this minor exception, this looks good.

@siimav siimav merged commit a40a83f into KSP-RO:master May 6, 2026
3 checks passed
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