Don't get science for data

[rsparkyc]

I made a simple MM patch:

@PART[probeCoreOcto2]:AFTER[RemoteTech]:FOR[Ryan]
{
    @MODULE[ModuleRTAntennaPassive]
    {
        @TRANSMITTER
        {
            @PacketSize = 0.1
        }
    }
}

I then created a craft using the modified probe core, some batteries, and a thermometer. I tried transmitting science data from the launch pad, and while the transmission completes (slowly, as I would expect), I don't get the science for it. RT https://github.com/RemoteTechnologiesGroup/RemoteTech/releases/tag/1.8.4, KSP v1.2.2. Log file attached. Player.log.zip

[KSP-TaxiService]

@rsparkyc Many thanks for the bug report and the short reproduction steps. However, I (and one member separately) can't reproduce the science-gain bug on my own side, even with the downloaded RT 1.8.4 mod and your MM patch. I am starting to think the cause may be related to the cross-platform differences, given that you are on Mac while I am on Windows.

So this boils down to two questions: 1) Is this bug reproducible on your Mac side? 2) Does your pull request (sending the whole data instead of 0.1f in the last data frame) resolve the bug on your side?

If both are yes, I have no opposition to the pull request since the change is only the massive increase in the size of the last data that is clamped by KSP anyway.

[rsparkyc]

The bug seems fairly reproducible on my side, especially when I play with realism overhaul. I think it has to do with the much smaller packet size used in RO, as opposed to the integer values for packets used many other places. Just to be sure, did you try reproducing with my MM patch (which reduces the packet size to 0.1)? It seemed to fail pretty consistently with that for me, but then again, it may be a platform issue. With my PR, the probably goes away entirely. I'd hate for it to be a "just me" thing though, it would be good if we had some people on mac/linux that would be able to reproduce it as well.

[KSP-TaxiService]

(I edited my message after posting to reflect that I used your MM patch)

It seems to hint that the bug does not always appear. Let me go back and repeat the test many times

[johnbouma]

@KSP-TaxiService

Since the amount of science data is preset when initializing the RnDCommsStream, I'm testing out a theory that we can just send the full packet size for all packets, while maintaining the amount of usable data sent separately. Can you see any glaring issues with that methodology?

[KSP-TaxiService]

@johnbouma I am seeing one potential issue with the codes (not the methodology) in your commit. The sum of the sentFrameSize still could fall short of the full science data size (thanks to the approx-value float type), preventing the science-gain occurrence.

Reading the KSP's decompiled codes on RnDCommsStream below:

....

public class RnDCommsStream
{
    ...
    public float fileSize;
    private float dataIn;
    ...

    public RnDCommsStream(ScienceSubject subject, float fileSize, float timeout, float xmitValue, bool xmitIncomplete, ResearchAndDevelopment RDInstance)
    {
        ...
        this.fileSize = fileSize; // this is scienceData.dataAmount
        this.dataIn = 0f; // total data transmitted so far
        ...
    }

    public void StreamData(float dataAmount, ProtoVessel source)
    {
        this.dataIn = Mathf.Min(this.fileSize, this.dataIn + dataAmount); // clamp the total data sent to fileSize if total data sent exceeding
        this.UTofLastTransmit = Time.time;
        if (this.dataIn == this.fileSize) // Squad, why choosing float-type equal? D:
        {
            this.submitStreamData(source);
        }
        else if (this.timedOut)
        {
            this.host.StartCoroutine(this.timeoutCoroutine(source));
        }
    }
    ...
}

I believe rsparkyc's pull request is more robust than our current fix because the last packet's size is the dataAmount value, which surely sums the dataIn value over the fileSize, causing the StreamData() function to set dataIn to fileSize and invoking submitStreamData. In our current fix, LastPacketAddtionalSize = 0.1f for the last packet may be not even enough to trigger the case above (as rsparkyc demonstrates on his side).

@rsparkyc Can you change to frame = scienceData.dataAmount; please (without +) in your pull request? I don't see the need to sum up.

[rsparkyc]

updated and tested

[neitsa]

Squashed and merged PR #715 ; thanks a lot to @rsparkyc for the PR!

[rsparkyc]

No problem, glad I could help