Search Unity

Can we make the AnalyzeRules implementation classes public?

Discussion in 'Addressables' started by jonagill_rr, Jun 27, 2019.

  1. jonagill_rr

    jonagill_rr

    Joined:
    Jun 21, 2017
    Posts:
    34
    It's cool that users can write their own AnalyzeRules by extending the exposed base class, but there's a lot of good functionality in CheckDupeDependencidesBase and its various subclasses that I'd love to be able to harness in our own rules. For instance, I want to write our own version of CheckBundleDupeDependencies that has all the same analysis code, but has a different approach to how it fixes duplicate dependency issues. As it is, I'm going to have to access the existing implementation via reflection, whereas it would be a lot easier if I could just write my own rule as an extension of the ones Unity has provided.

    Could all the classes in the AnalyzeRules directory be made public in the next release so that we can properly extend them for our own rules?
     
    RecursiveFrog likes this.
  2. jonagill_rr

    jonagill_rr

    Joined:
    Jun 21, 2017
    Posts:
    34
    Additionally, I think I have discovered a bug with registering additional AnalyzeRules. According to the documentation, you should only need to call AnalyzeWindow.RegisterNewRule<MyRule>() in order to add the new rule. While this will make your rule appear in the UI window, any attempts to run it will silently fail. This is because the code to run the selected rules is checking the serialized AnalyzeResultData to see if it contains your rule, and this file is uneditable (any attempts to edit it will immediately re-write themselves with the previous values). The only way to make this file reflect your new rule is to register your rule, then delete the AnalyzeResultData file entirely and allow it to regenerate itself.
     
  3. unity_bill

    unity_bill

    Unity Technologies

    Joined:
    Apr 11, 2017
    Posts:
    982
    the main reason we don't do this is that as soon as we make it public, we can't change it. We are locked in to supporting that API for the long haul. If this was closed off editor code, I'd consider taking the hit of being locked as you'd have no other way to access the code. But since the package is open source, I'm hoping you can just copy our class over. If our class is accessing other internal things making this quite difficult, then we do have something to address there.

    Sounds like a bug. I've filed a ticket, we'll look into it.
     
  4. unity_bill

    unity_bill

    Unity Technologies

    Joined:
    Apr 11, 2017
    Posts:
    982
  5. jonagill_rr

    jonagill_rr

    Joined:
    Jun 21, 2017
    Posts:
    34
    Odd. I just added another new rule and ran into the same issue where I had to delete my AnalyzeResultData and re-load the assembly before the rule would properly run.

    The line where it fails silently is pretty clear in AssetSettingsAnalyzeTreeView.cs:74, where it will refuse to run a rule that does not already have its name in the result data block. Surely instead of silently refusing to run, it should just add the new rule name to the list of results?

    Have you tried making a new rule CS file in the test project and verifying that it then runs successfully when you try to run it from the Analyze window? I imagine changing the name of an existing rule and re-compiling would also trigger this issue.
     
  6. unity_bill

    unity_bill

    Unity Technologies

    Joined:
    Apr 11, 2017
    Posts:
    982
    this should be fixed in addressables 1.2.x

    thanks for reporting it.
     
unityunity