in0finite / SanAndreasUnity

Open source reimplementation of GTA San Andreas game engine in Unity
https://discord.gg/p6jjud5
MIT License
2.1k stars 348 forks source link

Suspicious code fragments found by PVS-Studio #146

Open AGB-DevRel opened 7 months ago

AGB-DevRel commented 7 months ago

Hello I found some suspicious fragments in the project source code with the help of the PVS-Studio static analyzer. A few suspicious code snippets are bellow. Perhaps some of them are worth fixing.

Issue 1

private void OnDrawGizmosSelected()
{
  ....
  var vehicles = FindObjectsOfType<Vehicle>()
                .Where(....)
                .OrderBy(....)
                .ToArray();
  foreach (var vehicle in vehicles)
  {
    foreach (var seat in vehicle.Seats){....}
    var closestSeat = vehicle.GetSeatAlignmentOfClosestSeat(transform.position);
    if (closestSeat != Vehicle.SeatAlignment.None){....}
    break;
  }
}

Link to the sources A break occurs under any circumstances on the first iteration of the loop. As a result, only the first element from the entire vehicles collection will be used in the loop.

Issue 2

public void SetString(string key, string value)
{
  if (m_syncDictionary.TryGetValue(key, out string existingValue))
  {
    if (value != existingValue)
    {
      m_syncDictionary[key] = value; // <=
    }
  }
  m_syncDictionary[key] = value; // <=
}

Link to the sources The m_syncDictionary[key] = value assignment occurs regardless of the check results.

in0finite commented 7 months ago

Thanks for reporting this.

The first one can obviously be written in a better way (without foreach loop).

The second one is indeed a mistake.