VoxelPlugin / VoxelPluginFreeLegacy

Voxel Plugin Legacy for Unreal Engine
https://voxelplugin.com
1.54k stars 295 forks source link

TCP Multiplayer exception (setting density and material in same frame) #206

Open sintaxed opened 3 years ago

sintaxed commented 3 years ago

Modifying terrain in any way (add or destroy block) causes this crash after plugin upgrade:

https://pastebin.com/TueEHrEs

followed by this error and crash:

Thu Aug 27 21:32:34 EDT 2020 Error LogWindows === Critical error: === Thu Aug 27 21:32:34 EDT 2020 Error LogWindows Assertion failed: UncompressedData.Num() >= sizeof(bool) + sizeof(uint32) [File:P:/Projects/game/Plugins/VoxelPro/Source/Voxel/Private/VoxelMultiplayer/VoxelMultiplayerUtilities.cpp] [Line: 28] Thu Aug 27 21:32:34 EDT 2020 Error LogWindows Unhandled Exception: 0x438ae31d Thu Aug 27 21:32:34 EDT 2020 Error LogWindows [Callstack] 0x8900000001b90000 UnknownFunction []

Phyronnaz commented 3 years ago

@sintaxed Could you try with the new Tcp multiplayer example located under Voxel/Examples/Maps/Multiplayer?

sintaxed commented 3 years ago

This is what I use to add block and destroy is just Set Density(1) 1

and there's some data in the UVs

Phyronnaz commented 3 years ago

This should fix the crash, but not the underlying bug: https://gitlab.com/Phyronnaz/VoxelPluginProBeta/-/commit/2202b0d833eadb4350f8f6c46fa648a934cd07e8

Could you make a small repro project & send it to me?

sintaxed commented 3 years ago

All I do is use "set density" to change the one voxel, so my conclusion on THIS MATTER is it's an issue with replicating that. I changed my add/delete voxel functions to run multicast instead of the server, and there's the traceback pause only the first time I do an edit so the method serves for now.

Here is my workaround if someone else has the problem:

In this file: https://gitlab.com/Phyronnaz/VoxelPluginProBeta/-/commit/2202b0d833eadb4350f8f6c46fa648a934cd07e8

    TArray64<uint8> UncompressedData;
//  if (!ensure(FVoxelSerializationUtilities::DecompressData(Data, UncompressedData)))
    if (!FVoxelSerializationUtilities::DecompressData(Data, UncompressedData))
    {
        return;
    }

VoxelSerializationUtilities.cpp line 175:

// if (!ensureMsgf(Header.CompressedSize == CompressedData.Num() - sizeof(FHeader), TEXT("Archive is saying its size is %lld, but it's %lld"), Header.CompressedSize, CompressedData.Num() - sizeof(FHeader)))
  if (Header.CompressedSize != CompressedData.Num()- sizeof(FHeader))
  {
      UncompressedData.Empty();
      return false;
  }

And doing this to make sure everyone actually has the edit:

2021-03-03 16_09_26-GoblinKeep - Unreal Editor

sintaxed commented 3 years ago

As per @hein0r on discord this works: DataAccelerator.Set(CurrentPosition, SomeValue); //DataAccelerator.Set(CurrentPosition, SomeMaterial);

this works: //DataAccelerator.Set(CurrentPosition, SomeValue); DataAccelerator.Set(CurrentPosition, SomeMaterial);

this crashes: DataAccelerator.Set(CurrentPosition, SomeValue); DataAccelerator.Set(CurrentPosition, SomeMaterial);

Similar to the way I have it, but only crashes when you set the material at the same time as density it looks like

heinerlohmann commented 3 years ago

yup I (hein0r on discord) encountered the same issue. basically whenever i want to edit value AND material in multiplayer. i have tried different ways of doing that, but not found a way that works. i can change and synch to clients changes to value OR material. if i change both in between two synchronisations i end up with the same exception as sintaxed.

heinerlohmann commented 3 years ago

i managed to circumvent the issue with this workaround in the VoxelMultiplayerManager.cpp i think: (the code just won't format correctly, sorry)

`void FVoxelMultiplayerManager::SendData() const { VOXEL_FUNCTION_COUNTER();

check(Server.IsValid());
if (!Server->IsValid()) return;

TArray<TVoxelChunkDiff<FVoxelValue>> ValueDiffs;
TArray<TVoxelChunkDiff<FVoxelMaterial>> MaterialDiffs;
GetSubsystemChecked<FVoxelData>().GetDiffs(ValueDiffs, MaterialDiffs);

    // ORIGINAL CODE
//if (ValueDiffs.Num() > 0 || MaterialDiffs.Num() > 0)
//{
//  Server->SendDiffs(ValueDiffs, MaterialDiffs);
//}

    // NEW CODE
if (ValueDiffs.Num() > 0)
{
    Server->SendDiffs(ValueDiffs, TArray<TVoxelChunkDiff<FVoxelMaterial>>());
}
if (MaterialDiffs.Num() > 0)
{
    Server->SendDiffs(TArray<TVoxelChunkDiff<FVoxelValue>>(), MaterialDiffs);
}

}`

i think the underlying issue is that the serialisation just isnt working correctly when both value and material changed, because at the beginning of the data the client will only find the size of the compressed value data or if there is no value data it will get the size of the material data, that's why it works when only one of the two get synchronised. at least i think, that serialisation code is quite hard to read for a newbie like me :D

mjegen commented 2 years ago

For anyone else encountering this issue, it's worth noting that heinerlohmann's workaround may cause visual artifacts. When testing this issue with "Play as Client" and 2 Players in the Unreal editor, I was seeing the voxel edits appear in the second client followed by the application of the material after a very small delay. I didn't test in a shipping build, but given that the diffs are sent as two seperate network calls, the visual artifacts will likely occur under various network conditions.

heinerlohmann commented 2 years ago

For anyone else encountering this issue, it's worth noting that heinerlohmann's workaround may cause visual artifacts. When testing this issue with "Play as Client" and 2 Players in the Unreal editor, I was seeing the voxel edits appear in the second client followed by the application of the material after a very small delay. I didn't test in a compiled build, but given that the diffs are sent as two seperate network calls, the visual artifacts will likely occur under various network conditions.

Yes, it is just a workaround not a perfect solution. In most cases it might produce better results when sending material first and value second.