Skip to content

Commit

Permalink
fix: Misc Code Smells (#257)
Browse files Browse the repository at this point in the history 
* fix: code smells - remove unused usings

* fix: code smells - remove default inits

* fix: code smells - fix modifiers

* fix: code smells - casing fixes

* fix: code smells - remove unused code

* fix: code smells - rename interface to match method

* fix: code smells - split code into methods and reduce ifs
  • Loading branch information
@uweeby
uweeby committed Jul 11, 2020
1 parent 6d92d14 commit 278a127
Show file tree
Hide file tree
Showing 22 changed files with 123 additions and 135 deletions.
4 changes: 2 additions & 2 deletions Assets/Mirror/Cloud/Core/Logger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Mirror.Cloud
{
public static class Logger
{
public static bool VerboseLogging = false;
public readonly static bool VerboseLogging;
static readonly ILogger logger = LogFactory.GetLogger("MirrorCloudServices");

public static void LogRequest(string page, string method, bool hasJson, string json)
Expand All @@ -28,7 +28,7 @@ public static void LogResponse(UnityWebRequest statusRequest)
? LogType.Log
: LogType.Error;

string format = "Response: {0} {1} {2} {3}";
const string format = "Response: {0} {1} {2} {3}";
if (logger.IsLogTypeAllowed(logType))
{
// we split path like this to make sure api key doesn't leak
Expand Down
10 changes: 5 additions & 5 deletions Assets/Mirror/Cloud/ListServer/ListServerClientApi.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void Shutdown()

public void GetServerList()
{
runner.StartCoroutine(getServerList());
runner.StartCoroutine(InternalGetServerList());
}

public void StartGetServerListRepeat(int interval)
Expand All @@ -50,17 +50,17 @@ IEnumerator GetServerListRepeat(int interval)
{
while (true)
{
yield return getServerList();
yield return InternalGetServerList();

yield return new WaitForSeconds(interval);
}
}
IEnumerator getServerList()
IEnumerator InternalGetServerList()
{
UnityWebRequest request = requestCreator.Get("servers");
yield return requestCreator.SendRequestEnumerator(request, onSuccess);
yield return requestCreator.SendRequestEnumerator(request, OnSuccess);

void onSuccess(string responseBody)
void OnSuccess(string responseBody)
{
ServerCollectionJson serverlist = JsonUtility.FromJson<ServerCollectionJson>(responseBody);
_onServerListUpdated?.Invoke(serverlist);
Expand Down
36 changes: 18 additions & 18 deletions Assets/Mirror/Cloud/ListServer/ListServerServerApi.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public sealed class ListServerServerApi : ListServerBaseApi, IListServerServerAp
/// <summary>
/// How many failed pings in a row
/// </summary>
int pingFails = 0;
int pingFails;

public bool ServerInList => added;

Expand All @@ -38,10 +38,10 @@ public ListServerServerApi(ICoroutineRunner runner, IRequestCreator requestCreat

public void Shutdown()
{
stopPingCoroutine();
StopPingCoroutine();
if (added)
{
removeServerWithoutCoroutine();
InternalRemoveServerWithoutCoroutine();
}
added = false;
}
Expand All @@ -52,7 +52,7 @@ public void AddServer(ServerJson server)
bool valid = ValidateServerJson(server);
if (!valid) { return; }

runner.StartCoroutine(addServer(server));
runner.StartCoroutine(InternalAddServer(server));
}

bool ValidateServerJson(ServerJson server)
Expand Down Expand Up @@ -96,18 +96,18 @@ public void UpdateServer(ServerJson server)
maxPlayerCount = server.maxPlayerCount,
};

runner.StartCoroutine(updateServer(partialServer));
runner.StartCoroutine(InternalUpdateServer(partialServer));
}

public void RemoveServer()
{
if (!added) { return; }

stopPingCoroutine();
runner.StartCoroutine(removeServer());
StopPingCoroutine();
runner.StartCoroutine(InternalRemoveServer());
}

void stopPingCoroutine()
void StopPingCoroutine()
{
if (_pingCoroutine != null)
{
Expand All @@ -116,7 +116,7 @@ void stopPingCoroutine()
}
}

IEnumerator addServer(ServerJson server)
IEnumerator InternalAddServer(ServerJson server)
{
added = true;
sending = true;
Expand All @@ -132,15 +132,15 @@ void onSuccess(string responseBody)
serverId = created.id;

// Start ping to keep server alive
_pingCoroutine = runner.StartCoroutine(ping());
_pingCoroutine = runner.StartCoroutine(InternalPing());
}
void onFail(string responseBody)
{
added = false;
}
}

IEnumerator updateServer(PartialServerJson server)
IEnumerator InternalUpdateServer(PartialServerJson server)
{
// wait to not be sending
while (sending)
Expand All @@ -162,7 +162,7 @@ void onSuccess(string responseBody)

if (_pingCoroutine == null)
{
_pingCoroutine = runner.StartCoroutine(ping());
_pingCoroutine = runner.StartCoroutine(InternalPing());
}
}
}
Expand All @@ -171,7 +171,7 @@ void onSuccess(string responseBody)
/// Keeps server alive in database
/// </summary>
/// <returns></returns>
IEnumerator ping()
IEnumerator InternalPing()
{
while (pingFails <= MaxPingFails)
{
Expand All @@ -180,25 +180,25 @@ IEnumerator ping()

sending = true;
UnityWebRequest request = requestCreator.Patch("servers/" + serverId, new EmptyJson());
yield return requestCreator.SendRequestEnumerator(request, onSuccess, onFail);
yield return requestCreator.SendRequestEnumerator(request, OnSuccess, OnFail);
sending = false;
}

Logger.LogWarning("Max ping fails reached, stoping to ping server");
_pingCoroutine = null;


void onSuccess(string responseBody)
void OnSuccess(string responseBody)
{
pingFails = 0;
}
void onFail(string responseBody)
void OnFail(string responseBody)
{
pingFails++;
}
}

IEnumerator removeServer()
IEnumerator InternalRemoveServer()
{
sending = true;
UnityWebRequest request = requestCreator.Delete("servers/" + serverId);
Expand All @@ -208,7 +208,7 @@ IEnumerator removeServer()
added = false;
}

void removeServerWithoutCoroutine()
void InternalRemoveServerWithoutCoroutine()
{
UnityWebRequest request = requestCreator.Delete("servers/" + serverId);
UnityWebRequestAsyncOperation operation = request.SendWebRequest();
Expand Down
10 changes: 4 additions & 6 deletions Assets/Mirror/Components/Experimental/NetworkRigidbody.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,29 @@ public class NetworkRigidbody : NetworkBehaviour
static readonly ILogger logger = LogFactory.GetLogger(typeof(NetworkRigidbody));

[Header("Settings")]
[SerializeField] internal Rigidbody target = null;
[SerializeField] internal Rigidbody target;

[Tooltip("Set to true if moves come from owner client, set to false if moves always come from server")]
[SerializeField] bool clientAuthority = false;
[SerializeField] bool clientAuthority;

[Header("Velocity")]

[Tooltip("Syncs Velocity every SyncInterval")]
[SerializeField] bool syncVelocity = true;

[Tooltip("Set velocity to 0 each frame (only works if syncVelocity is false")]
[SerializeField] bool clearVelocity = false;
[SerializeField] bool clearVelocity;

[Tooltip("Only Syncs Value if distance between previous and current is great than sensitivity")]
[SerializeField] float velocitySensitivity = 0.1f;


[Header("Angular Velocity")]

[Tooltip("Syncs AngularVelocity every SyncInterval")]
[SerializeField] bool syncAngularVelocity = true;

[Tooltip("Set angularVelocity to 0 each frame (only works if syncAngularVelocity is false")]
[SerializeField] bool clearAngularVelocity = false;
[SerializeField] bool clearAngularVelocity;

[Tooltip("Only Syncs Value if distance between previous and current is great than sensitivity")]
[SerializeField] float angularVelocitySensitivity = 0.1f;
Expand All @@ -50,7 +49,6 @@ void OnValidate()
}
}


#region Sync vars
[SyncVar(hook = nameof(OnVelocityChanged))]
Vector3 velocity;
Expand Down
2 changes: 1 addition & 1 deletion Assets/Mirror/Components/Experimental/NetworkTransform.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ namespace Mirror.Experimental
[HelpURL("https://mirror-networking.com/docs/Components/NetworkTransform.html")]
public class NetworkTransform : NetworkTransformBase
{
protected override Transform targetTransform => transform;
protected override Transform TargetTransform => transform;
}
}
58 changes: 29 additions & 29 deletions Assets/Mirror/Components/Experimental/NetworkTransformBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace Mirror.Experimental
public abstract class NetworkTransformBase : NetworkBehaviour
{
// target transform to sync. can be on a child.
protected abstract Transform targetTransform { get; }
protected abstract Transform TargetTransform { get; }

[Header("Authority")]

Expand Down Expand Up @@ -98,7 +98,7 @@ public struct DataPoint
public Vector3 localScale;
public float movementSpeed;

public bool isValid => timeStamp != 0;
public bool IsValid => timeStamp != 0;
}

// Is this a client with authority over this transform?
Expand All @@ -117,7 +117,7 @@ void FixedUpdate()
// let the clients know that this has moved
if (HasEitherMovedRotatedScaled())
{
RpcMove(targetTransform.localPosition, targetTransform.localRotation, targetTransform.localScale);
RpcMove(TargetTransform.localPosition, TargetTransform.localRotation, TargetTransform.localScale);
}
}

Expand All @@ -134,11 +134,11 @@ void FixedUpdate()
// serialize
// local position/rotation for VR support
// send to server
CmdClientToServerSync(targetTransform.localPosition, targetTransform.localRotation, targetTransform.localScale);
CmdClientToServerSync(TargetTransform.localPosition, TargetTransform.localRotation, TargetTransform.localScale);
}
}
}
else if (goal.isValid)
else if (goal.IsValid)
{
// teleport or interpolate
if (NeedsTeleport())
Expand All @@ -153,9 +153,9 @@ void FixedUpdate()
else
{
// local position/rotation for VR support
ApplyPositionRotationScale(InterpolatePosition(start, goal, targetTransform.localPosition),
InterpolateRotation(start, goal, targetTransform.localRotation),
InterpolateScale(start, goal, targetTransform.localScale));
ApplyPositionRotationScale(InterpolatePosition(start, goal, TargetTransform.localPosition),
InterpolateRotation(start, goal, TargetTransform.localRotation),
InterpolateScale(start, goal, TargetTransform.localScale));
}

}
Expand All @@ -172,9 +172,9 @@ bool HasEitherMovedRotatedScaled()
if (changed)
{
// local position/rotation for VR support
if (syncPosition) lastPosition = targetTransform.localPosition;
if (syncRotation) lastRotation = targetTransform.localRotation;
if (syncScale) lastScale = targetTransform.localScale;
if (syncPosition) lastPosition = TargetTransform.localPosition;
if (syncRotation) lastRotation = TargetTransform.localRotation;
if (syncScale) lastScale = TargetTransform.localScale;
}
return changed;
}
Expand All @@ -183,18 +183,18 @@ bool HasEitherMovedRotatedScaled()
// SqrMagnitude is faster than Distance per Unity docs
// https://docs.unity3d.com/ScriptReference/Vector3-sqrMagnitude.html

bool HasMoved => syncPosition && Vector3.SqrMagnitude(lastPosition - targetTransform.localPosition) > localPositionSensitivity * localPositionSensitivity;
bool HasRotated => syncRotation && Quaternion.Angle(lastRotation, targetTransform.localRotation) > localRotationSensitivity;
bool HasScaled => syncScale && Vector3.SqrMagnitude(lastScale - targetTransform.localScale) > localScaleSensitivity * localScaleSensitivity;
bool HasMoved => syncPosition && Vector3.SqrMagnitude(lastPosition - TargetTransform.localPosition) > localPositionSensitivity * localPositionSensitivity;
bool HasRotated => syncRotation && Quaternion.Angle(lastRotation, TargetTransform.localRotation) > localRotationSensitivity;
bool HasScaled => syncScale && Vector3.SqrMagnitude(lastScale - TargetTransform.localScale) > localScaleSensitivity * localScaleSensitivity;

// teleport / lag / stuck detection
// - checking distance is not enough since there could be just a tiny fence between us and the goal
// - checking time always works, this way we just teleport if we still didn't reach the goal after too much time has elapsed
bool NeedsTeleport()
{
// calculate time between the two data points
float startTime = start.isValid ? start.timeStamp : Time.time - Time.fixedDeltaTime;
float goalTime = goal.isValid ? goal.timeStamp : Time.time;
float startTime = start.IsValid ? start.timeStamp : Time.time - Time.fixedDeltaTime;
float goalTime = goal.IsValid ? goal.timeStamp : Time.time;
float difference = goalTime - startTime;
float timeSinceGoalReceived = Time.time - goalTime;
return timeSinceGoalReceived > difference * 5;
Expand Down Expand Up @@ -241,7 +241,7 @@ void SetGoal(Vector3 position, Quaternion rotation, Vector3 scale)
};

// movement speed: based on how far it moved since last time has to be calculated before 'start' is overwritten
temp.movementSpeed = EstimateMovementSpeed(goal, temp, targetTransform, Time.fixedDeltaTime);
temp.movementSpeed = EstimateMovementSpeed(goal, temp, TargetTransform, Time.fixedDeltaTime);

// reassign start wisely
// first ever data point? then make something up for previous one so that we can start interpolation without waiting for next.
Expand All @@ -251,9 +251,9 @@ void SetGoal(Vector3 position, Quaternion rotation, Vector3 scale)
{
timeStamp = Time.time - Time.fixedDeltaTime,
// local position/rotation for VR support
localPosition = targetTransform.localPosition,
localRotation = targetTransform.localRotation,
localScale = targetTransform.localScale,
localPosition = TargetTransform.localPosition,
localRotation = TargetTransform.localRotation,
localScale = TargetTransform.localScale,
movementSpeed = temp.movementSpeed
};
}
Expand Down Expand Up @@ -295,11 +295,11 @@ void SetGoal(Vector3 position, Quaternion rotation, Vector3 scale)
// local position/rotation for VR support
// teleport / lag / obstacle detection: only continue at current position if we aren't too far away
// XC < AB + BC (see comments above)
if (Vector3.Distance(targetTransform.localPosition, start.localPosition) < oldDistance + newDistance)
if (Vector3.Distance(TargetTransform.localPosition, start.localPosition) < oldDistance + newDistance)
{
start.localPosition = targetTransform.localPosition;
start.localRotation = targetTransform.localRotation;
start.localScale = targetTransform.localScale;
start.localPosition = TargetTransform.localPosition;
start.localRotation = TargetTransform.localRotation;
start.localScale = TargetTransform.localScale;
}
}

Expand All @@ -314,7 +314,7 @@ void SetGoal(Vector3 position, Quaternion rotation, Vector3 scale)
static float EstimateMovementSpeed(DataPoint from, DataPoint to, Transform transform, float sendInterval)
{
Vector3 delta = to.localPosition - (from.localPosition != transform.localPosition ? from.localPosition : transform.localPosition);
float elapsed = from.isValid ? to.timeStamp - from.timeStamp : sendInterval;
float elapsed = from.IsValid ? to.timeStamp - from.timeStamp : sendInterval;

// avoid NaN
return elapsed > 0 ? delta.magnitude / elapsed : 0;
Expand All @@ -324,9 +324,9 @@ static float EstimateMovementSpeed(DataPoint from, DataPoint to, Transform trans
void ApplyPositionRotationScale(Vector3 position, Quaternion rotation, Vector3 scale)
{
// local position/rotation for VR support
if (syncPosition) targetTransform.localPosition = position;
if (syncRotation) targetTransform.localRotation = rotation;
if (syncScale) targetTransform.localScale = scale;
if (syncPosition) TargetTransform.localPosition = position;
if (syncRotation) TargetTransform.localRotation = rotation;
if (syncScale) TargetTransform.localScale = scale;
}

// where are we in the timeline between start and goal? [0,1]
Expand Down Expand Up @@ -382,7 +382,7 @@ Vector3 InterpolateScale(DataPoint start, DataPoint goal, Vector3 currentScale)

static float CurrentInterpolationFactor(DataPoint start, DataPoint goal)
{
if (start.isValid)
if (start.IsValid)
{
float difference = goal.timeStamp - start.timeStamp;

Expand Down

0 comments on commit 278a127

Please sign in to comment.