Skip to content

Fix NRE in watchdog#5

Open
Clayell wants to merge 5 commits intomainfrom
watchdog
Open

Fix NRE in watchdog#5
Clayell wants to merge 5 commits intomainfrom
watchdog

Conversation

@Clayell
Copy link
Copy Markdown
Collaborator

@Clayell Clayell commented Apr 14, 2026

Consistent NRE would show up when loading into the space center scene, as Settings could be null

[EXC 19:58:21.615] NullReferenceException: Object reference not set to an instance of an object
	AdvancedPQSTools.WatchDogs.Update () (at <9132a6a28a214f5b8e0a2d965abd7ef8>:0)
	UnityEngine.DebugLogHandler:LogException(Exception, Object)
	ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
	UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)

Sidenote:
What is this supposed to be doing?

foreach (ConfigNode node in GameDatabase.Instance.GetConfigNodes("ADVANCEDPQSTOOLS"))
      Settings = node;

If it's supposed to be finding the first node, why not use FirstOrDefault?

@Phantomical
Copy link
Copy Markdown

foreach (ConfigNode node in GameDatabase.Instance.GetConfigNodes("ADVANCEDPQSTOOLS"))
      Settings = node;

As a note, that actually seems to be finding the last settings node. Is that intentional?

@Clayell
Copy link
Copy Markdown
Collaborator Author

Clayell commented Apr 14, 2026

I also noticed that, which is why I didn't remove it. Seems to be incredibly fragile behavior if intended.

@ballisticfox
Copy link
Copy Markdown
Collaborator

That appears to be carried over from the original RealSolarSystem source:
https://github.com/KSP-RO/RealSolarSystem/blob/a92a04972f68d48c7f83c24f47ff6aa96ba1a6f4/Source/Watchdogs.cs#L20-L27

@Clayell
Copy link
Copy Markdown
Collaborator Author

Clayell commented Apr 19, 2026

Is anything in this file actually needed for AdvancedPQSTools? It all seems RSS-specific, if it isn't actually useful/needed it would probably be best to remove it.

@ballisticfox
Copy link
Copy Markdown
Collaborator

Is anything in this file actually needed for AdvancedPQSTools? It all seems RSS-specific, if it isn't actually useful/needed it would probably be best to remove it.

From my understanding of what this code does, which is resetting flight states and near clip/far clip plane camera settings in the event of another mod changing something, I don’t see how that is incredibly RSS specific nor unneeded

@Clayell
Copy link
Copy Markdown
Collaborator Author

Clayell commented Apr 21, 2026

I just checked, and AdvancedPQSTools doesn't even contain the config that would make this work.
The equivalent RSS config is here, but AdvancedPQSTools never brought it over.
That file has a comment noting that it is Custom settings for RealSolarSystem. Here is the commit history for those configs:
KSP-RO@079a5f2
KSP-RO@3c45a66
KSP-RO@59059ab
I could ask siimav what exactly they mean.

@ballisticfox
Copy link
Copy Markdown
Collaborator

ballisticfox commented Apr 23, 2026

@Clayell
Copy link
Copy Markdown
Collaborator Author

Clayell commented Apr 23, 2026

Ah, so they are.

As a note, that actually seems to be finding the last settings node. Is that intentional?

Should we fix this? I can make it instead look for the ADVANCEDPQSTOOLS node with a ClipPlanes node in it.

@ballisticfox
Copy link
Copy Markdown
Collaborator

Yeah that'd be ideal

@Clayell
Copy link
Copy Markdown
Collaborator Author

Clayell commented Apr 26, 2026

Done, ready for merge

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.

3 participants