479 onboarding azim #652
479 onboarding azim #652
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
❌ Missing newline at end of file! Files should always end with a single newline character.
| } | |
| } | |
This bad pattern was detected by the following regular expression:
.*
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| namespace SEE.Controls.Actions | ||
| { | ||
| /// <summary> | ||
| /// Action to create a new node for a selected city. |
There was a problem hiding this comment.
Documentation is wrong. Looks like a copy & paste error.
| { | ||
| /// <summary> | ||
| /// If the user clicks with the mouse hitting a game object representing a graph node, | ||
| /// this graph node is a parent to which a new node is created and added as a child. |
There was a problem hiding this comment.
This documentation is misleading. The term node should refer only to graph nodes or game objects representing graph nodes. Do you mean a marker instead?
| GameObject parent = raycastHit.collider.gameObject; | ||
| Vector3 position = parent.transform.position; | ||
| Vector3 scale = parent.transform.lossyScale; | ||
| addedSphere = GameNodeMarker.AddSphere(parent, position: position, worldSpaceScale: scale); |
There was a problem hiding this comment.
Why passing position and scale as parameters? Couldn't GameNodeMarker.AddSphere retrieve these?
| addedSphere = GameNodeMarker.AddSphere(parent, position: position, worldSpaceScale: scale); | ||
| // addedSphere has the scale and position of Vector3. | ||
| // The position at which the parent was hit will be the center point of the addedSphere. | ||
| momento = new Momento(parent, position: position, scale: scale); |
There was a problem hiding this comment.
Should be memento not momento.
There was a problem hiding this comment.
This is even a compiler error. Do not check-in code that does not compile!
| // The position at which the parent was hit will be the center point of the addedSphere. | ||
| momento = new Momento(parent, position: position, scale: scale); | ||
| memento.NodeID = addedSphere.name; | ||
| new MarkNetAction(parentID: memento.Parent.name, memento.Position, memento.Scale).Execute(); |
There was a problem hiding this comment.
As above, no need for the position and scale parameters.
| return new HashSet<string> | ||
| { | ||
| memento.Parent.name, | ||
| memento.NodeID |
There was a problem hiding this comment.
See above, a marker is not a node.
| namespace SEE.Game.SceneManipulation | ||
| { | ||
|
|
||
| public static class GameNodeMarker |
There was a problem hiding this comment.
Documentation is missing.
| public static GameObject NodeMarker(GameObject parent, Vector3 position, Vector3 worldSpaceScale) | ||
| { | ||
| GameObject sphere = GameObject.CreatePrimitive(PrimitiveType.Sphere); | ||
| Vector3 position = parent.transofrm.position; |
There was a problem hiding this comment.
Typo in transofrm leading to a compile error. Do not check-in code that does not compile!
| // Getter size and position of sphere. | ||
| float coords = Math.Min(scale.x, scale.y); | ||
| Vector3 size = new Vector3(coords, coords, coords); | ||
| Vector3 position = new Vector3(position.x, position.y + scale.y, position.z); |
There was a problem hiding this comment.
This will not work when there is an antenna or any other kind of decoration above the game object. As hinted in the issue description, you should use GetTop().
| namespace SEE.Net.Actions | ||
| { | ||
| /// <summary> | ||
| /// This class is responsible for adding a node via network from one client to all others and |
|
PR was only for the onboarding task. The onboarding task will not be merged. |
No description provided.