Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use anything from System.Reflection namespace in Update() loop #83

Closed
originalfoo opened this issue Jun 9, 2020 · 1 comment · Fixed by #105
Closed

Don't use anything from System.Reflection namespace in Update() loop #83

originalfoo opened this issue Jun 9, 2020 · 1 comment · Fixed by #105
Assignees
Labels
approved rule Indicates if the new proposed rule has been approved to move to implementation phase

Comments

@originalfoo
Copy link

originalfoo commented Jun 9, 2020

Problem statement

This is disturbingly common in commuinity submitted mods, especially small "that couldn't possibly be the cause of the lag" mods:

public class DecimateFps : MonoBehaviour
{
   protected void Update()
   {
        // Code using `System.Reflection` to get/set private properies
   }
}

Is also disastrous in LateUpdate(), OnGUI(), etc.

Often the System.Reflection code will be contained in a separate method - I'm not sure if the analyzer can track that down? Such methods often have a signature similar to Q ReadPrivate<T, Q>(T o, string fieldName) or WritePrivate<T, Q>(T o, string fieldName, object value).

Proposed solution

No code fix suggestion for this, it just needs flagging to user as a warning.

Performance: Never use System.Reflection features in performance critical messages.

Docs could mention:

If you must use System.Reflection to access a private member, consider caching FieldInfo or MethodInfo in Start(), or elsewhere, and use the cached references in the loop instead.

In many cases alternatives are available which don't require reflection. For example, if a game object has a private field referencing a UI component, a direct reference to the component can be obtained using code such as FindObjectOfType<SomeGO>().GetComponent<T>(); the component can be cached and subsequently directly accessed in the update loop.

@sailro
Copy link
Member

sailro commented Jun 15, 2020

Yes we are thinking as well on doing this kind of analysis.

@sailro sailro self-assigned this Jul 24, 2020
@sailro sailro added the approved rule Indicates if the new proposed rule has been approved to move to implementation phase label Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved rule Indicates if the new proposed rule has been approved to move to implementation phase
Projects
None yet
2 participants