Refactored actions logic into it's own component #25
Conversation
kkjamie
requested review from
FancySensei,
LukeAvery92,
borntocompile,
zieglerkevin,
danhalldubit and
SlaveykoSlaveykov-Dubit
| private DebugMenuItemNode rootNode; | ||
| private DebugMenuItemNode currentPageNode; | ||
|
|
||
| private void Awake() |
There was a problem hiding this comment.
This doesn't get the awake call right away since this object is disabled to begin with, we must manually initialize it
Scripts/DebugMenu.cs
Outdated
| @@ -59,19 +57,22 @@ public static DebugMenu Instance | |||
| private Button actionButtonTemplate; | |||
There was a problem hiding this comment.
this should be removed as it was moved to the other file
|
|
||
| if (ButtonPathExists(path)) | ||
| { | ||
| throw new Exception($"Cannot add button at {path}. A button/folder already exists!"); |
There was a problem hiding this comment.
Inconsistent with previous two lines (one line throw).
|
|
||
| if (!ButtonPathExists(path)) | ||
| { | ||
| throw new Exception($"Cannot remove the button {path}. It does not exist."); |
There was a problem hiding this comment.
Same. Inconsistent with the previous line (one line throw).
| { | ||
| if (currentNode.ContainsChild(part)) | ||
| { | ||
| currentNode = currentNode.GetChild(part); |
There was a problem hiding this comment.
Is it faster if you just GetChild then do a null check, rather than check ContainsChild first then GetChild?
| { | ||
| var backButton = Instantiate(actionButtonTemplate, actionButtonTemplate.transform.parent, false); | ||
| backButton.GetComponentInChildren<Text>().text = $"{backCharacter} Back"; | ||
| backButton.onClick.AddListener(() => { SetupPage(node.Parent); }); |
There was a problem hiding this comment.
Minor:
You can do backButton.onClick.AddListener(() => SetupPage(node.Parent));
| } | ||
|
|
||
| var label = actionButton.GetComponentInChildren<Text>(); | ||
| label.text = button.Name + (button.IsFolder ? $" {folderArrowCharacter}" : ""); |
There was a problem hiding this comment.
I prefer string.Empty over "", for me it adds intent to it so I know its not a mistake.
| label.text = button.Name + (button.IsFolder ? $" {folderArrowCharacter}" : ""); | ||
| actionButton.gameObject.SetActive(true); | ||
|
|
||
| firstChild = (firstChild != null) ? firstChild : actionButton.gameObject; |
There was a problem hiding this comment.
Parenthesis are redundent
| currentNode.AddFolder(part); | ||
| } | ||
| } | ||
| actionsPage.AddButton(path, action,hideDebugMenuOnClick); |
There was a problem hiding this comment.
needs a space after the second comma
Resolves issue #22
Key points: