Jump to content
  • Advertisement
  • 04/17/18 02:05 PM

    Checking the Unity C# Source Code

    General and Gameplay Programming

    PVS-studio team

    Recently a long-awaited event has happen - Unity Technologies uploaded the C# source code of the game engine, available for free download on Github. The code of the engine and the editor is available. Of course, we couldn't pass up, especially since lately we've not written so many articles about checking projects on C#. Unity allows to use the provided sources only for information purposes. We'll use them exactly in these ways. Let's try out the latest version PVS-Studio 6.23 on the Unity code.

    image1.png.df7030833ddc0a13410261e6b3e2d5a8.png

    Introduction

    Previously we've written an article about checking Unity. At that time so much C#-code was not available for the analysis: some components, libraries and examples of usage. However, the author of the article managed to find quite interesting bugs.

    How did Unity please us this time? I'm saying "please" and hope not to offend the authors of the project. Especially since the amount of the source Unity C#-code, presented on GitHub, is about 400 thousand lines (excluding empty) in 2058 files with the extension "cs". It's a lot, and the analyzer had a quite considerable scope.

    Now about the results. Before the analysis, I've slightly simplified the work, having enabled the mode of the code display according to the CWE classification for the found bugs. I've also activated the warnings suppression mechanism of the third level of certainty (Low). These settings are available in the drop-down menu of PVS-Studio in Visual Studio development environment, and in the parameters of the analyzer. Getting rid of the warnings with low certainty, I made the analysis of the Unity source code. As a result, I got 181 warnings of the first level of certainty (High) and 506 warnings of the second level of certainty (Medium).

    I have not studied absolutely all the warnings, because there were quite a lot of them. Developers or enthusiasts can easily conduct an in-depth analysis by testing Unity themselves. To do this, PVS-Studio provides free trial and free modes of using. Companies can also buy our product and get quick and detailed support along with the license.

    Judging by the fact that I immediately managed to find couple of real bugs practically in every group of warnings with one or two attempts, there are a lot of them in Unity. And yes, they are diverse. Let's review the most interesting errors.

    Results of the check

    Something's wrong with the flags

    PVS-Studio warning: V3001 There are identical sub-expressions 'MethodAttributes.Public' to the left and to the right of the '|' operator. SyncListStructProcessor.cs 240

    MethodReference GenerateSerialization()
    {
      ....
      MethodDefinition serializeFunc = new
          MethodDefinition("SerializeItem", MethodAttributes.Public |
                MethodAttributes.Virtual |
                MethodAttributes.Public |  // <=
                MethodAttributes.HideBySig,
                Weaver.voidType);
      ....
    }

    When combining enumeration flags MethodAttributes, an error was made: the Public value was used twice. Perhaps, the reason for this is the wrong code formatting.

    A similar bug is also made in code of the method GenerateDeserialization:

    • V3001 There are identical sub-expressions 'MethodAttributes.Public' to the left and to the right of the '|' operator. SyncListStructProcessor.cs 309

    Copy-Paste

    PVS-Studio warning: V3001 There are identical sub-expressions 'format == RenderTextureFormat.ARGBFloat' to the left and to the right of the '||' operator. RenderTextureEditor.cs 87

    public static bool IsHDRFormat(RenderTextureFormat format)
    {
      Return (format == RenderTextureFormat.ARGBHalf ||
        format == RenderTextureFormat.RGB111110Float ||
        format == RenderTextureFormat.RGFloat ||
        format == RenderTextureFormat.ARGBFloat ||
        format == RenderTextureFormat.ARGBFloat ||
        format == RenderTextureFormat.RFloat ||
        format == RenderTextureFormat.RGHalf ||
        format == RenderTextureFormat.RHalf);
    }

    I gave a piece of code, preliminary having formatted it, so the error is easily detected visually: the comparison with RenderTextureFormat.ARGBFloat is performed twice. In the original code, it looks differently:

    image3.png.fca6361ca85326090bd992576a8f215a.png

    Probably, another value of enumeration RenderTextureFormat has to be used in one of two identical comparisons.

    Double work

    PVS-Studio warning: V3008 CWE-563 The 'fail' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1633, 1632. UNetWeaver.cs 1633

    class Weaver
    {
      ....
      public static bool fail;
      ....
      static public bool IsValidTypeToGenerate(....)
      {
        ....
        if (....)
        {
          ....
          Weaver.fail = true;
          fail = true;
          return false;
        }
        return true;
      }
    ....
    }

    The true value is assigned twice to the value, as Weaver.fail and fail is one and the same static field of the Weaver class. Perhaps, there is no crucial error, but the code definitely needs attention.

    No options

    PVS-Studio warning: V3009 CWE-393 It's odd that this method always returns one and the same value of 'false'. ProjectBrowser.cs 1417

    // Returns true if we should early out of OnGUI
    bool HandleCommandEventsForTreeView()
    {
      ....
      if (....)
      {
        ....
        if (....)
          return false;
        ....
      }
      return false;
    }

    The method always returns false. Pay attention to the comment in the beginning.

    A developer forgot about the result

    PVS-Studio warning: V3010 CWE-252 The return value of function 'Concat' is required to be utilized. AnimationRecording.cs 455

    static public UndoPropertyModification[] Process(....)
    {
      ....
      discardedModifications.Concat(discardedRotationModifications);
      return discardedModifications.ToArray();
    }

    When concatenating two arrays discardedModifications and discardedRotationModifications the author forgot to save the result. Probably a programmer assumed that the result would be expressed immediately in the array discardedModifications. But it is not so. As a result, the original array discardedModifications is returned from the method. The code needs to be corrected as follows:

    static public UndoPropertyModification[] Process(....)
    {
      ....
      return discardedModifications.Concat(discardedRotationModifications)
                                   .ToArray();
    }

    Wrong variable was checked

    PVS-Studio warning: V3019 CWE-697 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'obj', 'newResolution'. GameViewSizesMenuItemProvider.cs 104

    private static GameViewSize CastToGameViewSize(object obj)
    {
      GameViewSize newResolution = obj as GameViewSize;
      if (obj == null)
      {
        Debug.LogError("Incorrect input");
        return null;
      }
      return newResolution;
    }

    In this method, the developers forgot to consider a situation where the variable objis not equal to null, but it will not be able to cast to the GameViewSize type. Then the variable newResolution will be set to null, and the debug output will not be made. A correct variant of code will be like this:

    private static GameViewSize CastToGameViewSize(object obj)
    {
      GameViewSize newResolution = obj as GameViewSize;
      if (newResolution == null)
      {
        Debug.LogError("Incorrect input");
      }
      return newResolution;
    }

    Deficiency

    PVS-Studio warning: V3020 CWE-670 An unconditional 'return' within a loop. PolygonCollider2DEditor.cs 96

    private void HandleDragAndDrop(Rect targetRect)
    {
      ....
      foreach (....)
      {
        ....
        if (....)
        {
          ....
        }
        return;
      }
      ....
    }

    The loop will execute only one iteration, after that the method terminates its work. Various scenarios are probable. For example, return must be inside the unit if, or somewhere before return, a directive continue is missing. It may well be that there is no error here, but then one should make the code more understandable.

    Unreachable code

    PVS-Studio warning: V3021 CWE-561 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless CustomScriptAssembly.cs 179

    public bool IsCompatibleWith(....)
    {
      ....
      if (buildingForEditor)
        return IsCompatibleWithEditor();
    
      if (buildingForEditor)
        buildTarget = BuildTarget.NoTarget; // Editor
      ....
    }

    Two identical checks, following one after another. It is clear that in case of buildingForEditor equality to the true value, the second check is meaningless, because the first method terminates its work. If the value buildingForEditor is false, neither then-brunch nor if operator will be executed. There is an erroneous construction that requires correction.

    Unconditional condition

    PVS-Studio warning: V3022 CWE-570 Expression 'index < 0 && index >= parameters.Length' is always false. AnimatorControllerPlayable.bindings.cs 287

    public AnimatorControllerParameter GetParameter(int index)
    {
      AnimatorControllerParameter[] param = parameters;
      if (index < 0 && index >= parameters.Length)
        throw new IndexOutOfRangeException(
          "Index must be between 0 and " + parameters.Length);
      return param[index];
    }

    The condition of the index check is incorrect - the result will always be false. However, in case of passing the incorrect index to the GetParameter method, the exception IndexOutOfRangeException will still be thrown when attempting to access an array element in the return block. Although, the error message will be slightly different. One has to use || in a condition instead of the operator && so that the code worked the way a developer expected:

    public AnimatorControllerParameter GetParameter(int index)
    {
      AnimatorControllerParameter[] param = parameters;
      if (index < 0 || index >= parameters.Length)
        throw new IndexOutOfRangeException(
          "Index must be between 0 and " + parameters.Length);
      return param[index];
    }

    Perhaps, due to the use of the Copy-Paste method, there is another the same error in the Unity code:

    PVS-Studio warning: V3022 CWE-570 Expression 'index < 0 && index >= parameters.Length' is always false. Animator.bindings.cs 711

    And another similar error associated with the incorrect condition of the check of the array index:

    PVS-Studio warning: V3022 CWE-570 Expression 'handle.valueIndex < 0 && handle.valueIndex >= list.Length' is always false. StyleSheet.cs 81

    static T CheckAccess<T>(T[] list, StyleValueType type,
      StyleValueHandle handle)
    {
      T value = default(T);
      if (handle.valueType != type)
      {
        Debug.LogErrorFormat(....  );
      }
      else if (handle.valueIndex < 0 && handle.valueIndex >= list.Length)
      {
        Debug.LogError("Accessing invalid property");
      }
      else
      {
        value = list[handle.valueIndex];
      }
      return value;
    }

    And in this case, a release of the IndexOutOfRangeException exception is possible.As in the previous code fragments, one has to use the operator || instead of && to fix an error.

    Simply strange code

    Two warnings are issued for the code fragment below.

    PVS-Studio warning: V3022 CWE-571 Expression 'bRegisterAllDefinitions || (AudioSettings.GetSpatializerPluginName() == "GVR Audio Spatializer")' is always true. AudioExtensions.cs 463

    PVS-Studio warning: V3022 CWE-571 Expression 'bRegisterAllDefinitions || (AudioSettings.GetAmbisonicDecoderPluginName() == "GVR Audio Spatializer")' is always true. AudioExtensions.cs 467

    // This is where we register our built-in spatializer extensions.
    static private void RegisterBuiltinDefinitions()
    {
      bool bRegisterAllDefinitions = true;
      
      if (!m_BuiltinDefinitionsRegistered)
      {
        if (bRegisterAllDefinitions ||
            (AudioSettings.GetSpatializerPluginName() ==
              "GVR Audio Spatializer"))
        {
        }
        
        if (bRegisterAllDefinitions ||
            (AudioSettings.GetAmbisonicDecoderPluginName() ==
              "GVR Audio Spatializer"))
        {
        }
        
        m_BuiltinDefinitionsRegistered = true;
      }
    }

    It looks like an incomplete method. It is unclear why it has been left as such and why developers haven't commented the useless code blocks. All, that the method does at the moment:

    if (!m_BuiltinDefinitionsRegistered)
    {
      m_BuiltinDefinitionsRegistered = true;
    }

    Useless method

    PVS-Studio warning: V3022 CWE-570 Expression 'PerceptionRemotingPlugin.GetConnectionState() != HolographicStreamerConnectionState.Disconnected' is always false. HolographicEmulationWindow.cs 171

    private void Disconnect()
    {
      if (PerceptionRemotingPlugin.GetConnectionState() !=
          HolographicStreamerConnectionState.Disconnected)
        PerceptionRemotingPlugin.Disconnect();
    }

    To clarify the situation, it is necessary to look at the declaration of the methodPerceptionRemotingPlugin.GetConnectionState():

    internal static HolographicStreamerConnectionState
    GetConnectionState()
    {
      return HolographicStreamerConnectionState.Disconnected;
    }

    Thus, calling the Disconnect() method leads to nothing.

    One more error relates to the same method PerceptionRemotingPlugin.GetConnectionState():

    PVS-Studio warning: V3022 CWE-570 Expression 'PerceptionRemotingPlugin.GetConnectionState() == HolographicStreamerConnectionState.Connected' is always false. HolographicEmulationWindow.cs 177

    private bool IsConnectedToRemoteDevice()
    {
      return PerceptionRemotingPlugin.GetConnectionState() ==
             HolographicStreamerConnectionState.Connected;
    }

    The result of the method is equivalent to the following:

    private bool IsConnectedToRemoteDevice()
    {
      return false;
    }

    As we can see, among the warnings V3022 many interesting ones were found. Probably, if one spends much time, he can increase the list. But let's move on.

    Not on the format

    PVS-Studio warning: V3025 CWE-685 Incorrect format. A different number of format items is expected while calling 'Format' function. Arguments not used: index. Physics2D.bindings.cs 2823

    public void SetPath(....)
    {
      if (index < 0)
        throw new ArgumentOutOfRangeException(
          String.Format("Negative path index is invalid.", index));
      ....
    }

    There is no error in code, but as the saying goes, the code "smells". Probably, an earlier message was more informative, like this: "Negative path index {0} is invalid.". Then it was simplified, but developers forgot to remove the parameter index for the method Format. Of course, this is not the same as a forgotten parameter for the indicated output string specifier, i.e. the construction of the type String.Format("Negative path index {0} is invalid."). In such a case, an exception would be thrown. But in our case we also need neatness when refactoring. The code has to be fixed as follows:

    public void SetPath(....)
    {
      if (index < 0)
        throw new ArgumentOutOfRangeException(
          "Negative path index is invalid.");
      ....
    }

    Substring of the substring

    PVS-Studio warning: V3053 An excessive expression. Examine the substrings 'UnityEngine.' and 'UnityEngine.SetupCoroutine'. StackTrace.cs 43

    static bool IsSystemStacktraceType(object name)
    {
      string casted = (string)name;
      return casted.StartsWith("UnityEditor.") ||
        casted.StartsWith("UnityEngine.") ||
        casted.StartsWith("System.") ||
        casted.StartsWith("UnityScript.Lang.") ||
        casted.StartsWith("Boo.Lang.") ||
        casted.StartsWith("UnityEngine.SetupCoroutine");
    }

    Search of the substring "UnityEngine.SetupCoroutine" in the condition is meaningless, because before that the search for "UnityEngine." is performed. Therefore, the last check should be removed or one has to clarify the correctness of substrings.

    Another similar error:

    PVS-Studio warning: V3053 An excessive expression. Examine the substrings 'Windows.dll' and 'Windows.'. AssemblyHelper.cs 84

    static private bool CouldBelongToDotNetOrWindowsRuntime(string
      assemblyPath)
    {
      return assemblyPath.IndexOf("mscorlib.dll") != -1 ||
        assemblyPath.IndexOf("System.") != -1 ||
        assemblyPath.IndexOf("Windows.dll") != -1 ||  // <=
        assemblyPath.IndexOf("Microsoft.") != -1 ||
        assemblyPath.IndexOf("Windows.") != -1 ||  // <=
        assemblyPath.IndexOf("WinRTLegacy.dll") != -1 ||
        assemblyPath.IndexOf("platform.dll") != -1;
    }

    Size does matter

    PVS-Studio warning: V3063 CWE-571 A part of conditional expression is always true if it is evaluated: pageSize <= 1000. UNETInterface.cs 584

    public override bool IsValid()
    {
      ....
      return base.IsValid()
        && (pageSize >= 1 || pageSize <= 1000)
        && totalFilters <= 10;
    }

    Condition for a check of a valid page size is erroneous. Instead of the operator ||, one has to use &&. The corrected code:

    public override bool IsValid()
    {
      ....
      return base.IsValid()
        && (pageSize >= 1 && pageSize <= 1000)
        && totalFilters <= 10;
    }

    Possible division by zero

    PVS-Studio warning: V3064 CWE-369 Potential division by zero. Consider inspecting denominator '(float)(width - 1)'. ClothInspector.cs 249

    Texture2D GenerateColorTexture(int width)
    {
      ....
      for (int i = 0; i < width; i++)
        colors[i] = GetGradientColor(i / (float)(width - 1));
      ....
    }

    The problem may occur when passing the value width = 1 into the method. In the method, it is not checked anyway. The method GenerateColorTexture is called in the code just once with the parameter 100:

    void OnEnable()
    {
      if (s_ColorTexture == null)
        s_ColorTexture = GenerateColorTexture(100);
      ....
    }

    So, there is no error here so far. But, just in case, in the method GenerateColorTexture the possibility of transferring incorrect width value should be provided.

    Paradoxical check

    PVS-Studio warning: V3080 CWE-476 Possible null dereference. Consider inspecting 'm_Parent'. EditorWindow.cs 449

    public void ShowPopup()
    {
      if (m_Parent == null)
      {
        ....
        Rect r = m_Parent.borderSize.Add(....);
        ....
      }
    }

    Probably, due to a typo, the execution of such code guarantees the use of the null reference m_Parent. The corrected code:

    public void ShowPopup()
    {
      if (m_Parent != null)
      {
        ....
        Rect r = m_Parent.borderSize.Add(....);
        ....
      }
    }

    The same error occurs later in the code:

    PVS-Studio warning: V3080 CWE-476 Possible null dereference. Consider inspecting 'm_Parent'. EditorWindow.cs 470

    internal void ShowWithMode(ShowMode mode)
    {
      if (m_Parent == null)
      {
        ....
        Rect r = m_Parent.borderSize.Add(....);
        ....
    }

    And here's another interesting bug that can lead to access by a null reference due to incorrect check:

    PVS-Studio warning: V3080 CWE-476 Possible null dereference. Consider inspecting 'objects'. TypeSelectionList.cs 48

    public TypeSelection(string typeName, Object[] objects)
    {
      System.Diagnostics.Debug.Assert(objects != null ||
                                      objects.Length >= 1);
      ....
    }

    It seems to me that Unity developers quite often make errors related to misuse of operators || and && in conditions. In this case, if objects has a null value, then this will lead to a check of second part of the condition (objects != null || objects.Length >= 1), which will entail the unexpected throw of an exception. The error should be corrected as follows:

    public TypeSelection(string typeName, Object[] objects)
    {
      System.Diagnostics.Debug.Assert(objects != null &&
                                      objects.Length >= 1);
      ....
    }

    Early nullifying

    PVS-Studio warning: V3080 CWE-476 Possible null dereference. Consider inspecting 'm_RowRects'. TreeViewControlGUI.cs 272

    public override void GetFirstAndLastRowVisible(....)
    {
      ....
      if (rowCount != m_RowRects.Count)
      {
        m_RowRects = null;
        throw new InvalidOperationException(string.Format("....",
                  rowCount, m_RowRects.Count));
      }
      ....
    }

    In this case, the exception throw (access by the null reference m_RowRects) will happen when generating the message string for another exception. Code might be fixed, for example, as follows:

    public override void GetFirstAndLastRowVisible(....)
    {
      ....
      if (rowCount != m_RowRects.Count)
      {
        var m_RowRectsCount = m_RowRects.Count;
        m_RowRects = null;
        throw new InvalidOperationException(string.Format("....",
                  rowCount, m_RowRectsCount));
      }
      ....
    }

    One more error when checking

    PVS-Studio warning: V3080 CWE-476 Possible null dereference. Consider inspecting 'additionalOptions'. MonoCrossCompile.cs 279

    static void CrossCompileAOT(....)
    {
      ....
      if (additionalOptions != null & additionalOptions.Trim().Length > 0)
        arguments += additionalOptions.Trim() + ",";  
      ....
    }

    Due to the fact that the & operator is used in a condition, the second part of the condition will always be checked, regardless of the result of the check of the first part. In case if the variable additionalOptions has the null value, the exception throw is inevitable. The error has to be corrected, by using the operator && instead of &.

    As we can see, among the warnings with the number V3080 there are rather insidious errors.

    Late check

    PVS-Studio warning: V3095 CWE-476 The 'element' object was used before it was verified against null. Check lines: 101, 107. StyleContext.cs 101

    public override void OnBeginElementTest(VisualElement element, ....)
    {
      if (element.IsDirty(ChangeType.Styles))
      {
        ....
      }
    
      if (element != null && element.styleSheets != null)
      {
        ....
      }
      ....
    }

    The variable element is used without preliminary check for null. While later in the code this check is performed. The code probably needs to be corrected as follows:

    public override void OnBeginElementTest(VisualElement element, ....)
    {
      if (element != null)
      {
        if (element.IsDirty(ChangeType.Styles))
        {
          ....
        }
    
        if (element.styleSheets != null)
        {
          ....
        }
      }
      ....
    }

    In code there are 18 more errors. Let me give you a list of the first 10:

    • V3095 CWE-476 The 'property' object was used before it was verified against null. Check lines: 5137, 5154. EditorGUI.cs 5137
    • V3095 CWE-476 The 'exposedPropertyTable' object was used before it was verified against null. Check lines: 152, 154. ExposedReferenceDrawer.cs 152
    • V3095 CWE-476 The 'rectObjs' object was used before it was verified against null. Check lines: 97, 99. RectSelection.cs 97
    • V3095 CWE-476 The 'm_EditorCache' object was used before it was verified against null. Check lines: 134, 140. EditorCache.cs 134
    • V3095 CWE-476 The 'setup' object was used before it was verified against null. Check lines: 43, 47. TreeViewExpandAnimator.cs 43
    • V3095 CWE-476 The 'response.job' object was used before it was verified against null. Check lines: 88, 99. AssetStoreClient.cs 88
    • V3095 CWE-476 The 'compilationTask' object was used before it was verified against null. Check lines: 1010, 1011. EditorCompilation.cs 1010
    • V3095 CWE-476 The 'm_GenericPresetLibraryInspector' object was used before it was verified against null. Check lines: 35, 36. CurvePresetLibraryInspector.cs 35
    • V3095 CWE-476 The 'Event.current' object was used before it was verified against null. Check lines: 574, 620. AvatarMaskInspector.cs 574
    • V3095 CWE-476 The 'm_GenericPresetLibraryInspector' object was used before it was verified against null. Check lines: 31, 32. ColorPresetLibraryInspector.cs 31

    Wrong Equals method

    PVS-Studio warning: V3115 CWE-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. CurveEditorSelection.cs 74

    public override bool Equals(object _other)
    {
      CurveSelection other = (CurveSelection)_other;
      return other.curveID == curveID && other.key == key &&
        other.type == type;
    }

    Overload of the Equals method was implemented carelessly. One has to take into account the possibility of obtaining null as a parameter, as this can lead to a throw of an exception, which hasn't been considered in the calling code. In addition, the situation, when _other can't be cast to the type CurveSelection, will lead to a throw of an exception. The code has to be fixed. A good example of the implementation of Object.equals overload is given in the documentation.

    In the code, there are other similar errors:

    • V3115 CWE-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. SpritePackerWindow.cs 40
    • V3115 CWE-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. PlatformIconField.cs 28
    • V3115 CWE-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. ShapeEditor.cs 161
    • V3115 CWE-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. ActiveEditorTrackerBindings.gen.cs 33
    • V3115 CWE-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'. ProfilerFrameDataView.bindings.cs 60

    Once again about the check for null inequality

    PVS-Studio warning: V3125 CWE-476 The 'camera' object was used after it was verified against null. Check lines: 184, 180. ARBackgroundRenderer.cs 184

    protected void DisableARBackgroundRendering()
    {
      ....
      if (camera != null)
        camera.clearFlags = m_CameraClearFlags;
    
      // Command buffer
      camera.RemoveCommandBuffer(CameraEvent.BeforeForwardOpaque,
                                 m_CommandBuffer);
      camera.RemoveCommandBuffer(CameraEvent.BeforeGBuffer,
                                 m_CommandBuffer);
    }

    When the camera variable is used the first time, it is checked for null inequality. But further along the code the developers forget to do it. The correct variant could be like this:

    protected void DisableARBackgroundRendering()
    {
      ....
      if (camera != null)
      {
        camera.clearFlags = m_CameraClearFlags;
    
        // Command buffer
        camera.RemoveCommandBuffer(CameraEvent.BeforeForwardOpaque,
                                   m_CommandBuffer);
        camera.RemoveCommandBuffer(CameraEvent.BeforeGBuffer,
                                   m_CommandBuffer);
      }
    }

    Another similar error:

    PVS-Studio warning: V3125 CWE-476 The 'item' object was used after it was verified against null. Check lines: 88, 85. TreeViewForAudioMixerGroups.cs 88

    protected override Texture GetIconForItem(TreeViewItem item)
    {
      if (item != null && item.icon != null)
        return item.icon;
    
      if (item.id == kNoneItemID) // <=
        return k_AudioListenerIcon;
      
      return k_AudioGroupIcon;
    }

    An error, that in some cases leads to an access by a null link. The execution of the condition in the first block if enables the exit from the method. However, if this does not happen, then there is no guarantee that the reference item is non-zero. Here is the corrected version of the code:

    protected override Texture GetIconForItem(TreeViewItem item)
    {
      if (item != null)
      {
        if (item.icon != null)
          return item.icon;
        
        if (item.id == kNoneItemID)
          return k_AudioListenerIcon;
      }
    
      return k_AudioGroupIcon;
    }

    In the code there are 12 similar errors. Let me give you a list of the first 10:

    • V3125 CWE-476 The 'element' object was used after it was verified against null. Check lines: 132, 107. StyleContext.cs 132
    • V3125 CWE-476 The 'mi.DeclaringType' object was used after it was verified against null. Check lines: 68, 49. AttributeHelper.cs 68
    • V3125 CWE-476 The 'label' object was used after it was verified against null. Check lines: 5016, 4999. EditorGUI.cs 5016
    • V3125 CWE-476 The 'Event.current' object was used after it was verified against null. Check lines: 277, 268. HostView.cs 277
    • V3125 CWE-476 The 'bpst' object was used after it was verified against null. Check lines: 96, 92. BuildPlayerSceneTreeView.cs 96
    • V3125 CWE-476 The 'state' object was used after it was verified against null. Check lines: 417, 404. EditorGUIExt.cs 417
    • V3125 CWE-476 The 'dock' object was used after it was verified against null. Check lines: 370, 365. WindowLayout.cs 370
    • V3125 CWE-476 The 'info' object was used after it was verified against null. Check lines: 234, 226. AssetStoreAssetInspector.cs 234
    • V3125 CWE-476 The 'platformProvider' object was used after it was verified against null. Check lines: 262, 222. CodeStrippingUtils.cs 262
    • V3125 CWE-476 The 'm_ControlPoints' object was used after it was verified against null. Check lines: 373, 361. EdgeControl.cs 373

    The choice turned out to be small

    PVS-Studio warning: V3136 CWE-691 Constant expression in switch statement. HolographicEmulationWindow.cs 261

    void ConnectionStateGUI()
    {
      ....
      HolographicStreamerConnectionState connectionState =
        PerceptionRemotingPlugin.GetConnectionState();
      switch (connectionState)
      {
        ....
      }
      ....
    }

    The method PerceptionRemotingPlugin.GetConnectionState() is to blame here. We have already come across it when we were analyzing the warnings V3022:

    internal static HolographicStreamerConnectionState
      GetConnectionState()
    {
      return HolographicStreamerConnectionState.Disconnected;
    }

    The method will return a constant. This code is very strange. It needs to be paid attention.

    Conclusions

    image4.png.deef28d7de649b2c81a70aa832590d46.png

    I think we can stop at this point, otherwise the article will become boring and overextended. Again, I listed the errors that I just couldn't miss. Sure, the Unity code contains a big number of the erroneous and incorrect constructions, that need to be fixed. The difficulty is that many of the issued warnings are very controversial and only the author of the code is able to make the exact "diagnosis" in each case.

    Generally speaking about the Unity project, we can say that it is rich for errors, but taking into account the size of its code base (400 thousand lines), it's not so bad. Nevertheless, I hope that the authors will not neglect the code analysis tools to improve the quality of their product.

    Use PVS-Studio and I wish you bugless code!

     



      Report Article


    User Feedback

    Create an account or sign in to leave a review

    You need to be a member in order to leave a review

    Create an account

    Sign up for a new account in our community. It's easy!

    Register a new account

    Sign in

    Already have an account? Sign in here.

    Sign In Now

    There are no reviews to display.


  • Advertisement
  • Advertisement
  • intellogo.png

    Are you ready to promote your game?

    Submit your game for Intel® certification by December 21, 2018 and you could win big! 

    Click here to learn more.

  • Latest Featured Articles

  • Featured Blogs

  • Advertisement
  • Popular Now

  • Similar Content

    • By tspartant
      Hey everyone! My name is Ryan. 
       
      Visualistic Studios is looking for experienced developers of all talents to join a game development team focused on completing contract work for compensation. 
       
      Work Description
      Typically you will either be assisting the team or working on your own contract.
      We usually bid $16-$25/h, however contracts can go above and below that so all pay grades are welcome, just be realistic. 
       
      Short Term Contracts
      Long Term Contracts
       
      We have the highest priority for these skills right now
       
      Programming - Unity, Unreal Blueprints
      Environment Artist
      Character Artist
      Character Animation
      UI Artist
      3D Asset Optimization
       
      VR/Mobile experience is a plus. 
       
      The Process 
      All communication is done through discord. All tasks and design documents will be laid out in "HackNPlan" for organization. 
      Initially, you'll get in contact with me and answer a few questions so I can get a scope of your experience. Afterwards, our outreach team will start looking for jobs that fit your description. Nothing is guaranteed, but if we know you're interested we can start looking 
       
      Our Experience
      For the past 3 years I've been working in game development contracting, and the past year I've been working full time from home. Since then, I've received more and more contracts and I'm now at the point that I have too many for myself to handle. This sparked the idea of creating a game development team for contract work! I've also been running my own hobby company for 5 years, and have a lot of experience in team management. 
       
      Get in contact!
      Please fill out this form so we can get all of the information out of the way, then we'll get in contact with you!
      Thank you everyone for reading, hope to hear from you soon!
    • By addictCoderCS
      Hi there,
      I'm working on an web RPG. This is not and action RPG. 
      The problem is I work full-time as a software developer and while I may get some code for the game done at work, I'm still a little short on time. So I'm looking for a second programmer to help me out. Please no beginners. I prefer working with someone who has built a full game (client, game server, web services, db)
      Requirements:
      Proficient in C# Proficient in .Net Core 2.X Experience with ASP.Net Core MVC Experience with ASP.Net Core Web API Experience with Unity 2018 Proficient in SQL and SQLite Proficient in EntityFramework Experience in AWS (RDS and EC2) Experience with IIS I'll handle the cost of any third-party services, domain names, etc. I'm just looking for a little help to get this game built in a reasonable amount of time. It will also be nice to bounce some ideas off each other.
       
      If you are interested, please send me an email: addictcodercs@gmail.com
       
       
    • By JeZ-l-Lee
      Hi,
      We are working on a sequel to my all time favorite application:
      "Garry Kitchen's GameMaker" for the Commodore 64.
      (it's a very simple game creation IDE)
      Application builds and runs on Windows(R) and Linux.
      (the only dependency is SDL2 and there is a makefile included to build on any Linux)
      NOTE: This is a just started work-in-progress...don't expect too much
      We will be updating this thread posting as production progresses.
      Please post complaints and suggestions to this forum thread.
      This is our most ambitious project to date so don't expect a beta for at least 6-12 months...
      Thanks!
      JeZxLee
      www.FallenAngelSoftware.com
      If you are unfamiliar with this superb game creation IDE then please check out the wiki below:
      en.wikipedia.org Garry Kitchen's GameMaker
      Garry Kitchen's GameMaker is an IDE for the Commodore 64, Apple II, and IBM PCs, created by Garry Kitchen and released by Activision in 1985. The software is notable as one of the earliest all-in-one game design products aimed at the general consumer, preceded by Broderbund’s The Arcade Machine in 1982. Two add-on disks are available for the Commodore 64 version: Sports, and Science Fiction. These include sprites, music, and background elements for loading into GameMaker. To demonstrate the vers...
        You can download the current entire project below on GitHub:
      GitHub FallenAngelSoftware/SDL2-C64GKGM2
      100% FREE Cross-Platform Open-Source SDL2 Video Game Engine! - FallenAngelSoftware/SDL2-C64GKGM2
        Here is a screenshot:

    • By GameDev.net
      GameDaily.Biz spoke to Improbable about its new shortcuts to multiplayer game development for Unity and Unreal. 

      Improbable helps game developers build believable online worlds with its bespoke technology, SpatialOS. Now, that task is much easier and accessible for those building games on the technology with the recent release of the SpatialOS Game Development Kit (GDK) for Unity. With these kits, Improbable hopes that developers find it easier to create vast, dynamic and unique worlds.
      This GDK for Unity includes a 200-gamer, first-person project that allows developers to experiment and tinker with their ideas for what their vision of a multiplayer game will look like.
      GameDaily.Biz met with Improbable’s Head of Product Marketing, Paul Thomas, and Head of Comms, Daniel Nye Griffiths, to speak about the SpatialOS GDK for Unity, as well as the upcoming launch of the SpatialOS GDK for Unreal Engine.
      In its first week, the SpatialOS GDK for Unity achieved over 2,000 developer sign ups to use it. “What we're trying to do is basically make it really fast for people to build multiplayer games,” said Thomas. “It comes with all the multiplayer networking so that developers don’t have to do any multiplayer networking. It comes with feature modules to allow [easy] solutions to common multiplayer problems, like player movement and shooting. And it comes with a cool starter project where you have 200 players in a free-for-all scenario. You can obviously use the power of SpatialOS to scale that project up to more players, with NPCs, and things like that. It gives people a really good base to start building multiplayer games.”
      There are several games currently in development or early access that utilize SpatialOS. The first into Early Access was Spilt Milk Studios’ Lazarus, a space MMO where the player becomes a pilot in a universe that ends every week, complete with a map that’s twice the size of Austria. Additionally, Bossa Studios released its survival exploration game Worlds Adrift into Steam Early Access earlier this year.
      Also using SpatialOS is Scavengers from Midwinter Entertainment, a studio founded by former 343 Industries studio head and Halo 4 Creative Director, Josh Holmes; the game is heavily inspired by his Halo 5: Guardians’ multiplayer mode, Warzone. Right alongside that company, Berlin-based Klang Studios is working on Seed, a simulation MMO that, according to its developers, lets players “interact and collaborate to create a world driven by real emotion and aspiration.”
      According to Thomas, for those looking to use the SpatialOS GDK for Unity, there is no limit to  what their games can do with Improbable’s tech.
      “What we're doing is expanding the possible gameplay you can do. Traditionally, when you make a multiplayer game, you're constrained by one single server. So you can say you have a 64-player game with a handful of NPCs or you could have a world that's 3km by 3km. With Spatial, you can go beyond that, test a much broader canvas to start thinking about different gameplay.”
      “You can go for a massive online persistent MMO with 10,000 players and hundreds of thousands of NPCs, something very, very vast and big like that. But you can also have smaller experiences. For example, there's a lot of interesting space in just extending what you see in the Battle Royale genre and session-based gameplay.”
      Thomas continued: “Our partners at Automaton have a game in development called Mavericks. The interesting thing there is they have a Battle Royale with 1,000 people, but what I really find interesting is the gameplay mechanics they've put in, like footprints so you can track people. They've added a cool fire propagation mechanic so you can start a fire that  spreads across the map and changes the world. Or you can add destructible buildings and things like that.”
      “So I think even looking at smaller scale games, we add a lot of value in terms of the new gameplay you can start adding. I'm just interested to see what people do with this extra power - what they can come up with.”
      While Battle Royale games and MMOs are obvious standouts for genres that best fit with SpatialOS, Thomas introduced some other ideas of genres that could benefit from the technology.
      “I also think there's a space for very interesting MMORTSs as well,” he said. “An RTS where you have persistent systems, like telling AIs to do things and then coming back to them a week later and seeing what's happened is an interesting space.”
      “I also see interesting mobile experiences that could come up. Having these worlds where you lay down some interesting things and then come back a few weeks later to see how they've evolved and changed, and the massive player interaction. Say for example with Pokemon Go, we can actually roam around the world and battle on the streets. I can see something like that working very well. Again, these are just ideas we've had and talked to people about. It's about giving people that flexibility and the ability to explore these ideas.”
      Klang’s Seed
      Griffiths added the possibility of events in a game that will have a massive, rippling, and lasting impact on its world as something that has people excited. One example he gives is how someone on one side of the map can do something that’ll have a knock-on effect for the rest of the world in real time.
      “There's a whole bunch of different angles you can take, some of which are about much larger player numbers or a much larger map, but there are other things you can do which are taking a relatively constrained game experience, a smaller map, a smaller number of players and adding richness to the game as well.”
      In fact, this is something that Thomas refers to as a “persistent in memory database,” meaning that for every object in the game world, there’s a history. Two examples cited by Thomas: “...a player could chop down a tree and that tree stays disappeared forever. Or a player can kill a big monster that was raiding a town and that town no longer gets raided by that monster, and this changes the dynamics of the world. Worlds can have a history. That means players can have a lot more meaning in these MMO worlds.”
      “Normally in MMOs, they're kinda like roller coaster rides: you go into a dungeon, you kill the boss and that guy respawns. It all resets,” Thomas continues. “But in Spatial MMOs, you could have these persistent effects that should change the gameplay meaningfully for all the rest of the player base.”
      “The other one I think that is interesting is the level of dynamism that you could have. So because you can have so much more server-side compute, you could potentially have NPCs roaming around the world changing their mind and deciding all of a sudden, 'oh, we're going to attack this player's base' or 'we're gonna go attack this town' and they have a lot more range and emotion and intelligence to them that you'd not see in other MMOs.
      “Normally in MMOs, NPCs sit there tethered. You go near them and they come and attack you, you run away, and they go back to where they were. In a Spatial MMO, that NPC can trace you across the whole map or a group of them can decide to get together and attack someone..”

      Bossa Studios' Worlds Adrift
       

      Next week, Improbable plans to launch its SpatialOS GDK for Unreal Engine, which will have a big focus on ease of use for access to Unreal, as well as a big emphasis on porting your projects to SpatialOS.
      “One of the things we'll be trying to push is a porting guide so you'll be able to take your existing Unreal game, move it onto SpatialOS and then you can grow to expand it with new and extra gameplay,” says Thomas. “ You can bring across your existing Unreal game and it feels very, very native and similar to Unreal if you're familiar with Unreal.”
      Griffiths continued, explaining how testing these experiences includes free cloud deployments, to a certain point. “If you're developing in SpatialOS in other ways, we provide a sandbox environment so you can get your game running. When you’re happy, you can port it over and sort of experiment with it in a free sandbox environment with a small number of cores to get started.”
      Based on what we learned, Improbable’s SpatialOS GDK for Unity will give developers enhanced flexibility to produce more in depth and engaging videos games. That said, we look forward to catching up with the company in the near future to see how this exciting technology is being used in the different games that we play.
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!