Search Unity

Please stop using Assembly.GetTypes() in your Packages!

Discussion in 'Assets and Asset Store' started by YannickTriqueneaux, Nov 2, 2022.

  1. YannickTriqueneaux

    YannickTriqueneaux

    Joined:
    Apr 9, 2020
    Posts:
    4
    Hello Packages Programmers!

    I'd like to ask you to stop searching types in assemblies in your packages!
    This is a very bad practice in C# that cause many issues.
    When doing this you're actually loading every types of the game assemblies, which cause memory allocations that won't be release. You're literally forcing games to be heavier in memory.

    It also increas the loading times of games, because depending of the number of assemblies the game needs, this might takes few seconds to load every types like this, freezing the game during secondes!

    I won't only complain about this bad practice, I'll also give you a solution to prevent it:
    So here is a design pattern I propose, that from my point of view should become more popular in the C# world:

    So generally, this bad partice is used to find specific types, like types that has a specific attribute on it, or inherits of a type, or implements an interface.

    Most of the time for your packages to find Client's implementation of what you expect them to implement.

    Exemple:
    You have a package that searchs every types of components of type "WeaponComponent".
    So you might have a Manager somewhere that needs to know all Weapons implementation that the client might use in its game.

    Typically your "WeaponManager" will contains the code:

    Code (CSharp):
    1.  CurrentDomain.GetAssemblies().SelectMany(a => a.GetTypes()).Where(type => typeof(WeaponComponent).IsAssignableFrom(type)).ToArray()
    Here is my solution to prevent doing this bad practice:

    Code (CSharp):
    1. public class WeaponManager
    2. {
    3.      [AttributeUsage(AttributeTargets.Assembly, AllowMultiple = true)]
    4.      public class RegisterWeaponAttribute : Attribute
    5.      {
    6.           public InjectionLoadable(Type typeOfWeapon)
    7.           {
    8.                 TypeOfWeapon= typeOfWeapon;
    9.           }
    10.  
    11.           public Type TypeOfWeapon { get; }
    12.      }
    13.  
    14.  
    15.  
    16.      private Type[] FindAllWeaponTypes()
    17.      {
    18.            return CurrentDomain.GetAssemblies()
    19.                      .SelectMany(a => a.GetCustomAttributes(false)
    20.                          .OfType<WeaponManager.RegisterWeaponAttribute>()
    21.                      )
    22.                      .Select(att => att.TypeOfWeapon)
    23.                      .ToArray();
    24.      }
    25. }
    And ask your client to put this attribute above their WeaponComponent declaration files:

    Code (CSharp):
    1. [assembly: WeaponManager.RegisterWeapon(typeof(BazookaWeaponComponent))]
    2.  
    3. public class BazookaWeaponComponent : WeaponComponent
    4. {
    5.      //the weapon implementation
    6. }

    So the idea is to register the type implementations inside of the Assemblies attributes, which is more smaller than the number of types in assemblies, which makes the searches much faster.
    Also it's only loading the assembly attributes and not all types of the universe.

    The attribute is not that annoying to write, which still make the Client syntaxe almost as easy as before, since you can put it directly inside the file that declars type to register.
    Still this is less annoying than having to Register things manually in the manager that have a Register method.
    Which usually breaks some design pattern like Dependency Injections, or Plugin Pattern, that may also cause circular dependencies.
    You might think it's a hack, assembly attributes are not made for this. But from my point of view they are especially made for this: It's like you code is saying "Hey if you're looking for these types in my assembly, here is the list of what my assembly is implementing".

    This example is for component, but this is actually working for any systems that needs types discovery.

    I hope one day to see this bad practice desapear. I´m bored having to modify every packages or libraries that does this mistake. Which makes the all Immutable Unity Package system a failure.
     
    Last edited: Nov 2, 2022
    paperGecko and Neto_Kokku like this.