-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade v4 #18
Upgrade v4 #18
Conversation
Thanks @pasotee for the PR.
|
@@ -217,20 +211,20 @@ void UConfigCatSubsystem::ForceRefresh() | |||
ConfigCatClient->forceRefresh(); | |||
} | |||
|
|||
void UConfigCatSubsystem::SetDefaultUser(const FConfigCatUser& User) | |||
void UConfigCatSubsystem::SetDefaultUser(const UConfigCatUserWrapper* User) | |||
{ | |||
if (!ensure(ConfigCatClient)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please create an EnsureConfigCatClient() function where this log message can be printed:
bool EnsureConfigCatCline()
{
if (ensure(ConfigCatClient))
{
return true;
}
UE_LOG(LogConfigCat, Warning, TEXT("Trying to access the ConfigCatClient before initialization or after shutdown."));
return false;
}
In every function we can use this EnsureConfigCatClient() to check the client easier:
if (!EnsureConfigCatClient())
{
return;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a great idea. I added it now, thanks.
@@ -0,0 +1,117 @@ | |||
// Fill out your copyright notice in the Description page of Project Settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the standard? Is this line needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is an oversight on my side. I have adjusted it with the proper copyright.
Source/ConfigCatWrappers/Private/ConfigCatPercentageOptionWrapper.cpp
Outdated
Show resolved
Hide resolved
Source/ConfigCatWrappers/Private/ConfigCatEvaluationWrapper.cpp
Outdated
Show resolved
Hide resolved
Source/ConfigCatWrappers/Private/ConfigCatEvaluationWrapper.cpp
Outdated
Show resolved
Hide resolved
Source/ConfigCatWrappers/Private/ConfigCatEvaluationWrapper.cpp
Outdated
Show resolved
Hide resolved
|
||
FString UConfigCatUserConditionWrapper::GetStringComparisonValue() const | ||
{ | ||
// UserCondition.comparisonValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can implement it like this:
// UserCondition.comparisonValue; | |
const auto comparisonValuePtr = get_if<string>(&UserCondition.comparisonValue); | |
if (comparisonValuePtr) | |
{ | |
return UTF8_TO_TCHAR(comparisonValuePtr->c_str()); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting those and for the suggestions, I didn't know they were a thing.
However, I have to implement the Has...Value
functions as well, so I will be using that to check the values.
|
||
double UConfigCatUserConditionWrapper::GetNumberComparisonValue() const | ||
{ | ||
// UserCondition.comparisonValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// UserCondition.comparisonValue; | |
const auto comparisonValuePtr = get_if<double>(&UserCondition.comparisonValue); | |
if (comparisonValuePtr) | |
{ | |
return *comparisonValuePtr; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function is now implemented
// UserCondition.comparisonValue; | ||
return {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// UserCondition.comparisonValue; | |
return {}; | |
TArray<FString> Result; | |
const auto comparisonValuePtr = get_if<vector<string>>(&UserCondition.comparisonValue); | |
if (comparisonValuePtr) | |
{ | |
for (const auto& value : *comparisonValuePtr) | |
{ | |
Result.Emplace(UTF8_TO_TCHAR(value.c_str())); | |
} | |
} | |
return Result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function is now implemented
This is awesome. I think it would benefit both the CI & CD pipelines, so I've added it to both |
Describe the purpose of your pull request