-
Notifications
You must be signed in to change notification settings - Fork 5
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
479 Onboarding sarah #655
479 Onboarding sarah #655
Conversation
… selected. If the same node will clicked again, it will be unmarked. Fixes needed, because the unmarking doesn't work for the client yet
…llRootTypesJustContainsAllRoots order corrected
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.
A Redo
is missing. Other than that there were only a few minor code issues.
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.
This file should not be checked in.
@@ -76,6 +76,10 @@ static ActionStateTypes() | |||
Color.yellow.Darker(), "Materials/ModernUIPack/Eye", | |||
HideAction.CreateReversibleAction); | |||
|
|||
MarkNode = | |||
new("Mark Node", "Mark or unmark a node within a graph", | |||
Color.blue.Darker(),"",MarkAction.CreateReversibleAction); |
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.
A blank should be added between parameters.
|
||
namespace SEE.Controls.Actions | ||
{ | ||
internal class MarkAction : AbstractPlayerAction |
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.
Every class should have a documentation above the class declaration.
{ | ||
/// <summary> | ||
/// The node that was marked when this action was executed. It is saved so | ||
/// action can be removed on Undo(). |
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.
the needs to be added.
new MarkNetAction(parent.name).Execute(); | ||
result = true; | ||
CurrentState = IReversibleAction.Progress.Completed; | ||
} |
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.
Indentation should be fixed.
result.transform.position = new Vector3(parent.GetTop().x, parent.GetTop().y + (diameterSphere/2), parent.GetTop().z); | ||
|
||
result.transform.SetParent(parent.transform); | ||
Portal.SetPortal(parent, gameObject: 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.
Nice, you thought about the portal.
|
||
namespace SEE.Net | ||
{ | ||
public class MarkNetAction : AbstractNetAction |
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.
Documentation above the class in missing.
/// Constructor. | ||
/// </summary> | ||
/// <param name="parentID">unique ID of the parent which gets the marker</param> | ||
public MarkNetAction |
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.
Parameter list should be one the same line as the constructor name.
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 has changed here? Why was this file updated?
/// <summary> | ||
/// The information we need to re-mark a node. | ||
/// </summary> | ||
private struct Memento |
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.
If the memento remembers only one value, you could as well do without a Memento struct.
PR was only for the onboarding task. The onboarding task will not be merged. |
#479 A yellow sphere will be created when a node within a graph is selected. When its selected again the sphere will be deleted.