Skip to content

fix science transmission for KSP 1.0.5#524

Merged
d4rksh4de merged 6 commits intoRemoteTechnologiesGroup:developfrom
lamont-granquist:lcg/fix-1.0.5-sciene
Apr 10, 2016
Merged

fix science transmission for KSP 1.0.5#524
d4rksh4de merged 6 commits intoRemoteTechnologiesGroup:developfrom
lamont-granquist:lcg/fix-1.0.5-sciene

Conversation

@lamont-granquist
Copy link
Copy Markdown
Contributor

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

Note that I'm not setup to build this module right now (I'm on a mac and don't quite grok mono), but this is the patch everyone has been saying they've been applying which fixes these issues.

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

paging @Peppie23 ?

@Ghoughpteighbteau
Copy link
Copy Markdown

+1 for this PR. Seems like a simple fix.

@lamont-granquist Fancy Mouse said he had to do a silly hack for a NullReferenceException. I don't see that in this pull request, is that relevant?

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

no don't know anything about any NRE fix...

@Ghoughpteighbteau
Copy link
Copy Markdown

haha, wtf.

that's sketchy. Are you just randomly changing things? were you able to identify the NRE?

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

its got something to do with the last fractional bit of science transmission, so if we just don't do that -- then no NRE -- which is almost certainly wrong, but the bug becomes lolwut rather than game breaking...

@@ -91,7 +91,7 @@ private IEnumerator Transmit(Callback callback = 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.

In FancyMouse's fix, line 75-77 changed to:

scienceDataQueue.RemoveAt(0);
scienceData.triggered = true;
var subject = ResearchAndDevelopment.GetSubjectByID(scienceData.subjectID);
if (subject == null)
subject = new ScienceSubject("", "", 1, 0, 0);
int packets = Mathf.CeilToInt(scienceData.dataAmount / PacketSize);

This seems to be the quirky NRE hack they were talking about.

@langleyVa
Copy link
Copy Markdown

I am currently having the issue with survey data not being transmitted while ScienceAlert is not installed. If this PR fixes the issue, I'd love to see it go through. I diffed FancyMouses version of ModuleRTDataTransmitter.cs to the one in the repo and annotated the PR with what exactly they did.

Sadly, I don't know the code enough to tell whether that is a good fix or not, but hope that collecting that information helps the discussion here.

This is the NRE I found in my KSP.log:

[LOG 12:51:09.893] Sending data to vessel comms. 1 devices to choose from. Will try to pick the best one
[ERR 12:51:09.893] [R&D]: No Science Subject found with id survey@Minmus
[LOG 12:51:11.332] RemoteTech: Changing RnDCommsStream timeout from 0.3 to 0.3200684
[LOG 12:51:11.332] RemoteTech: [Transmitter]: Uploading Data... (Orbital Survey Data) - 6.67 Mits/sec. Packets to go: 5 - Files to Go: 0
[LOG 12:51:12.161] RemoteTech: [Transmitter]: Uploading Data... (Orbital Survey Data) - 6.67 Mits/sec. Packets to go: 4 - Files to Go: 0
[LOG 12:51:12.515] RemoteTech: [Transmitter]: Uploading Data... (Orbital Survey Data) - 6.67 Mits/sec. Packets to go: 3 - Files to Go: 0
[LOG 12:51:12.862] RemoteTech: [Transmitter]: Uploading Data... (Orbital Survey Data) - 6.67 Mits/sec. Packets to go: 2 - Files to Go: 0
[LOG 12:51:13.202] RemoteTech: [Transmitter]: Uploading Data... (Orbital Survey Data) - 6.67 Mits/sec. Packets to go: 1 - Files to Go: 0
[LOG 12:51:13.551] RemoteTech: [Transmitter]: Uploading Data... (Orbital Survey Data) - 6.67 Mits/sec. Packets to go: 0 - Files to Go: 0
[EXC 12:51:13.551] NullReferenceException: Object reference not set to an instance of an object
ResearchAndDevelopment.GetScienceValue (Single dataAmount, .ScienceSubject subject, Single xmitScalar)
ResearchAndDevelopment.SubmitScienceData (Single dataAmount, .ScienceSubject subject, Single xmitScalar, .ProtoVessel source, Boolean reverseEngineered)
RnDCommsStream.submitStreamData (.ProtoVessel source)
RnDCommsStream.StreamData (Single dataAmount, .ProtoVessel source)
RemoteTech.Modules.ModuleRTDataTransmitter+c__Iterator0.MoveNext ()
[EXC 12:51:13.976] NullReferenceException: Object reference not set to an instance of an object
ResearchAndDevelopment.GetScienceValue (Single dataAmount, .ScienceSubject subject, Single xmitScalar)
ResearchAndDevelopment.SubmitScienceData (Single dataAmount, .ScienceSubject subject, Single xmitScalar, .ProtoVessel source, Boolean reverseEngineered)
RnDCommsStream.submitStreamData (.ProtoVessel source)
RnDCommsStream+�
.MoveNext ()

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

certainly looks like a better fix than my hacktastic one. updated this PR with FancyMouse's version.

@NathanKell
Copy link
Copy Markdown
Contributor

@Peppie23 @erendrake @Starstrider42 @ArSn anything moving here? :) Or indeed RT generally?

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

not lately...

@rsparkyc
Copy link
Copy Markdown
Contributor

Please don't tell me RT is dead... Maybe since there was talk about KSP 1.1 adding in these sorts of features (which now has been put off to a later version) this is no longer being maintained?

@erendrake
Copy link
Copy Markdown
Member

I hadn't heard that the feature was pushed off 1.1? I should boot RT up and see if we can get some forward motion in here :)

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

Its also highly likely that the KSP stock version of communication will be a bit dumbed down compared to RT and most likely there will still be RT users that want the old RT experience...

The aero model in stock didn't make FAR go away, and the resource model in stock didn't make Karbonite/Regolith go away.

Be super nice to get this bug fixed one way or another and make a patch release.

@rsparkyc
Copy link
Copy Markdown
Contributor

I agree, I don't recall if the stock version will have signal delay. This shows relay networks being delayed till 1.2, which points to this blog post, but for the life of me I can't find it. Edit: first sentence talks about it being delayed.

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

I know for certain I read it in one of the devnotes tuesday-wednesdays that it was cut out of 1.1

@lamont-granquist
Copy link
Copy Markdown
Contributor Author

Here found the right thread with the announcement that new comms had been cut from 1.1:

http://forum.kerbalspaceprogram.com/index.php?/topic/131399-no-new-comms-system-in-11/

@ArSn
Copy link
Copy Markdown
Member

ArSn commented Mar 22, 2016

@NathanKell There is not much going on here atm indeed.

Granted, I did not contribute a lot to this project (yet), but I feel like I should give my 2 cents in the discussion here of why that is.

I had a lot of fun working on the issues that were around when there was no word at all of the relay system being integrated in stock KSP. However, as some people here already suggested, the notice of this system being added in 1.1 really took the wind out of our sails (or at least mine, that is).
It is unfortunate that it has been delayed to after 1.1, but that decision is not too old now.

Still, the problem at hand is, that one just does not know how much of it will be integrated in stock, and therefore, one does also not know if RT will be dead after the update which includes the relay system in stock (whether that is 1.2, 1.3 or anything else).

I would be happy to spin up my efforts in this project again, if there is actually an audience that likes to keep it, and also, if it will actually still make a difference to the stock game. I know you are not a psychic either, and thus can not promise what the future will bring, however, for the people contributing to RT, I imagine it to be crucial information, whether, when, and how much of RT is going to be integrated in stock, and thus, I would highly appreciate it if there would be a more in-detail explanation of those plans.

@NathanKell
Copy link
Copy Markdown
Contributor

@ArSn the stock feature is not going to be RT-like, AFAIK, except to the extent that relay networks are involved. There certainly won't be a flight computer, and probably not signal delay either; my guess is just that certain functions (but not all functions) will be locked out when off the grid. (I've been busy in my own dev work, haven't had a chance to play at all with that branch; though NDA would not let me say much if I had, either, sadly).

I can state for a fact that Realism Overhaul (and its entire user base) will still be wanting to use RemoteTech going forward, and my guess is many others will as well ( @lamont-granquist 's comparison to stock aero vs FAR will be very apt, I think).

@Ghoughpteighbteau
Copy link
Copy Markdown

I think if you're going to be putting your money on what the future will look like, then put your money on stock communications being as friendly as possible. That has been the Squad modus operandi for everything. Planes that worked in the updated aerodynamics still flew into the ground in FAR.

Remote tech is a cruel cruel mod, an aspect Squad will never copy. There's always going to be a market for masochism in KSP. So... I wouldn't worry about it.

Do you need some third parties to test this PR? I can get a dev environment set up if need be.

@langleyVa
Copy link
Copy Markdown

@BobPalmer, you implement the stock relay network features, right? Can you drop a few words on whether RemoteTech will still have it's place or nah?

@Starstrider42
Copy link
Copy Markdown
Contributor

Maybe I should say something since I got @mentioned. 😉

I think the main problem has been that none of the developers have been available for the last few months. I'm finding it hard just to keep my own mod going (this week marked the first time in two months that I did any KSP modding), let alone a project as demanding as RemoteTech. I'm pretty sure that Peppie23 is in the same ignore-everything-KSP limbo that I spent a year in, as I tried contacting him when I got back but never got a reply.

That said, I was pleasantly surprised to see Erendrake's response above (and I have no idea who ArSn is, but belated welcome to the RTG). Hopefully they'll be able to fix at least the most frustrating bugs. I'm afraid I can't commit even to taking up my old job with the RT manual, let alone actual development. I'll let you guys know if things change.

@Ghoughpteighbteau
Copy link
Copy Markdown

If the current remote tech devs are not feeling up to maintaining, that's fine! You're not slaves, but you should consider promoting some enthusiastic fans. Take advantage of the good old open source model.

@tomekpiotrowski
Copy link
Copy Markdown
Member

@d4rksh4de Is this whole science transmission issue (reported here #511) still a problem in KSP 1.1?

@d4rksh4de
Copy link
Copy Markdown
Member

@tomekpiotrowski Haven't checked that.

@tomekpiotrowski tomekpiotrowski added this to the 1.6.10 milestone Apr 8, 2016
@tomekpiotrowski
Copy link
Copy Markdown
Member

I will add this to the milestone for now. If it was an issue in 1.0.5 there's a good chance it's still an issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

10 participants