Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ReadPayload is called with an invalid payload on client when it uses a predict spawn #855

Closed
Navlyc opened this issue Mar 8, 2025 · 7 comments

Comments

@Navlyc
Copy link

Navlyc commented Mar 8, 2025

General
Unity version: 2022.3.22f1
Fish-Networking version: 4.6.2
Discord link: https://discord.com/channels/424284635074134018/923983677980045382/1347684214363000832

Description
When I use a predict spawn a ReadPayload is called on the spawned object with an invalid payload on client owner of the spawn. I mean the payload doesn't match with what I write in the WritePayload function. After an investigation the payloadLength in ManagedObjects.Spawning.cs line 465 seems corrupted, I get sometime negative or huge values (-1828650512, 1308688880, -234814992).

Replication
Steps to reproduce the behavior:

  1. Add Debug.Log(payloadLength.Value) in ManagedObjects.Spawning.cs line 466
  2. Call a predict spawn (without pool in my case)
  3. Look the console log on the client owner of the spawn and notice the value, it's invalid (negative or huge values like -1828650512, 1308688880, -234814992).

Expected behavior
ReadPayload matching with WritePayload.

Screenshots
I believe I found a fix
Image
We need to add payload.Count (or segmentReader.Length) in last parameter of ReadPayload called in ClientObject.cs line 423 because this function believes the length is in the segmentReader and will read the first bytes of the payload instead. When the value is positive the following code is executed with an offseted reader giving a corrupted payload.

@FirstGearGames FirstGearGames added the Acknowledged Your issue has been acknowledged and will be investigated. label Mar 9, 2025
@FirstGearGames
Copy link
Owner

FirstGearGames commented Mar 9, 2025

Did you have a project file to this?

I assume this can only be tested when spawning as client only?

@Navlyc
Copy link
Author

Navlyc commented Mar 9, 2025

Hello, I'm sorry, I dont have sample project for this. Yes you need just to predict spawn as client. Server can be host or not, we get the same issue.

@FirstGearGames
Copy link
Owner

FirstGearGames commented Mar 9, 2025

Hey! I was not able to replicate the issue. I tried with two clients and server, as well two clients and host, as well one client + host/server. Payload length has always returned 0 as expected.

Here's my test project.

Steps:
Open 2 or 3 editors. Start one as server, rest as client. Predicted spawn occurs when client connects.

payloadbug.zip

@FirstGearGames FirstGearGames added Unable To Reproduce Problem cannot be reproduced. User will need to troubleshoot. Waiting For Information Not enough information has been supplied to resolve this issue. and removed Acknowledged Your issue has been acknowledged and will be investigated. labels Mar 9, 2025
@Navlyc
Copy link
Author

Navlyc commented Mar 9, 2025

Hello, sorry my description was probably not clear. I updated your sample with this code in ReadWritePayload.cs :

`
using FishNet.Connection;
using FishNet.Object;
using FishNet.Serializing;
using UnityEngine;

public class ReadWritePayload : NetworkBehaviour
{
const int PAYLOAD_TEST = 1000;

public override void WritePayload(NetworkConnection connection, Writer writer)
{
    writer.WriteInt32(PAYLOAD_TEST);
    Debug.Log("WritePayload " + PAYLOAD_TEST);
}

public override void ReadPayload(NetworkConnection connection, Reader reader)
{
    int payloadTest = reader.ReadInt32();
    Debug.Log("ReadPayload " + payloadTest);

    if (payloadTest != PAYLOAD_TEST)
    {
        Debug.LogError($"Not expected payload {payloadTest}, expected {PAYLOAD_TEST}");
    }
}

}
`
and I get the issue in the console :

Image

I added the Debug.Log($"ReadPayload Length : {payloadLength}"); in ManagedObjects.Spawning.cs line 466.

@Navlyc
Copy link
Author

Navlyc commented Mar 9, 2025

And the following screenshot is what I get with my shared fix above :

Image

@FirstGearGames
Copy link
Owner

Thanks, I took a better look and your solution seems appropriate. The size was already parsed into the array segment thus can be passed into the ReadPayload method.

I used your solution except modified to reader.Length for consistency reasons. This will be resolved in the next release.

If you wanted to submit a PR for the fix to get credit please feel free to.

@FirstGearGames FirstGearGames added Resolved Pending Release and removed Waiting For Information Not enough information has been supplied to resolve this issue. Unable To Reproduce Problem cannot be reproduced. User will need to troubleshoot. labels Mar 10, 2025
@Navlyc
Copy link
Author

Navlyc commented Mar 10, 2025

Thanks you, it's OK. You can submit.

FirstGearGames pushed a commit that referenced this issue Mar 14, 2025
- Fixed Beta SyncList NullReferenceException error (#850).
- Improved List and Dictionary collection readers now take collections from caches first.
- Fixed NetworkTransform setting Rigidbody/2D interpolation to none on owners.
- Fixed server configuring NetworkTransform before there was an owner.
- Added PreciseTick(uint) constructor to generate a PreciseTick with 0 subtick value.
- Fixed ReadPayload data corruption (#855).
- Improved DebugManager inspector.
- Fixed UniversalTickSmoother conditionally providing excessively large adaptive interpolation values.
- Added Reader.ReadStringAllocated.
- Obsoleted Reader.ReadString.
- Added DebugManager Validate Rpc Lengths to verify RPCs potentially corrupting data.
- Added SceneProcessorBase.GetLastLoadedScene.
- Fixed OnStopNetwork incorrectly invoking on clientHost when object despawned for client but not yet on server.
FirstGearGames pushed a commit that referenced this issue Mar 14, 2025
- Fixed Beta SyncList NullReferenceException error (#850).
- Improved List and Dictionary collection readers now take collections from caches first.
- Fixed NetworkTransform setting Rigidbody/2D interpolation to none on owners.
- Fixed server configuring NetworkTransform before there was an owner.
- Added PreciseTick(uint) constructor to generate a PreciseTick with 0 subtick value.
- Fixed ReadPayload data corruption (#855).
- Improved DebugManager inspector.
- Fixed UniversalTickSmoother conditionally providing excessively large adaptive interpolation values.
- Added Reader.ReadStringAllocated.
- Obsoleted Reader.ReadString.
- Added DebugManager Validate Rpc Lengths to verify RPCs potentially corrupting data.
- Added SceneProcessorBase.GetLastLoadedScene.
- Fixed OnStopNetwork incorrectly invoking on clientHost when object despawned for client but not yet on server.
- Added New NetworkBehaviour template menu under Fish-Networking > Configuratoin, thanks Rain1950! (#851).
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants