Improved RP-1 tech progression#2955
Conversation
Dunbaratu
left a comment
There was a problem hiding this comment.
I think I may have found why when I tested this on an existing RP-1 installation it caused the problems.
| { | ||
| try | ||
| { | ||
| FindRP1Modules(); |
There was a problem hiding this comment.
If FindRP1Modules() threw an exception (see other comment above in this review), then the rest of OnStart() would get skipped, causing all kinds of stuff to be uninitialized.
src/kOS/Module/kOSProcessor.cs
Outdated
| SafeHouse.Logger.Log(string.Format(" techNodeField type={0}", techNodePropInfo.PropertyType.Name)); | ||
| var techNodeProp = techNodePropInfo.GetValue(RP1AvionicsModule); | ||
| var diskSpaceField = techNodeProp.GetType().GetField("kosDiskSpace"); | ||
| SafeHouse.Logger.Log(string.Format(" techNodeField kosDiskSpace={0}", diskSpaceField.GetValue(techNodeProp).ToString())); |
There was a problem hiding this comment.
I don't see a null protect check on this use of diskSpaceField.GetValue() like you have elsewhere in this PR for these new fields. (So if using an older RP-1 that doesn't have your PR in it, I think this line would throw a nullref exception.)
Changes RP-1 kOS cores to have tech progression that follows the user selected avionics tech. This requires changes in RP-1 in addition for correct behaviour, but should be safe to merge before that as it will do nothing if the changes are not present.
ab8c5bd to
f08af0a
Compare
|
The exception was in some debug logging code that isn't really required ironically. I have removed it, so it should be more robust now. |
|
The required changes in RP-1 have now been released, so that should make testing this a little easier. |


Changes RP-1 kOS cores to have tech progression that follows the user selected avionics tech.
This requires changes in RP-1 in addition for correct behaviour, but should be safe to merge before that as it will do nothing if the changes are not present.