search

readyplayerme / rpm-unreal-sdk

Ready Player Me Unreal SDK
Other
20 stars 8 forks source link

SDK-483 feat: adds app id to setup guide #39

Closed MaxAndreassenRPM closed 9 months ago

MaxAndreassenRPM commented 9 months ago

Adds app id to setup guide.

Video of update: https://readyplayerme.slack.com/files/U04FL69MG3V/F061L3RH144/wbp_rpm_setupguide_2023-10-17_15-31-15.mp4

Blueprints: image image

I had to update the next button enable event blueprint logic a bit to have the option of having errors for both fields act in the correct way. Any suggestions on how to improve this are welcome 😄

MaxAndreassenRPM commented 9 months ago

Code looks fine. Somewhat a minor thing but can you rename "?" button to something more descriptive? At the moment it is Button_0 it will just make it a bit less confusing to work with I think. image image

Also all of these URL's (below photo) I feel like it would be better to do these in CPP. Either in the Editor function library or by creating our own EditorUtilityWidget and included such extended functionality (note this would require updating our current widgets and break things when updating potentially). image

But the reasoning here is that all of these URLs are so easily editable and could mistakenly be removed or edited incorrectly from blueprints. And for me I feel it would be easier to just see all the URL's in one CPP file. Plus if it was in CPP devs couldn't remove these by simply editing the BP if that makes sense.

But I am curious on Gevorgs thoughts here, I don't really think it's a blocker though.

Nice catch with the tooltip, now renaming 👍

Re: the urls, what you're saying makes total sense to me - you could have a CPP class of custom blueprint functions designed to each launch a specific url, perhaps even in their own function library? - I think it makes more sense to add a separate task for this though, as the scope of this task about the app id addition already extended with the tooltip addition, and this would blow it up even further 😅

MaxAndreassenRPM commented 9 months ago

Done ✅

image

gadamyan commented 9 months ago

I wouldn't move Urls to C++, here is why.

  1. More complexity. Lots of C++ code and architecture for no reason.
  2. We put unchangeable logic in the C++. We don't put UI-related things that can be changed every day into the C++. If tomorrow Bernhard comes and says we don't need any links in the setup guide, or the links are changed, Instead of changing the C++ logic I will remove the unneeded things from the UI. This is how I make a difference C++ is rigid logic. Blueprint is changeable UI related.