Skip to content
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 of new see members yvo #485

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions Assets/SEE/Controls/Actions/ActionStateType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,23 @@ public class ActionStateType
new ActionStateType(5, "Edit Node", "Edit a node",
Color.green.Darker(), "Materials/ModernUIPack/Settings",
EditNodeAction.CreateReversibleAction);
public static ActionStateType MarkNode { get; } =
new ActionStateType(6, "Mark Node", "Marks a node.",
Color.green.Darker(), "Materials/ModernUIPack/Settings",
MarkAction.CreateReversibleAction);
public static ActionStateType ScaleNode { get; } =
new ActionStateType(6, "Scale Node", "Scale a node",
new ActionStateType(7, "Scale Node", "Scale a node",
Color.green.Darker(), "Materials/ModernUIPack/Crop",
ScaleNodeAction.CreateReversibleAction);
public static ActionStateType Delete { get; } =
new ActionStateType(7, "Delete", "Delete a node or an edge",
new ActionStateType(8, "Delete", "Delete a node or an edge",
Color.yellow.Darker(), "Materials/ModernUIPack/Trash",
DeleteAction.CreateReversibleAction);
public static ActionStateType ShowCode { get; } =
new ActionStateType(8, "Show Code", "Display the source code of a node.",
new ActionStateType(9, "Show Code", "Display the source code of a node.",
Color.black, "Materials/ModernUIPack/Document", ShowCodeAction.CreateReversibleAction);
public static ActionStateType Draw { get; } =
new ActionStateType(9, "Draw", "Draw a line",
new ActionStateType(10, "Draw", "Draw a line",
Color.magenta.Darker(), "Materials/ModernUIPack/Pencil",
DrawAction.CreateReversibleAction);
#endregion
Expand Down
167 changes: 167 additions & 0 deletions Assets/SEE/Controls/Actions/MarkAction.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
using System.Collections.Generic;
using SEE.Game;
using SEE.GO;
using SEE.Net;
using SEE.Net.Actions;
using SEE.Utils;
using UnityEngine;

namespace SEE.Controls.Actions
{
/// <summary>
/// Action to select and mark an element in a code city.
/// </summary>
internal class MarkAction : AbstractPlayerAction
{
/// <summary>
/// If the user clicks with the mouse hitting a game object representing a graph node, this graph node
/// will be marked with a sphere above.
/// </summary>
/// <returns>true if completed</returns>
public override bool Update()
{
bool result = false;

// FIXME: Needs adaptation for VR where no mouse is available.
if (Input.GetMouseButtonDown(0)
&& Raycasting.RaycastGraphElement(out RaycastHit raycastHit, out GraphElementRef _) == HitGraphElement.Node)
{
// the hit object is the parent in which to create the sphere marker
GameObject parent = raycastHit.collider.gameObject;
// The position at which the parent was hit will be the center point of the new node marker
Vector3 position = parent.transform.position;
Vector3 scale = parent.transform.lossyScale;
addedSphere = GameNodeMarker.AddSphere(parent, position: position, worldSpaceScale: scale);
if (addedSphere != null)
{
memento = new Memento(parent, position: position, scale: scale);
memento.NodeID = addedSphere.name;
new MarkNetAction(parentID: memento.Parent.name, memento.Position, memento.Scale).Execute();
result = true;
currentState = ReversibleAction.Progress.Completed;
}
else
{
Debug.LogError($"New marker could not be created.\n");
}
}
return result;
}

/// <summary>
/// The game object that was added when this action was executed. It is saved so
/// that it can be removed by Undo().
/// </summary>
private GameObject addedSphere;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The unmarking, i.e., destruction of the added sphere, could be handled by GameNodeMarker.


/// <summary>
/// Memento capturing the data necessary to re-do this action.
/// </summary>
private Memento memento;

/// <summary>
/// The information we need to re-add a marker whose addition was undone.
/// </summary>
private struct Memento
{
/// <summary>
/// The parent of the new marker.
/// </summary>
public readonly GameObject Parent;
/// <summary>
/// The position of the new marker in world space.
/// </summary>
public readonly Vector3 Position;
Copy link
Collaborator

Choose a reason for hiding this comment

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

GameNodeMarker could take care of position and scale of the sphere. No need to bother with these here.

/// <summary>
/// The scale of the new marker in world space.
/// </summary>
public readonly Vector3 Scale;
/// <summary>
/// The node ID for the added marker. It must be kept to re-use the
/// original name of the marker in Redo().
/// </summary>
public string NodeID;

/// <summary>
/// Constructor setting the information necessary to re-do this action.
/// </summary>
/// <param name="parent">parent of the marker</param>
/// <param name="position">position of the marker in world space</param>
/// <param name="scale">scale of the new sphere in world space</param>
public Memento(GameObject parent, Vector3 position, Vector3 scale)
{
Parent = parent;
Position = position;
Scale = scale;
NodeID = null;
}
}

/// <summary>
/// Undoes this MarkAction.
/// </summary>
public override void Undo()
{
base.Undo();
if (addedSphere != null)
{
new DeleteNetAction(addedSphere.name).Execute();
Copy link
Collaborator

Choose a reason for hiding this comment

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

addedSphere.name would need to be unique. In your implementation I cannot see how this is guaranteed.

Moreover, DeleteNetAction is generally intended to be used to delete nodes or edges. We would need to check whether it is safe to be used for other kinds of game objects, too.

Destroyer.DestroyGameObject(addedSphere);
addedSphere = null;
}
}

/// <summary>
/// Redoes this MarkAction.
/// </summary>
public override void Redo()
{
base.Redo();
addedSphere = GameNodeMarker.AddSphere(memento.Parent, position: memento.Position, worldSpaceScale: memento.Scale);
if (addedSphere != null)
{
new MarkNetAction(parentID: memento.Parent.name,memento.Position, memento.Scale).Execute();
}
}

/// <summary>
/// Returns a new instance of <see cref="MarkAction"/>.
/// </summary>
/// <returns>new instance</returns>
public static ReversibleAction CreateReversibleAction()
{
return new MarkAction();
}

/// <summary>
/// Returns a new instance of <see cref="MarkAction"/>.
/// </summary>
/// <returns>new instance</returns>
public override ReversibleAction NewInstance()
{
return CreateReversibleAction();
}

/// <summary>
/// Returns the <see cref="ActionStateType"/> of this action.
/// </summary>
/// <returns><see cref="ActionStateType.MarkNode"/></returns>
public override ActionStateType GetActionStateType()
{
return ActionStateType.MarkNode;
}

/// <summary>
/// Returns all IDs of gameObjects manipulated by this action.
/// </summary>
/// <returns>all IDs of gameObjects manipulated by this action</returns>
public override HashSet<string> GetChangedObjects()
{
return new HashSet<string>
{
memento.Parent.name,
memento.NodeID
Copy link
Collaborator

Choose a reason for hiding this comment

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

The conflict resolution via GetChangedObjects() is intended for nodes and edges, not for decorations of these. Since marks can be toggled only via their containing node, the conflict resolution for its containing node should suffice. Moreover, memento.NodeID would need to be unique.

};
}
}
}
71 changes: 71 additions & 0 deletions Assets/SEE/Game/GameNodeMarker.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
using System;
using SEE.DataModel.DG;
using SEE.Game.City;
using SEE.GO;
using SEE.Net;
using SEE.Utils;
using UnityEngine;

namespace SEE.Game
{
/// <summary>
/// This class provides functionality for adding a sphere marker above selected nodes.
/// </summary>
public static class GameNodeMarker
{
/// <summary>
/// Creates and returns a new sphere game object as child of <paramref name="parent"/> at the
/// given <paramref name="position"/>. The diameter of the sphere is the minimum of the width (x axis)
/// and depth (y axis) of <paramref name="worldSpaceScale"/>.
/// </summary>
/// <param name="parent">The node that should be the parent of new marker</param>
/// <param name="position">the position in world space for the center point of the new marker </param>
/// <param name="worldSpaceScale">the scale in world space of the new marker</param>
/// <returns>new marker game object or null if none could be created or a marker already exists</returns>
/// <exception cref="ArgumentNullException">If <paramref name="parent"/> is <c>null</c>.</exception>
public static GameObject AddSphere(GameObject parent, Vector3 position, Vector3 worldSpaceScale)
Copy link
Collaborator

Choose a reason for hiding this comment

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

As stated above, AddSphere could determine the necessary position and scale of the sphere. No need to pass these as parameters.

{
GameObject sphere;

if(parent == null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

There should be a blank between if and (.

{
throw new ArgumentNullException(nameof(parent));
}
else if (DeleteExistingSphere(parent))
{
sphere = null;
}
else
{
sphere = GameObject.CreatePrimitive(PrimitiveType.Sphere);
float diameter = Math.Min(worldSpaceScale.x, worldSpaceScale.y);
sphere.transform.localScale = new Vector3(diameter, diameter, diameter);
sphere.transform.position = new Vector3(position.x, position.y + 0.1f, position.z);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good to use a named constant instead of the magic number 0.1f.

sphere.transform.SetParent(parent.gameObject.transform);
sphere.GetComponent<Renderer>().material.color = Color.red;
}
return sphere;
}

/// <summary>
/// This function searches all children of <paramref name="parent"/> for existing children with the name
/// "Sphere". If a sphere marker is already present it will be destroyed.
/// </summary>
/// <param name="parent">the game object to be checked for existing sphere marker</param>
/// <returns>true if marker exists</returns>
private static bool DeleteExistingSphere(GameObject parent)
{
foreach (Transform child in parent.transform)
{
if (child.name == "Sphere")
{
//FIXME: network operation for deleting an existing marker not working properly
//new DeleteNetAction(sphere.name).Execute();
Copy link
Collaborator

Choose a reason for hiding this comment

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

DeleteNetAction should never be called here. That needs to be handled by the client of GameNodeMarker. This helper class GameNodeMarker whole purpose is to make changes to the scene.

Destroyer.DestroyGameObject(child.gameObject);
return true;
}
}
return false;
}
}
}
72 changes: 72 additions & 0 deletions Assets/SEE/Net/Actions/MarkNetAction.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
using SEE.Controls.Actions;
using SEE.Game;
using UnityEngine;

namespace SEE.Net.Actions
{
/// <summary>
/// This class is responsible for marking a node via network from one client to all others and to the server.
/// </summary>
public class MarkNetAction : AbstractNetAction
{
// Note: All attributes are made public so that they will be serialized
// for the network transfer.

/// <summary>
/// The ID of the parent gameObject of the new GameObject.
/// </summary>
public string ParentID;

/// <summary>
/// The position of the new marker.
/// </summary>
public Vector3 Position;

/// <summary>
/// The scale of the new marker.
/// </summary>
public Vector3 Scale;

/// <summary>
/// Constructor.
/// </summary>
/// <param name="parentID">unique ID of the parent in which to add the new marker</param>
/// <param name="position">the position for the new marker</param>
/// <param name="scale">the scale of the new marker in world space</param>
public MarkNetAction
(string parentID,
Vector3 position,
Vector3 scale)
: base()
{
this.ParentID = parentID;
this.Position = position;
this.Scale = scale;
}

/// <summary>
/// Things to execute on the server (none for this class). Necessary because it is abstract
/// in the superclass.
/// </summary>
protected override void ExecuteOnServer()
{
// Intentionally left blank.
}

/// <summary>
/// Creates a new GameObject on each client.
/// </summary>
protected override void ExecuteOnClient()
{
if (!IsRequester())
{
GameObject parent = GraphElementIDMap.Find(ParentID);
if (parent == null)
{
throw new System.Exception($"There is no node with the ID {ParentID}.");
}
GameNodeMarker.AddSphere(parent, Position, Scale);
}
}
}
}