Closed RachithP closed 6 months ago
Here is a design sketch for how I think we should synchronize Unreal enums across Python and C++.
Suppose there is some underlying Unreal enum that is not tagged as a UENUM
that we would like to be able to easily pass to and from Python.
enum class UnrealEnum
{
Zero = 0,
One = 1,
Two = 2
};
Our first step is to declare a new UENUM
with matching values. No matter what approach we use, we will need to declare an extra enum somewhere. I think it's better to do this in C++ rather than Python, because we can avoid explicitly referring to the raw underlying integer values.
UENUM()
enum class ESpEnum
{
Zero = static_cast<std::underlying_type_t<UnrealEnum>>(UnrealEnum::Zero),
One = static_cast<std::underlying_type_t<UnrealEnum>>(UnrealEnum::One),
Two = static_cast<std::underlying_type_t<UnrealEnum>>(UnrealEnum::Two)
};
Our next step is to declare a USTRUCT
that contains our UENUM
. We don't need to do anything similar in the MSGPACK
approach, so the fact that we need this extra boilerplate is a legitimate downside of my proposed approach. But it will provide a lot of extra convenience in the following step.
USTRUCT()
struct FSpEnum
{
GENERATED_BODY()
UPROPERTY()
ESpEnum Enum;
};
Now suppose we want to call an Unreal function from a service entry point, where the Unreal function takes as input an FVector
, an FRotator
, and an UnrealEnum
. We can do this very conveniently using the UnrealObj
class that we're already using in other parts of our code.
UnrealObj<FVector> location("Location");
UnrealObj<FRotator> rotation("Rotation");
UnrealObj<FSpEnum> sp_enum("Enum");
UnrealObjUtils::setObjectPropertiesFromStrings({&location, &rotation, &sp_enum}, unreal_obj_strings);
// this cast operation is hideous but we can wrap it in a helper function if we find that we are doing it often
UnrealEnum unreal_enum = static_cast<UnrealEnum>(static_cast<std::underlying_type_t<ESpEnum>>(sp_enum.get().Enum));
GEngine->UnderlyingUnrealFunction(location.get(), rotation.get(), unreal_enum);
In this example, the type of unreal_obj_strings
is std::map<std::string, std::string>
, and its key-value pairs should be set as follows.
unreal_obj_strings.at("Location")
should be:{
"X": 1.23
"Y": 4.56
"Z": 7.89
}
unreal_obj_strings.at("Rotation")
should be:{
"Roll": 1.23
"Pitch": 4.56
"Yaw": 7.89
}
unreal_obj_strings.at("Enum")
should be:{
"Enum": "Two"
}
I think this approach is preferable to the MSGPACK
approach for the following reasons.
Two
instead of the integer 2
or a value from a Python enum, which means we don't have to maintain a Python enum).FVector
and FRotator
in this example) are handled using the exact same code.Unreal::callFunction
and CppFuncService
already use exactly the same convention for passing Unreal types, so this approach is more consistent with the rest of the code.MSGPACK
approach, we can keep this enum synchronized with the underlying Unreal enum without explicitly referring to the integer values (e.g., we don't need to set Two = 2
in our code, we can instead set Two = ugly_cast<UnrealEnum>(UnrealEnum::Two)
, which is less brittle).MSGPACK
approach. But it's only a few lines of declarative boilerplate in a header file, so I think it's worth it.If you are persuaded by this reasoning, you might think that we should use this approach for all structs and enums, and we should never use the MSGPACK
approach for anything. But I still like the MSGPACK
approach for structs and enums that we expect to use in tight loops, e.g., everything in CppFuncService
. GameWorldServicePropertyDesc
is a less obvious case (I could see reasonable arguments for each approach), but I think it is reasonable to leave it how it is. If we do that, then we have a reasonable rule of thumb that we can use to decide which approach to use for a given struct or enum. If we are trying to expose an existing Unreal struct or enum, prefer the USTRUCT
/UENUM
approach. But if we are trying to expose one of our own structs or enums, prefer the MSGPACK
approach.
Hey Rachith, you marked this as being ready to review, but there are a still a bunch of unaddressed comments. Was that intended? Did you want me to review again?
Hey Rachith, you marked this as being ready to review, but there are a still a bunch of unaddressed comments. Was that intended? Did you want me to review again?
Hey Mike, It wasn't intended for another round of review, I changed it as the draft state didn't make sense at this stage of this PR.
This PR is looking great. None of the changes I suggested are super complicated, but they might touch a lot of the new code, so I'd like to have one more look before approving.