Search Unity

Feedback ParticleSystem Module API Improvement Proposal

Discussion in 'Scripting' started by peturdarri, Feb 16, 2022.

  1. peturdarri

    peturdarri

    Joined:
    Mar 19, 2016
    Posts:
    23
    Anyone who has used the ParticleSystem scripting API is aware of how unintuitive and unfriendly it is to set properties on the modules:
    Code (CSharp):
    1. particleSystem.main.playOnAwake = false; // error CS1612: Cannot modify the return value of 'ParticleSystem.main' because it is not a variable.
    The recommended approach is to cache it in a temporary local variable:
    Code (CSharp):
    1. var main = particleSystem.main;
    2. main.playOnAwake = false;
    The reason for this has been explained in this Unity blog post:
    https://blog.unity.com/technology/particle-system-modules-faq
    and it's a reasonable sacrifice to eliminate allocation and improve performance.

    But I propose a fix that will allow the first example to compile while still maintaining the zero-allocation struct modules.

    For context, here's a rough slice of how ParticleSystem is implemented internally:
    Code (CSharp):
    1. public class ParticleSystem
    2. {
    3.     public MainModule main => new MainModule(this);
    4.  
    5.     public struct MainModule
    6.     {
    7.         internal ParticleSystem m_ParticleSystem;
    8.  
    9.         internal MainModule(ParticleSystem particleSystem)
    10.         {
    11.             m_ParticleSystem = particleSystem;
    12.         }
    13.      
    14.         public extern float duration { get; set; }
    15.     }
    16. }
    [Online C# compile result]

    And here's my proposal:
    Code (CSharp):
    1. public class ParticleSystem
    2. {
    3.     private readonly MainModule _main;
    4.  
    5.     internal ParticleSystem()
    6.     {
    7.         _main = new MainModule(this);
    8.     }
    9.  
    10.     public ref readonly MainModule main => ref _main;
    11.  
    12.     public readonly struct MainModule
    13.     {
    14.         internal readonly ParticleSystem m_ParticleSystem;
    15.      
    16.         internal MainModule(ParticleSystem particleSystem)
    17.         {
    18.             m_ParticleSystem = particleSystem;
    19.         }
    20.  
    21.         public extern float duration { get; set; }
    22.     }
    23. }
    [Online C# compile result]

    Returning the module using ref means you're no longer modifying a copy and the compiler is happy. Adding readonly makes it impossible to assign the module property to match the original API.

    And even better, this change is not a breaking one. The original recommended approach of caching the module is still valid and compiles with this API, thanks to ref being optional on the call site.

    Cons
    The only con I can think of is that there's an additional memory overhead of storing the modules in fields in the ParticleSystem class. I've calculated that overhead to be 184 bytes per ParticleSystem, with 23 modules each 8 bytes in size. I think that's very reasonable for such an improvement to the API.

    Additionally, all the modules have to be preinitialized instead lazily on-demand, but on the other hand that moves the initialization cost to the start and makes it fixed, instead of like now where the module has to be created every time it is requested.

    Let me know if I've missed something obvious here. I hope to see this fix implemented.
     
    Last edited: Feb 23, 2022
    Romaleks360 likes this.
  2. peturdarri

    peturdarri

    Joined:
    Mar 19, 2016
    Posts:
    23
    ParticleSystem seems to complain if the modules are created in the constructor like in the example above. Probably too early. The easy fix is to reassign the field with a new instance in the property getter, similar to how the original property works, but there's probably a better solution to avoid the constructor call.
    Code (CSharp):
    1. public ref readonly MainModule main
    2. {
    3.     get
    4.     {
    5.         _main = new MainModule(this);
    6.         return ref _main;
    7.     }
    8. }
     
    Last edited: May 3, 2022
  3. Romaleks360

    Romaleks360

    Joined:
    Sep 9, 2017
    Posts:
    72
    particle system api really needs a change! it's awfully unintuitive and requires 2 lines of code for one simple set, and that's just terrible. I would rather take a performance penalty and write it in a familiar short way.
     
    peturdarri likes this.
  4. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,694
    What?! Every single time you do a GetComponent<T>() call this is the case!!!!

    Code (csharp):
    1. Rigidbody rb = GetComponent<Rigidbody>();
    2. rb.velocity = Vector3.forward * 10;
    Terrible??! I am ... I just ... I don't even know what to say here.

    Is your computer set up on some kind of pay by lines of code taken system?

    If so, put it on one line! It's not that hard... here, I'll show you how:

    Code (csharp):
    1. {var main = particleSystem.main; main.playOnAwake = false;}
    DONE!
     
  5. Romaleks360

    Romaleks360

    Joined:
    Sep 9, 2017
    Posts:
    72
    No need to be this emotional.
    That's not the case with GetComponent. You can totally write GetComponent<Rigidbody>(). velocity = Vector3.forward * 10 in one line, I am doing this all the time. It returns a class and not a struct, after all.
    I am not paying for each line of code, but that's an important metric for API ease of use. You would notice this for sure when extensively using Particle System from code, especially with lambdas or switch expression statements. Each module of PS has this annotation of a counter-intuitive use of structs. Simple things don't have to be explained on each page of Scripting Manual.

    I am currently making a certain Particle System wrapper for designers at our studio, and that's indeed a big issue of readability and usability. Not sure what you are trying to convince me in - there won't be this thread if API was good.
     
    Last edited: Sep 3, 2022
  6. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,694
    Again, absolutely no magic or mystery here. It is just plain old C#, nothing whatsoever to do with Unity.

    Perhaps you may wish to review the difference between reference types and value types.

    Components are classes (reference types). Getting one simply "points" to the thing.

    Structs are value types. Getting one makes a complete unrelated disconnected copy.

    Code (csharp):
    1.     public struct MyV2
    2.     {
    3.         public float x;
    4.         public float y;
    5.     }
    6.  
    7.     MyV2 v2 { get; set; }  // pretend this is your ParticleSystem.main getter
    8.  
    9.     void Start ()
    10.     {
    11.         // This is illegal for the fundamental C# reason of not
    12.         // allowing the user to write ambiguous code.
    13.         this.v2.x = 123;
    14.     }
    If you change
    MyV2
    into a class, the above code becomes legal.

    Of course, you and I don't get the privilege of rewriting the Unity API from struct to class.

    The only pseudo-magic comes from how manipulating the contents of that struct gets back to the engine through getters / setters. Nothing else.
     
    Last edited: Sep 3, 2022
  7. Romaleks360

    Romaleks360

    Joined:
    Sep 9, 2017
    Posts:
    72
    Who was talking about magic? I know how this works, and I know the difference between value types and reference types. Perhaps you don't? It was you who wrote that GetComponent and set requires 2 lines of code. And after I told you this can be done in one line, you decided to explain to me how this works. I know how this works, I know these basics and everything you wrote in your post - it's obvious. But you are totally missing the point of my post, changing the topic and making this too personal for no reason. Please don't reply back unless you want to discuss a problem with particle system API.
     
  8. peturdarri

    peturdarri

    Joined:
    Mar 19, 2016
    Posts:
    23
    This Unity employee has admitted that it is unintuitive and they would like improve it, but don't think they can without breaking backwards compatibility, which my proposal addresses.

    If you can't see how developers new to the Particle System scripting API would find it unintuitive, I don't know what I can tell you to convince you otherwise. It's enough of a problem that Unity felt the need to publish a blog post on the topic.
     
    Romaleks360 likes this.