channeldorg / channeld-ue-plugin

Enables distributed simulation with Unreal Engine's dedicated servers. 为虚幻引擎专用服务器提供分布式模拟能力的开源插件.
Apache License 2.0
132 stars 38 forks source link

Modify property decorator to determine mem offset at runtime #69

Closed Kureans closed 1 year ago

Kureans commented 1 year ago

This is to prevent crashing on builds across different machines with different memory architectures.

When attempting to deploy DS on a separate linux machine, it will crash will client tries to login. This is due to inconsistency in memory offsets between Windows & Linux for properties, so when memory offset is generated at compile time, it will be based on the host build machine, hence it will be incompatible if we try to deploy the build on a machine with different architecture. (Value of Num_PropMemOffset different from Property->GetOffset_ForInternal() on Linux)

For example, when debug print statements were added inside the generated code, Property->GetOffsetForInternal() produced different values across different machines.

image

(The white background log is on windows with an offset of 2040, the bottom log is on linux with an offset of 1672, both for the same property)

However, it comes at a runtime cost, so if we know that client/server will be deployed on machines with the same memory architecture, we can use PropPtrForGlobalStruct instead, which is the original implementation.

indiest commented 1 year ago

Thanks for the contribution and the detailed explanation!

One question: how to switch between generating code with or without PropPtrForGlobalStruct?

Kureans commented 1 year ago

At the moment, I have not implemented the ability to switch code generation with/without PropPtrForGlobalStruct, the changes still have to be made within the code itself (Change PropPtrForGlobalStruct to PropPtr when we know we are building for different machines).

There is the possibility of enabling switching between generating mem offset at compile time / run time by adding on to the plugin:

However, both options feel like they are too big of a change to put together with this PR, so only the code functionality has been added.

indiest commented 1 year ago

Some thought: for the same type of replicator, use FindPropertyByName once for each property and cache their memory offsets at runtime for the replicator instances created afterwards. Maybe in this way, we can balance the runtime cost and the compatibility issues.

Kureans commented 1 year ago

Think that's a great suggestion, I've just integrated that into PropPtr and removed the PropPtrForGlobalStruct entirely.

indiest commented 1 year ago

Hi @Kureans , I'm testing your change and it seems that the demo project can't compile with the generated code: image

Maybe for the RPC parameter structs, the generated code should be kept as-is, assuming the memory offsets are always the same for the fields of the C++ structs, even in different system or memory architectures.