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

[UNR-2003] Create CookAndGenerateSchemaCommandlet #1342

Merged
merged 47 commits into from
Sep 26, 2019

Conversation

mattyoung-improbable
Copy link
Contributor

@mattyoung-improbable mattyoung-improbable commented Sep 10, 2019

Description

This PR Includes:

  1. A new CookAndGenerateSchemaCommentlet that will run a regular Unreal Cook but also generate schema for any supported classes that get created during the cook.
  2. Changed what is considered a supported for schema generation to be only classes that are explicitly SpatialTypes. (All AActor, UActorComponent, UGameplayAbility, UAttributeSet) subclasses are explicitly SpatialTypes.
    NOTE Any users that have custom replicated subobjects will need to make sure they are marked as SpatialType (either with UClass specifier or in the blueprint class properties window).

Engine PR: https://github.com/improbableio/UnrealEngine/pull/250

@improbable-prow-robot
Copy link

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

@improbable-prow-robot improbable-prow-robot added jira/UNR size/XL Denotes a PR that changes 300-599 lines, ignoring generated files. labels Sep 10, 2019
for (TObjectIterator<UClass> ClassIt; ClassIt; ++ClassIt)
if (SupportedClass->IsEditorOnly())
{
UE_LOG(LogSpatialGDKSchemaGenerator, Verbose, TEXT("[%s] Editor-only Class Not supported for schema gen"), *GetPathNameSafe(SupportedClass));
Copy link
Collaborator

Choose a reason for hiding this comment

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

The capitalization of this message is triggering me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return Classes;
}

TSet<UClass*> FilterClasses(TSet<UClass*> Classes)
Copy link
Collaborator

@m-samiec m-samiec Sep 12, 2019

Choose a reason for hiding this comment

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

Suggested change
TSet<UClass*> FilterClasses(TSet<UClass*> Classes)
TSet<UClass*> FilterClasses(const TSet<UClass*>& Classes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return true;
}

SPATIALGDKEDITOR_API bool SpatialGDKGenerateSchemaForClasses(TSet<UClass*> Classes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
SPATIALGDKEDITOR_API bool SpatialGDKGenerateSchemaForClasses(TSet<UClass*> Classes)
bool SpatialGDKGenerateSchemaForClasses(TSet<UClass*> Classes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

{
if (SchemaGeneratedClasses.Contains(Class)) continue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: New line please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

{
if (SchemaGeneratedClasses.Contains(Class)) continue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should SchemaGeneratedClasses get cleared before this loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generate Schema is being called in batches, so it's useful to cache generated classes between calls. This code really needs cleaning up to encapsulate this state properly, most likely as part of the testing work.

}

UE_LOG(LogCookAndGenerateSchemaCommandlet, Display, TEXT("Generate Schema for C++ and in-memory Classes."));
if (!SpatialGDKGenerateSchema(false /* bSaveSchemaDatabase */, false /* bRunSchemaCompiler */))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not do this at the end with the blueprint classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of doing this I've used a class iterator to add the classes to ReferencedClasses upfront.

@@ -410,47 +410,90 @@ bool SaveSchemaDatabase()
return true;
}

TArray<UClass*> GetAllSupportedClasses()
bool IsSupportedClass(UClass* SupportedClass)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bool IsSupportedClass(UClass* SupportedClass)
bool IsSupportedClass(const UClass* SupportedClass)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -684,21 +727,65 @@ bool RunSchemaCompiler()
}
}

bool SpatialGDKGenerateSchema()
bool SpatialGDKGenerateSchema(bool bSaveSchemaDatabase, bool bRunSchemaCompiler)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth considering removing parameter options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


return RunSchemaCompiler();
return true;
}

#undef LOCTEXT_NAMESPACE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move outside namespaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

{
if (UClass* NestedClass = Cast<UClass>(TypeNode->Type))
{
if (!Classes.Contains(NestedClass) && !SchemaGeneratedClasses.Contains(NestedClass) && IsSupportedClass(NestedClass))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!Classes.Contains(NestedClass) && !SchemaGeneratedClasses.Contains(NestedClass) && IsSupportedClass(NestedClass))
if (!SchemaGeneratedClasses.Contains(NestedClass) && IsSupportedClass(NestedClass))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -994,7 +994,8 @@ void USpatialNetDriver::ServerReplicateActors_ProcessPrioritizedActors(UNetConne
// or it's an editor placed actor and the client hasn't initialized the level it's in
if (Channel == nullptr && GuidCache->SupportsObject(Actor->GetClass()) && GuidCache->SupportsObject(Actor->IsNetStartupActor() ? Actor : Actor->GetArchetype()))
{
if (Actor->GetClass()->HasAnySpatialClassFlags(SPATIALCLASS_NotSpatialType))
if (Actor->GetClass()->HasAnySpatialClassFlags(SPATIALCLASS_NotSpatialType) ||
!Actor->GetClass()->HasAnySpatialClassFlags(SPATIALCLASS_ExplicitSpatialType))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe these flags are mutually exclusive. So we should just be able to check for the existence of ExplicitSpatialType.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good Spot. Done

…it's mutually exclusive to SPATIALCLASS_NotSpatialType
…com:spatialos/UnrealGDK into feature/cook-and-generate-schema-commandlet
return false;
}

// User told us to ignore this class
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


UE_LOG(LogCookAndGenerateSchemaCommandlet, Display, TEXT("Schema Generation Finished in %.2f seconds"), Duration.GetTotalSeconds());

if (!SaveSchemaDatabase())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a ticket to iterate on this process so we don't have to run the cook command multiple times?

Copy link
Collaborator

@m-samiec m-samiec left a comment

Choose a reason for hiding this comment

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

Nice!

…ts/CookAndGenerateSchemaCommandlet.cpp

Co-Authored-By: improbable-valentyn <32096431+improbable-valentyn@users.noreply.github.com>
@m-samiec m-samiec merged commit e3e8469 into master Sep 26, 2019
@m-samiec m-samiec deleted the feature/cook-and-generate-schema-commandlet branch September 26, 2019 09:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
jira/UNR size/XL Denotes a PR that changes 300-599 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants