Skip to content
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

Update ConnectedPlayer.h #141

Closed
wants to merge 2 commits into from
Closed

Conversation

PhyrexianTR
Copy link

PlayerId is exposed to the Blueprint due to creating an array struct for the "Update Connected Players" function.

PlayerId is exposed to the Blueprint due to creating an array struct for the "Update Connected Players" function.
@nassosterz-ms
Copy link
Contributor

nassosterz-ms commented Jan 31, 2023

MicrosoftTeams-image

This is with main. I am able to make (and also tried breaking) the ConnectedPlayer struct. I understand you want to be able to set player id also when you make the struct, so changing BlueprintReadOnly to BlueprintReadWrite seems reasonable, the other changes dont seem to be needed.

@PhyrexianTR
Copy link
Author

image
This is what it looks like with PlayerId.

USTRUCT(BlueprintType)
struct FConnectedPlayer
{
GENERATED_BODY()
public:
UPROPERTY(BlueprintReadOnly)
UPROPERTY(EditAnywhere, BlueprintReadWrite)
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with the change BlueprintReadOnly -> BlueprintReadWrite.

Any reason you are suggesting the EditAnywhere change?

Copy link
Author

Choose a reason for hiding this comment

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

I just wanted to have accessibility both on C++ and Blueprint Graph.

Copy link
Contributor

Choose a reason for hiding this comment

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

The EditAnywhere property isnt related to C++ afaik. In C++ you can access FStringId easily since FConnectedPlayer is a struct

@nassosterz-ms
Copy link
Contributor

Closing this PR since there are conflicts, will re open a new one with the changes that have been suggested. Please feel free to review it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants