Review

Exiled.API/Features/Core/Components/TickComponent.cs

@@ -24,7 +24,7 @@ public sealed class TickComponent : EObject
         /// The default fixed tick rate (60 per second).
         /// </summary>
 #pragma warning disable SA1310
-        public const float DEFAULT_FIXED_TICK_RATE = 0.016f;
+        public const float DEFAULT_FIXED_TICK_RATE = 1f / 60f;
 #pragma warning restore SA1310
 
         private readonly HashSet<CoroutineHandle> boundHandles;

Exiled.API/Features/Core/ConstProperty.cs

@@ -1,4 +1,4 @@
-// -----------------------------------------------------------------------
+// -----------------------------------------------------------------------
 // <copyright file="ConstProperty.cs" company="Exiled Team">
 // Copyright (c) Exiled Team. All rights reserved.
 // Licensed under the CC BY-SA 3.0 license.
@@ -48,6 +48,7 @@ public ConstProperty(T constantValue, Type[] typesToPatch, MethodInfo[] skipMeth
         /// </summary>
         ~ConstProperty()
         {
+            // @Nao Never be call, it need to remove it reference from List beffor the dtor call
             List.Remove(this);
 
             foreach (MethodInfo methodInfo in PatchedMethods)

Exiled.API/Features/Core/GameEntity.cs

@@ -44,11 +44,6 @@ protected GameEntity(GameObject gameObject)
             // List.Add(this);
         }
 
-        /// <summary>
-        /// Finalizes an instance of the <see cref="GameEntity"/> class.
-        /// </summary>
-        ~GameEntity() => List.Remove(this);
-
         /// <summary>
         /// Gets all active <see cref="GameEntity"/> instances.
         /// <para/>

Exiled.API/Features/Core/Generic/EBehaviour.cs

@@ -60,6 +60,8 @@ protected EBehaviour()
         /// </remarks>
         protected virtual void FindOwner()
         {
+            // @Nao T is a GameEntity, why not add in inisde of it the method a abstract.
+            // Or create an specific Interaface requesting to implement this method
             MethodInfo method = typeof(T).GetMethod("Get", BindingFlags.Public | BindingFlags.Static, null, new[] { typeof(GameObject) }, null);
 
             if (method != null)

Exiled.API/Features/Core/Generic/Singleton.cs

@@ -19,6 +19,9 @@ namespace Exiled.API.Features.Core.Generic
     public sealed class Singleton<T> : TypeCastObject<T>
         where T : class
     {
+        // @Nao, this is not realy a singleton if you have mutliple value.
+        // NB: each Singleton<T> ave a new instance of dictionary.
+        // exemple Singleton<int>.Instance is not the same Singleton<bool>.Instance
         private static readonly Dictionary<T, Singleton<T>> Instances = new();
 
         /// <summary>
@@ -35,6 +38,8 @@ public Singleton(T value)
         /// <summary>
         /// Finalizes an instance of the <see cref="Singleton{T}"/> class.
         /// </summary>
+        // @Nao Probably never call, it self reference. Maybe an future issue doing memory leek
+        // it will get call only after the instance get remove form Instances
         ~Singleton() => Instances.Remove(Value);
 
         /// <summary>
@@ -59,6 +64,10 @@ public Singleton(T value)
         /// <typeparam name="TObject">The type of the object.</typeparam>
         /// <param name="instance">The object instance.</param>
         /// <returns><see langword="true"/> if the object instance is not null and can be casted as the specified type; otherwise, <see langword="false"/>.</returns>
+        // @nao It will return the Instance.
+        // But with this curent class definition you can register derived class of T.
+        // They can be registred but not retrived. Also why make this method generic if this not to get
+        // other value than the instance.
         public static bool TryGet<TObject>(out TObject instance)
             where TObject : class => (instance = Instance as TObject) is not null;
 
@@ -72,13 +81,7 @@ public static bool TryGet<TObject>(out TObject instance)
         /// <returns><see langword="true"/> if the instance was destroyed; otherwise, <see langword="false"/>.</returns>
         public static bool Destroy(T @object)
         {
-            if (Instances.TryGetValue(@object, out Singleton<T> _))
-            {
-                Instances[@object] = null;
-                return Instances.Remove(@object);
-            }
-
-            return false;
+            return Instances.Remove(@object);
         }
     }
 }

Exiled.API/Features/Core/Generic/UniqueUnmanagedEnumClass.cs

@@ -40,6 +40,10 @@ public UniqueUnmanagedEnumClass()
             values ??= new();
             TypeCode code = Convert.GetTypeCode(typeof(TSource).GetField("MinValue").GetValue(null));
 
+            // @Nao If the value is not an Uxxxxx it will get an overflow. Like if it use an Int16, Byte or SByte
+            // Maybe use a long for nextValue and do "nextValue = typeof(TSource).GetField("MinValue").GetValue(null)"
+            // it also be a struct containing only unmanged struct. The best solution is proably to
+            // look for upper version of the framwork to resolve this with the new numeric interfaces.
             if (code is TypeCode.UInt16 or TypeCode.UInt32 or TypeCode.UInt64)
                 nextValue = 0;
 
@@ -53,6 +57,9 @@ public UniqueUnmanagedEnumClass()
                 while (values.ContainsKey(value));
 
                 Value = value;
+
+                // @nao If the value is register when a new instance is created.
+                // Maybe put the ctor in protected to avoid exteranal code (outisde the enum) to create new values
                 values.Add(value, (TObject)this);
             }
         }
@@ -285,7 +292,7 @@ public int CompareTo(object obj) =>
         /// </returns>
         public int Compare(TObject x, TObject y) => x == null ? -1 : y == null ? 1 : x.Value.CompareTo(y.Value);
 
-/// <inheritdoc/>
+        /// <inheritdoc/>
         TypeCode IConvertible.GetTypeCode() => Value.GetTypeCode();
 
         /// <inheritdoc/>

Exiled.API/Features/Core/Generic/UnmanagedEnumClass.cs

@@ -22,6 +22,7 @@ namespace Exiled.API.Features.Core.Generic
     /// </summary>
     /// <typeparam name="TSource">The type of the <see langword="unmanaged"/> source object to handle the instance of.</typeparam>
     /// <typeparam name="TObject">The type of the child object to handle the instance of.</typeparam>
+    // @Nao you can use Comparer<NonComparable>.Default, it will result as using the IComparable<TObject>.Compare
     public abstract class UnmanagedEnumClass<TSource, TObject> : IComparable, IEquatable<TObject>, IComparable<TObject>, IComparer<TObject>, IConvertible, IEnumClass
         where TSource : unmanaged, IComparable, IFormattable, IConvertible, IComparable<TSource>, IEquatable<TSource>
         where TObject : UnmanagedEnumClass<TSource, TObject>
@@ -253,6 +254,7 @@ public override bool Equals(object obj) =>
         /// Zero This instance occurs in the same position in the sort order as other.
         /// Greater than zero This instance follows other in the sort order.
         /// </returns>
+        // @Nao if x and y is null it should return, 0.
         public int CompareTo(object obj) =>
             obj == null ? -1 : obj is TSource value ? Value.CompareTo(value) : obj is TObject derived ? Value.CompareTo(derived.Value) : -1;
 
@@ -269,6 +271,8 @@ public int CompareTo(object obj) =>
         /// Zero This instance occurs in the same position in the sort order as other.
         /// Greater than zero This instance follows other in the sort order.
         /// </returns>
+        // @Nao if x and y is equal to null it should return, 0.
+        // Maybe use the default comparator, it handle it by default and if needed it can be replaced by a custom one.
         public int Compare(TObject x, TObject y) => x == null ? -1 : y == null ? 1 : x.Value.CompareTo(y.Value);
 
         /// <inheritdoc/>

Exiled.API/Features/Player.cs

@@ -4066,6 +4066,7 @@ public void PlayCassieAnnouncement(string words, bool makeHold = false, bool mak
         /// <param name="makeHold">Same on <see cref="Cassie.MessageTranslated(string, string, bool, bool, bool)"/>'s isHeld.</param>
         /// <param name="makeNoise">Same on <see cref="Cassie.MessageTranslated(string, string, bool, bool, bool)"/>'s isNoisy.</param>
         /// <param name="isSubtitles">Same on <see cref="Cassie.MessageTranslated(string, string, bool, bool, bool)"/>'s isSubtitles.</param>
+        // @Nao, why not inherit the documentation for the param ?
         public void SendCassieAnnouncement(string words, string translation, bool makeHold = false, bool makeNoise = true, bool isSubtitles = true)
         {
             StringBuilder announcement = StringBuilderPool.Pool.Get();

Exiled.API/Features/Respawn.cs

@@ -23,6 +23,9 @@ namespace Exiled.API.Features
     /// </summary>
     public static class Respawn
     {
+        private const string MtfChopperGameObjectName = "Chopper";
+        private const string ChaosVanGameObjectName = "CIVanArrive";
+
         private static GameObject ntfHelicopterGameObject;
         private static GameObject chaosCarGameObject;
 
@@ -34,7 +37,7 @@ public static GameObject NtfHelicopter
             get
             {
                 if (ntfHelicopterGameObject == null)
-                    ntfHelicopterGameObject = GameObject.Find("Chopper");
+                    ntfHelicopterGameObject = GameObject.Find(MtfChopperGameObjectName);
 
                 return ntfHelicopterGameObject;
             }
@@ -48,7 +51,7 @@ public static GameObject ChaosVan
             get
             {
                 if (chaosCarGameObject == null)
-                    chaosCarGameObject = GameObject.Find("CIVanArrive");
+                    chaosCarGameObject = GameObject.Find(ChaosVanGameObjectName);
 
                 return chaosCarGameObject;
             }

Exiled.API/Features/VirtualAssemblies/VirtualPlugin.cs

@@ -175,7 +175,7 @@ public static IEnumerable<VirtualPlugin> EnableAll()
                     continue;
 
                 VirtualPlugin vp = Activator.CreateInstance(type) as VirtualPlugin;
-                vp.TryRegister(type.GetCustomAttribute(typeof(VirtualPluginAttribute)) as VirtualPluginAttribute);
+                vp.TryRegister(type.GetCustomAttribute<VirtualPluginAttribute>());
                 vps.Add(vp);
             }
 

Exiled.CustomModules/API/Features/CustomEscapes/EscapeSettings.cs

@@ -74,7 +74,7 @@ public EscapeSettings(
             Role = role;
             CustomRole = CustomRole.Get(customRole);
             Position = position == default ? DefaultPosition : position;
-            DistanceThreshold = distanceThreshold == DefaultMaxDistanceTolerance ? DefaultMaxDistanceTolerance : distanceThreshold;
+            DistanceThreshold = distanceThreshold;
         }
 
         /// <summary>

Exiled.CustomModules/API/Features/CustomGamemodes/GameModeSettings.cs

@@ -186,7 +186,7 @@ public class GameModeSettings : TypeCastObject<GameModeSettings>, IAdditivePrope
         public virtual uint[] NonSpawnableCustomRoles { get; set; }
 
         /// <summary>
-        /// Gets or sets a <see cref="UUCustomTeamType"/>[] containing all non spawnable custom teams.
+        /// Gets or sets a <see cref="SpawnableTeamType"/>[] containing all non spawnable custom teams.
         /// <para/>
         /// If not empty, all undefined teams will be able to spawn.
         /// <br/>

Exiled.CustomModules/API/Features/CustomRoles/CustomRole.cs

@@ -239,8 +239,7 @@ public virtual bool EvaluateConditions
                     {
                         if (pawn.Role == RequiredRoleToSpawn)
                         {
-                            if ((RoleExtensions.GetTeam(RequiredRoleToSpawn) is Team.SCPs && !pawn.IsScp) ||
-                                (RoleExtensions.GetTeam(RequiredRoleToSpawn) is not Team.SCPs && pawn.IsScp))
+                            if ((RoleExtensions.GetTeam(RequiredRoleToSpawn) is Team.SCPs) != pawn.IsScp)
                                 continue;
 
                             return true;

Exiled.CustomModules/API/Features/CustomRoles/CustomTeam.cs

@@ -35,6 +35,9 @@ namespace Exiled.CustomModules.API.Features.CustomRoles
     /// </remarks>
     public abstract class CustomTeam : CustomModule
     {
+        private const string VanilaConfigMtfMinTime = "minimum_MTF_time_to_spawn";
+        private const string VanilaConfigMtfMaxTime = "maximum_MTF_time_to_spawn";
+
         private static readonly Dictionary<Player, CustomTeam> PlayersValue = new();
         private static readonly List<CustomTeam> Registered = new();
         private static readonly Dictionary<Type, CustomTeam> TypeLookupTable = new();
@@ -111,17 +114,18 @@ public abstract class CustomTeam : CustomModule
         /// This property provides access to a curated collection of <see cref="uint"/> objects, encapsulating all available custom role  within the context of units.
         /// <br/>The collection is designed to be both queried and modified as needed to accommodate dynamic scenarios within the game architecture.
         /// </remarks>
+        // @Nao, adding the possiblity to have custom roles
         public virtual IEnumerable<uint> Units { get; set; } = new uint[] { };
 
         /// <summary>
         /// Gets or sets the minimum amount time after which any team will be allowed to spawn.
         /// </summary>
-        public virtual float MinNextSequenceTime { get; set; } = GameCore.ConfigFile.ServerConfig.GetFloat("minimum_MTF_time_to_spawn", 280f);
+        public virtual float MinNextSequenceTime { get; set; } = GameCore.ConfigFile.ServerConfig.GetFloat(VanilaConfigMtfMinTime, 280f);
 
         /// <summary>
         /// Gets or sets the maximum amount time after which any team will be spawned.
         /// </summary>
-        public virtual float MaxNextSequenceTime { get; set; } = GameCore.ConfigFile.ServerConfig.GetFloat("maximum_MTF_time_to_spawn", 350f);
+        public virtual float MaxNextSequenceTime { get; set; } = GameCore.ConfigFile.ServerConfig.GetFloat(VanilaConfigMtfMaxTime, 350f);
 
         /// <summary>
         /// Gets or sets the relative spawn probability of the <see cref="CustomTeam"/>.
@@ -226,8 +230,7 @@ public virtual bool EvaluateConditions
                     {
                         if (pawn.Role == RequiredRoleToSpawn)
                         {
-                            if ((RoleExtensions.GetTeam(RequiredRoleToSpawn) is Team.SCPs && !pawn.IsScp) ||
-                                (RoleExtensions.GetTeam(RequiredRoleToSpawn) is not Team.SCPs && pawn.IsScp))
+                            if ((RoleExtensions.GetTeam(RequiredRoleToSpawn) is Team.SCPs) != pawn.IsScp)
                                 continue;
 
                             return true;

Exiled.CustomModules/API/Features/CustomRoles/RoleBehaviour.cs

@@ -178,37 +178,49 @@ protected virtual RoleTypeId FakeAppearance
         /// <summary>
         /// Evaluates the specified conditions affecting the round's ending conditions.
         /// </summary>
-        /// <returns>The corresponding evaluation.</returns>
+        /// <returns>
+        /// <see langword="false"/> if the round should continue.
+        /// <br/>Otherwise, <see langword="true"/> if the round can end.
+        /// All other condition need to be <see langword="true"/> to end the round.
+        /// </returns>
         public virtual bool EvaluateEndingConditions()
         {
-            if (CustomRole.TeamsOwnership.Length == 1)
-                return true;
-
-            SummaryInfo summaryInfo = World.Get().SummaryInfo;
-
-            if (CustomRole.TeamsOwnership.Contains(Team.SCPs) && summaryInfo.FoundationForces <= 0 && summaryInfo.ChaosInsurgency <= 0)
-                return true;
-
-            if (CustomRole.TeamsOwnership.Any(team => team is Team.ClassD or Team.ChaosInsurgency) && summaryInfo.FoundationForces <= 0 && summaryInfo.Anomalies <= 0)
-                return true;
-
-            if (CustomRole.TeamsOwnership.Any(team => team is Team.FoundationForces or Team.Scientists) && summaryInfo.ChaosInsurgency <= 0 && summaryInfo.Anomalies <= 0)
-                return true;
-
-            if (CustomRole.TeamsOwnership.IsEmpty())
+            SummaryInfo summaryInfo;
+            switch (CustomRole.TeamsOwnership.Length)
             {
-                int uniqueFaction = 0;
-                if (summaryInfo.FoundationForces > 0)
-                    ++uniqueFaction;
-                if (summaryInfo.ChaosInsurgency > 0)
-                    ++uniqueFaction;
-                if (summaryInfo.Anomalies > 0)
-                    ++uniqueFaction;
-
-                return uniqueFaction <= 1;
-            }
+                case 0:
+                    summaryInfo = World.Get().SummaryInfo;
+                    int uniqueFaction = 0;
+                    if (summaryInfo.FoundationForces > 0)
+                        ++uniqueFaction;
+                    if (summaryInfo.ChaosInsurgency > 0)
+                        ++uniqueFaction;
+                    if (summaryInfo.Anomalies > 0)
+                        ++uniqueFaction;
+                    return uniqueFaction <= 1;
+
+                case 1: return true;
+
+                default:
+                    // @Nao, i mouve repeted code here to see better
+                    // For me this following condition are always true.
+                    // Expete if it existe a case where there is -1 Chaos
+                    summaryInfo = World.Get().SummaryInfo;
+
+                    bool hasChaos = summaryInfo.ChaosInsurgency <= 0;
+                    bool hasFondtion = summaryInfo.FoundationForces <= 0;
+                    bool hasScp = summaryInfo.ChaosInsurgency <= 0;
+
+                    if (CustomRole.TeamsOwnership.Contains(Team.SCPs) && hasFondtion && hasChaos)
+                        return true; // @Nao Why true, that mean the round can end, but if there is some fondation left, the round should not end...
+
+                    if (CustomRole.TeamsOwnership.Any(team => team is Team.ClassD or Team.ChaosInsurgency) && hasFondtion && hasScp)
+                        return true;
 
-            return false;
+                    if (CustomRole.TeamsOwnership.Any(team => team is Team.FoundationForces or Team.Scientists) && hasChaos && hasScp)
+                        return true;
+                    return false;
+            }
         }
 
         /// <inheritdoc/>

Exiled.CustomModules/API/Features/RespawnManager.cs

@@ -174,21 +174,24 @@ private void OnPreRespawningTeam(PreRespawningTeamEventArgs ev)
         {
             PreviousKnownTeam = NextKnownTeam;
 
+            // @Nao are you sur that is "or SpawnableTeamType" and not "or not SpawnableTeamType"
             if (NextKnownTeam is null or SpawnableTeamType)
                 return;
 
             CustomTeam customTeam = CustomTeam.Get((uint)NextKnownTeam);
             if (customTeam is null)
                 return;
 
-            if (customTeam.TeamsOwnership.Any(t =>
-                    t == (ev.NextKnownTeam is SpawnableTeamType.ChaosInsurgency ?
-                        Team.ChaosInsurgency : Team.FoundationForces)))
+            if (customTeam.TeamsOwnership.Any(t => t == (Team)ev.NextKnownTeam))
             {
                 ev.MaxWaveSize = customTeam.Size;
                 return;
             }
 
+            // @Nao, it cool to use the event system for this. But if an other plugin
+            // allready say "ev.IsAllowed = false". it will get ignore
+            // and the team will still spawn...
+            // And post event shoold be here to spawn. Or an EventHandler handling last
             ev.IsAllowed = false;
             Spawn();
             Respawning.RespawnManager.Singleton?.RestartSequence();