Skip to content
This repository has been archived by the owner on Aug 29, 2024. It is now read-only.

UNR-1402: Crash when attempting to replicate a non supported type #1295

Merged
merged 4 commits into from
Aug 20, 2019

Conversation

sjtsilvester
Copy link
Contributor

Contributions: We are not currently taking public contributions - see our contributions policy. However, we are accepting issues and we do want your feedback.


Description

Unreal produces a warning (rather than crashing the game) if the game attempts to replicate an unsupported type (such as TMap). This PR changes the GDK to match this behaviour.

Release note

Attempting to replicate unsupported types (such as TMap) produce a log error rather than crashing the game.

Tests

Tested by replicating a TMap in GDKCharacter in the example project and verifying that the game no longer crashed but produced an error.

How can this be verified by QA?

Create an actor that contains a replicated TMap property and play with spatial. The game should run normally with an error message in the output.

Primary reviewers

@joshuahuburn @improbable-valentyn

@improbable-prow-robot
Copy link

Corresponding JIRA ticket: https://improbableio.atlassian.net/browse/UNR-1402

@improbable-prow-robot improbable-prow-robot added jira/UNR size/XS Denotes a PR that changes 0-14 lines, ignoring generated files. labels Aug 13, 2019
@@ -314,9 +316,13 @@ void ComponentFactory::AddProperty(Schema_Object* Object, Schema_FieldId FieldId
{
// These properties can be set to replicate, but won't serialize across the network.
}
else if (Property->IsA<UMapProperty>())
{
UE_LOG(LogComponentFactory, Error, TEXT("Replicated TMaps are not supported."));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to indicate to the user which property this is (variable name, class etc.)

else
{
checkf(false, TEXT("Tried to add unknown property in field %d"), FieldId);
UE_LOG(LogComponentFactory, Error, TEXT("Tried to add unknown property in field %d"), FieldId);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does native unreal do here? I'm more in favour of explicit check rather than potentially missing a replicated prop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each property would have its own NetSerializeItem which prints an error or does nothing if the property can't be replicated.

@improbable-prow-robot improbable-prow-robot added size/S Denotes a PR that changes 15-39 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-14 lines, ignoring generated files. labels Aug 19, 2019
@sjtsilvester sjtsilvester merged commit 12f4654 into master Aug 20, 2019
@sjtsilvester sjtsilvester deleted the UNR-1402-crash-replicating-unsupported-type branch August 20, 2019 10:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
jira/UNR size/S Denotes a PR that changes 15-39 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants