Skip to content

Commit

Permalink
feat: user friendly weaver error (#896)
Browse files Browse the repository at this point in the history
  • Loading branch information
@paulpach @miwarnec
paulpach authored and miwarnec committed Jun 9, 2019
1 parent f39cded commit 954a3d5
Show file tree
Hide file tree
Showing 16 changed files with 176 additions and 199 deletions.
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Processors/CommandProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,13 @@ public static bool ProcessMethodsValidateCommand(TypeDefinition td, MethodDefini
{
if (!md.Name.StartsWith("Cmd"))
{
Weaver.Error("Command function [" + td.FullName + ":" + md.Name + "] doesnt have 'Cmd' prefix");
Weaver.Error($"{md} must start with Cmd. Consider renaming it to Cmd{md.Name}");
return false;
}

if (md.IsStatic)
{
Weaver.Error("Command function [" + td.FullName + ":" + md.Name + "] cant be a static method");
Weaver.Error($"{md} cannot be static");
return false;
}

Expand Down
10 changes: 5 additions & 5 deletions Assets/Mirror/Editor/Weaver/Processors/MessageClassProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ static void GenerateSerialization(TypeDefinition td)
{
if (field.FieldType.FullName == td.FullName)
{
Weaver.Error("GenerateSerialization for " + td.Name + " [" + field.FullName + "]. [MessageBase] member cannot be self referencing.");
Weaver.Error($"{td} has field ${field} that references itself");
return;
}
}
Expand All @@ -58,13 +58,13 @@ static void GenerateSerialization(TypeDefinition td)

if (field.FieldType.Resolve().HasGenericParameters)
{
Weaver.Error("GenerateSerialization for " + td.Name + " [" + field.FieldType + "/" + field.FieldType.FullName + "]. [MessageBase] member cannot have generic parameters.");
Weaver.Error($"{field} cannot have generic type {field.FieldType}. Consider creating a class that derives the generic type");
return;
}

if (field.FieldType.Resolve().IsInterface)
{
Weaver.Error("GenerateSerialization for " + td.Name + " [" + field.FieldType + "/" + field.FieldType.FullName + "]. [MessageBase] member cannot be an interface.");
Weaver.Error($"{field} has unsupported type. Use a concrete class instead of interface {field.FieldType}");
return;
}

Expand All @@ -78,7 +78,7 @@ static void GenerateSerialization(TypeDefinition td)
}
else
{
Weaver.Error("GenerateSerialization for " + td.Name + " unknown type [" + field.FieldType + "/" + field.FieldType.FullName + "]. [MessageBase] member variables must be basic types.");
Weaver.Error($"{field} has unsupported type");
return;
}
}
Expand Down Expand Up @@ -123,7 +123,7 @@ static void GenerateDeSerialization(TypeDefinition td)
}
else
{
Weaver.Error("GenerateDeSerialization for " + td.Name + " unknown type [" + field.FieldType + "]. [SyncVar] member variables must be basic types.");
Weaver.Error($"{field} has unsupported type");
return;
}
}
Expand Down
18 changes: 9 additions & 9 deletions Assets/Mirror/Editor/Weaver/Processors/MonoBehaviourProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ static void ProcessSyncVars(TypeDefinition td)
{
if (ca.AttributeType.FullName == Weaver.SyncVarType.FullName)
{
Weaver.Error("Script " + td.FullName + " uses [SyncVar] " + fd.Name + " but is not a NetworkBehaviour.");
Weaver.Error($"[SyncVar] {fd} must be inside a NetworkBehaviour. {td} is not a NetworkBehaviour");
}
}

if (SyncObjectInitializer.ImplementsSyncObject(fd.FieldType))
{
Weaver.Error(string.Format("Script {0} defines field {1} with type {2}, but it's not a NetworkBehaviour", td.FullName, fd.Name, Helpers.PrettyPrintType(fd.FieldType)));
Weaver.Error($"{fd} is a SyncObject and must be inside a NetworkBehaviour. {td} is not a NetworkBehaviour");
}
}
}
Expand All @@ -40,34 +40,34 @@ static void ProcessMethods(TypeDefinition td)
{
if (ca.AttributeType.FullName == Weaver.CommandType.FullName)
{
Weaver.Error("Script " + td.FullName + " uses [Command] " + md.Name + " but is not a NetworkBehaviour.");
Weaver.Error($"[Command] {md} must be declared inside a NetworkBehaviour");
}

if (ca.AttributeType.FullName == Weaver.ClientRpcType.FullName)
{
Weaver.Error("Script " + td.FullName + " uses [ClientRpc] " + md.Name + " but is not a NetworkBehaviour.");
Weaver.Error($"[ClienRpc] {md} must be declared inside a NetworkBehaviour");
}

if (ca.AttributeType.FullName == Weaver.TargetRpcType.FullName)
{
Weaver.Error("Script " + td.FullName + " uses [TargetRpc] " + md.Name + " but is not a NetworkBehaviour.");
Weaver.Error($"[TargetRpc] {md} must be declared inside a NetworkBehaviour");
}

string attributeName = ca.Constructor.DeclaringType.ToString();

switch (attributeName)
{
case "Mirror.ServerAttribute":
Weaver.Error("Script " + td.FullName + " uses the attribute [Server] on the method " + md.Name + " but is not a NetworkBehaviour.");
Weaver.Error($"[Server] {md} must be declared inside a NetworkBehaviour");
break;
case "Mirror.ServerCallbackAttribute":
Weaver.Error("Script " + td.FullName + " uses the attribute [ServerCallback] on the method " + md.Name + " but is not a NetworkBehaviour.");
Weaver.Error($"[ServerCallback] {md} must be declared inside a NetworkBehaviour");
break;
case "Mirror.ClientAttribute":
Weaver.Error("Script " + td.FullName + " uses the attribute [Client] on the method " + md.Name + " but is not a NetworkBehaviour.");
Weaver.Error($"[Client] {md} must be declared inside a NetworkBehaviour");
break;
case "Mirror.ClientCallbackAttribute":
Weaver.Error("Script " + td.FullName + " uses the attribute [ClientCallback] on the method " + md.Name + " but is not a NetworkBehaviour.");
Weaver.Error($"[ClientCallback] {md} must be declared inside a NetworkBehaviour");
break;
}
}
Expand Down
40 changes: 18 additions & 22 deletions Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void Process()
{
if (netBehaviourSubclass.HasGenericParameters)
{
Weaver.Error("NetworkBehaviour " + netBehaviourSubclass.Name + " cannot have generic parameters");
Weaver.Error($"{netBehaviourSubclass} cannot have generic parameters");
return;
}
Weaver.DLog(netBehaviourSubclass, "Process Start");
Expand Down Expand Up @@ -126,7 +126,7 @@ public static bool WriteArguments(ILProcessor worker, MethodDefinition md, bool
MethodReference writeFunc = Writers.GetWriteFunc(pd.ParameterType);
if (writeFunc == null)
{
Weaver.Error("WriteArguments for " + md.Name + " type " + pd.ParameterType + " not supported");
Weaver.Error($"{md} has invalid parameter {pd}" );
return false;
}
// use built-in writer func on writer object
Expand Down Expand Up @@ -189,7 +189,7 @@ void GenerateConstants()
}
else
{
Weaver.Error("No cctor for " + netBehaviourSubclass.Name);
Weaver.Error($"{netBehaviourSubclass} has invalid class constructor");
return;
}
}
Expand Down Expand Up @@ -221,7 +221,7 @@ void GenerateConstants()
}
else
{
Weaver.Error("No ctor for " + netBehaviourSubclass.Name);
Weaver.Error($"{netBehaviourSubclass} has invalid constructor");
return;
}

Expand All @@ -231,7 +231,7 @@ void GenerateConstants()

if (ctor == null)
{
Weaver.Error("No ctor for " + netBehaviourSubclass.Name);
Weaver.Error($"{netBehaviourSubclass} has invalid constructor");
return;
}

Expand Down Expand Up @@ -350,7 +350,7 @@ void GenerateSerialization()
}
else
{
Weaver.Error("GenerateSerialization for " + netBehaviourSubclass.Name + " unknown type [" + syncVar.FieldType + "]. Mirror [SyncVar] member variables must be basic types.");
Weaver.Error($"{syncVar} has unsupported type. Use a supported Mirror type instead");
return;
}
}
Expand Down Expand Up @@ -398,7 +398,7 @@ void GenerateSerialization()
}
else
{
Weaver.Error("GenerateSerialization for " + netBehaviourSubclass.Name + " unknown type [" + syncVar.FieldType + "]. Mirror [SyncVar] member variables must be basic types.");
Weaver.Error($"{syncVar} has unsupported type. Use a supported Mirror type instead");
return;
}

Expand Down Expand Up @@ -486,7 +486,7 @@ void DeserializeField(FieldDefinition syncVar, ILProcessor serWorker, MethodDefi
MethodReference readFunc = Readers.GetReadFunc(syncVar.FieldType);
if (readFunc == null)
{
Weaver.Error("GenerateDeSerialization for " + netBehaviourSubclass.Name + " unknown type [" + syncVar.FieldType + "]. Mirror [SyncVar] member variables must be basic types.");
Weaver.Error($"{syncVar} has unsupported type. Use a supported Mirror type instead");
return;
}
VariableDefinition tmpValue = new VariableDefinition(syncVar.FieldType);
Expand Down Expand Up @@ -629,7 +629,7 @@ public static bool ProcessNetworkReaderParameters(TypeDefinition td, MethodDefin
}
else
{
Weaver.Error("ProcessNetworkReaderParameters for " + td.Name + ":" + md.Name + " type " + arg.ParameterType + " not supported");
Weaver.Error($"{md} has invalid parameter {arg}. Unsupported type {arg.ParameterType}, use a supported Mirror type instead");
return false;
}
}
Expand All @@ -646,17 +646,17 @@ public static bool ProcessMethodsValidateFunction(TypeDefinition td, MethodRefer
{
if (md.ReturnType.FullName == Weaver.IEnumeratorType.FullName)
{
Weaver.Error(actionType + " function [" + td.FullName + ":" + md.Name + "] cannot be a coroutine");
Weaver.Error($"{md} cannot be a coroutine");
return false;
}
if (md.ReturnType.FullName != Weaver.voidType.FullName)
{
Weaver.Error(actionType + " function [" + td.FullName + ":" + md.Name + "] must have a void return type.");
Weaver.Error($"{md} cannot return a value. Make it void instead");
return false;
}
if (md.HasGenericParameters)
{
Weaver.Error(actionType + " [" + td.FullName + ":" + md.Name + "] cannot have generic parameters");
Weaver.Error($"{md} cannot have generic parameters");
return false;
}
return true;
Expand All @@ -669,40 +669,36 @@ public static bool ProcessMethodsValidateParameters(TypeDefinition td, MethodRef
ParameterDefinition p = md.Parameters[i];
if (p.IsOut)
{
Weaver.Error(actionType + " function [" + td.FullName + ":" + md.Name + "] cannot have out parameters");
Weaver.Error($"{md} cannot have out parameters");
return false;
}
if (p.IsOptional)
{
Weaver.Error(actionType + "function [" + td.FullName + ":" + md.Name + "] cannot have optional parameters");
Weaver.Error($"{md} cannot have optional parameters");
return false;
}
if (p.ParameterType.Resolve().IsAbstract)
{
Weaver.Error(actionType + " function [" + td.FullName + ":" + md.Name + "] cannot have abstract parameters");
Weaver.Error($"{md} has invalid parameter {p}. Use concrete type instead of abstract type {p.ParameterType}");
return false;
}
if (p.ParameterType.IsByReference)
{
Weaver.Error(actionType + " function [" + td.FullName + ":" + md.Name + "] cannot have ref parameters");
Weaver.Error($"{md} has invalid parameter {p}. Use supported type instead of reference type {p.ParameterType}");
return false;
}
// TargetRPC is an exception to this rule and can have a NetworkConnection as first parameter
if (p.ParameterType.FullName == Weaver.NetworkConnectionType.FullName &&
!(ca.AttributeType.FullName == Weaver.TargetRpcType.FullName && i == 0))
{
Weaver.Error(actionType + " [" + td.FullName + ":" + md.Name + "] cannot use a NetworkConnection as a parameter. To access a player object's connection on the server use connectionToClient");
Weaver.Error("Name: " + ca.AttributeType.FullName + " parameter: " + md.Parameters[0].ParameterType.FullName);
Weaver.Error($"{md} has invalid parameer {p}. Cannot pass NeworkConnections");
return false;
}
if (p.ParameterType.Resolve().IsDerivedFrom(Weaver.ComponentType))
{
if (p.ParameterType.FullName != Weaver.NetworkIdentityType.FullName)
{
Weaver.Error(actionType + " function [" + td.FullName + ":" + md.Name + "] parameter [" + p.Name +
"] is of the type [" +
p.ParameterType.Name +
"] which is a Component. You cannot pass a Component to a remote call. Try passing data from within the component.");
Weaver.Error($"{md} has invalid parameter {p}. Cannot pass components in remote method calls");
return false;
}
}
Expand Down
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Processors/PropertySiteProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ static void InjectServerGuard(TypeDefinition td, MethodDefinition md, bool logWa
{
if (!Weaver.IsNetworkBehaviour(td))
{
Log.Error("[Server] guard on non-NetworkBehaviour script at [" + md.FullName + "]");
Weaver.Error($"[Server] {md} must be declared in a NetworkBehaviour");
return;
}
ILProcessor worker = md.Body.GetILProcessor();
Expand All @@ -119,7 +119,7 @@ static void InjectClientGuard(TypeDefinition td, MethodDefinition md, bool logWa
{
if (!Weaver.IsNetworkBehaviour(td))
{
Log.Error("[Client] guard on non-NetworkBehaviour script at [" + md.FullName + "]");
Weaver.Error($"[Client] {md} must be declared in a NetworkBehaviour");
return;
}
ILProcessor worker = md.Body.GetILProcessor();
Expand Down
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ public static bool ProcessMethodsValidateRpc(TypeDefinition td, MethodDefinition
{
if (!md.Name.StartsWith("Rpc"))
{
Weaver.Error("Rpc function [" + td.FullName + ":" + md.Name + "] doesnt have 'Rpc' prefix");
Weaver.Error($"{md} must start with Rpc. Consider renaming it to Rpc{md.Name}");
return false;
}

if (md.IsStatic)
{
Weaver.Error("ClientRpc function [" + td.FullName + ":" + md.Name + "] cant be a static method");
Weaver.Error($"{md} must not be static");
return false;
}

Expand Down
6 changes: 3 additions & 3 deletions Assets/Mirror/Editor/Weaver/Processors/SyncEventProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static MethodDefinition ProcessEventInvoke(TypeDefinition td, EventDefini
}
if (eventField == null)
{
Weaver.Error("[" + td.Name + "] ERROR: no event field?!");
Weaver.Error($"{td} not found. Did you declare the event?");
return null;
}

Expand Down Expand Up @@ -113,13 +113,13 @@ public static void ProcessEvents(TypeDefinition td, List<EventDefinition> events
{
if (!ed.Name.StartsWith("Event"))
{
Weaver.Error("Event [" + td.FullName + ":" + ed.FullName + "] doesnt have 'Event' prefix");
Weaver.Error($"{ed} must start with Event. Consider renaming it to Event{ed.Name}");
return;
}

if (ed.EventType.Resolve().HasGenericParameters)
{
Weaver.Error("Event [" + td.FullName + ":" + ed.FullName + "] cannot have generic parameters");
Weaver.Error($"{ed} must not have generic parameters. Consider creating a new class that inherits from {ed.EventType} instead");
return;
}

Expand Down
2 changes: 1 addition & 1 deletion Assets/Mirror/Editor/Weaver/Processors/SyncObjectInitializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ static void GenerateSyncObjectInstanceInitializer(ILProcessor ctorWorker, FieldD
}
catch (Exception)
{
Weaver.Error("Missing parameter-less constructor for:" + fd.FieldType.Name);
Weaver.Error($"{fd} does not have a default constructor");
return;
}

Expand Down

0 comments on commit 954a3d5

Please sign in to comment.