Skip to content
This repository was archived by the owner on Nov 8, 2020. It is now read-only.

Work in Progress: Adding in a UE4 blueprintable enum for login type - #33

Draft
Sessional wants to merge 1 commit into
fastlorisstudios:masterfrom
Sessional:swapping-type-to-bp-enum
Draft

Sessional wants to merge 1 commit into
fastlorisstudios:masterfrom
Sessional:swapping-type-to-bp-enum

Conversation

@Sessional

Copy link
Copy Markdown

Hoping you'll be kind enough to provide a "50% code review" and set me on a different course if this isn't what you have in mind.

Left a few //TODO statements for places where I'm not sure what to do with them. (C++ Newby)

The goal: provide a drop down for "Type" in the blueprint node so that a user can specify how to do logins inside their game without leaving blueprint or replicating behavior.

Justification: While not mandatory, prettifies the example interaction with the Blueprint Request Login node. And makes it simpler to swap the mode of authentication in the example project.

@fastlorisstudios fastlorisstudios left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The biggest request would be code formatting, to ensure the project is maintained the same throughout.
No spaces after some declaration (if, switch etc) and its braces. Spaces within the braces. All {scope} brackets in-line (i.e. opening on a new line, never the same as the declaration). Tab alignment of declarations (makes scrolling down the file easier).

In the player controller, create a new function RequestLoginByType, with the parameters you've specified. In there, do the Type to String conversion and call the original. In the RequestLoginByType UFUNCTION declaration you can neaten this up with a Meta and rename it, so on the BP side it simply says "Request Login" for both.

@Sessional
Sessional force-pushed the swapping-type-to-bp-enum branch from 52eae31 to 0d6e63d Compare July 23, 2020 13:23

@fastlorisstudios fastlorisstudios left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Much of the design approach(es) taken for this Plugin mimic the Steam OSS Plugin. As such, there is no Blueprint Library in that Plugin, so I don't think it appropriate for us to have one in this Plugin, either.

Your own question about including the .cpp needs resolving before a merge. You also have the header include in the PlayerController header - is this include necessary?

And again, formatting considerations. All {scope} braces vertically in-line is a minimum.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants