Skip to content

Conversation

@varinotmUnity
Copy link
Contributor

@varinotmUnity varinotmUnity commented Jan 13, 2026

DO NOT FORGET TO INCLUDE A CHANGELOG ENTRY

I didn't modify the changelog, as we already have many entries for the obsolete API...

Purpose of this PR

More fixes to obsolete API, this time, about the TreViewItem.

from the ticket :

As part of the GameObject/Entity Integration we have in 6.4 obsoleted implicit casting between EntityId and int, and all APIs that take in InstanceID.

As a result certain packages need to be updated to match this change, including yours: ‘com.unity.probuilder’.

Here are the places which cause errors in your package:

AssetTreeView.cs(15, 105, 138, 143, 199): 'TreeViewItem' is obsolete: 'TreeViewItem is now deprecated. You can likely now use TreeViewItem<int> instead and not think more about it. But if you were using that identifier to store InstanceID data, you should instead opt to upgrade your TreeViews to use TreeViewItem<InstanceID> to get the proper typing.'
and : TreeViewState' is obsolete: 'TreeViewState is now deprecated. You can likely now use TreeViewState<int> instead and not think more about it. But if you were using that identifier to store InstanceID data, you should instead opt to upgrade your TreeViews to use TreeViewState<InstanceID> to get the proper typing.'
AssetIdRemapEditor.cs(134, 481): 'TreeViewItem' is obsolete: 'TreeViewItem is now deprecated. You can likely now use TreeViewItem<int> instead and not think more about it. But if you were using that identifier to store InstanceID data, you should instead opt to upgrade your TreeViews to use TreeViewItem<InstanceID> to get the proper typing.'

How you will need to change this depends on the implementation, however as an example, in ‘AssetIdRemapEditor.cs’

#pragma warning disable CS0618 // Type or member is obsolete
[SerializeField]
TreeViewState m_TreeViewState = null;
#pragma warning restore CS0618

The TreeViewItem without a type specified is obsolete and should be replaced.

In 6.5 usages of obsolete APIs will cause errors, which is necessary as the size of EntityId will change from 32bit to 64bit.

As per trunk rules of engagement, we will be disabling trunk tests to unblock current 6.5 work. These tests can be re-enabled once the deprecated API usages are addressed.

If you have any questions, don't hesitate to post them in the #devs-entities channel.

Links

https://jira.unity3d.com/browse/UUM-132061

Comments to Reviewers

N/A

@varinotmUnity varinotmUnity self-assigned this Jan 13, 2026
@u-pr-agent
Copy link

u-pr-agent bot commented Jan 13, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

UUM-132061 - Partially compliant

Compliant requirements:

  • Replace deprecated non-generic TreeViewItem with TreeViewItem<T>
  • Replace deprecated non-generic TreeViewState with TreeViewState<T>
  • Update affected files in com.unity.probuilder to remove obsolete API usages that will become errors in 6.5

Non-compliant requirements:

  • Prefer TreeViewItem<InstanceID> / TreeViewState<InstanceID> when the identifier represents an InstanceID

Requires further human verification:

  • Confirm the chosen generic identifier type (int) is correct for each TreeView (i.e., not actually representing InstanceIDs) across supported Unity versions (6.4/6.5).
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

Mostly mechanical generic-type updates across a few editor tree view classes, with limited behavioral change but requiring compile checks across Unity version defines.
🏅 Score: 86

The PR appropriately migrates off obsolete TreeView APIs, but the choice of `int` instead of a stronger `InstanceID` type may not satisfy the intent of the deprecation and should be validated.
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Type Choice

The TreeView migration uses TreeView<int> / TreeViewItem<int> and TreeViewState<int>, but if the item ids are actually Unity InstanceIDs the ticket guidance suggests using InstanceID-typed TreeView generics; verify that these ids are not instance ids (they appear to be custom incremental node ids) and that no downstream code assumes InstanceID semantics.

sealed class AssetTreeItem : TreeViewItem<int>
{
    string m_RelativePath;
    string m_FullPath;
    bool m_IsEnabled;
    bool m_IsDirectory;
    bool m_IsMixedState;

    public AssetTreeItem(int id, string fullPath, string relativePath) : base(id, 0)
    {
        m_IsDirectory = Directory.Exists(fullPath);
        m_FullPath = fullPath;
        m_RelativePath = relativePath;
        m_IsEnabled = true;
        displayName = m_FullPath.Replace("\\", "/").Replace(Application.dataPath, "Assets/");
    }

    public bool enabled
    {
        get { return m_IsEnabled; }
        set { m_IsEnabled = value; }
    }

    public bool isDirectory
    {
        get { return m_IsDirectory; }
        set { m_IsDirectory = value; }
    }

    public string fullPath
    {
        get { return m_FullPath; }
    }

    public string relativePath
    {
        get { return m_RelativePath; }
    }

    public bool isMixedState { get { return m_IsMixedState; } }

    public void SetEnabled(bool isEnabled)
    {
        enabled = isEnabled;

        if (children != null)
        {
            foreach (var child in children)
            {
                AssetTreeItem asset = child as AssetTreeItem;

                if (asset != null)
                    asset.SetEnabled(isEnabled);
            }
        }

        var upstream = parent;

        while (upstream != null)
        {
            var up = upstream as AssetTreeItem;

            if (up != null && up.children != null)
            {
                AssetTreeItem firstChild = up.children.FirstOrDefault() as AssetTreeItem;

                if (firstChild != null)
                {
                    up.m_IsMixedState = up.children.Any(x =>
                        {
                            var y = x as AssetTreeItem;
                            return y.enabled != firstChild.enabled;
                        });

                    if (!up.m_IsMixedState)
                        up.enabled = firstChild.enabled;
                }
                else
                {
                    up.m_IsMixedState = false;
                }
            }

            upstream = upstream.parent;
        }
    }
}

class AssetTreeView : TreeView<int>
{
    string m_RootDirectory = null;
    GUIContent m_TempContent = new GUIContent();
    Rect m_ToggleRect = new Rect(0, 0, 0, 0);

    public string directoryRoot
    {
        get { return m_RootDirectory; }
        set { m_RootDirectory = value; }
    }

    public AssetTreeItem GetRoot()
    {
        return rootItem.hasChildren
            ? rootItem.children.First() as AssetTreeItem
            : null;
    }

    IEnumerable<Regex> m_DirectoryIgnorePatterns;
    IEnumerable<Regex> m_FileIgnorePatterns;

    public void SetDirectoryIgnorePatterns(string[] regexStrings)
    {
        m_DirectoryIgnorePatterns = regexStrings.Select(x => new Regex(x));
    }

    public void SetFileIgnorePatterns(string[] regexStrings)
    {
        m_FileIgnorePatterns = regexStrings.Select(x => new Regex(x));
    }

    public AssetTreeView(TreeViewState<int> state, MultiColumnHeader header) : base(state, header)
    {
        showBorder = true;
        columnIndexForTreeFoldouts = 0;
        rowHeight = 18f;
    }
    protected override TreeViewItem<int> BuildRoot()
    {
        AssetTreeItem root = new AssetTreeItem(0, Application.dataPath, "")
        {
            depth = -1,
            enabled = false
        };

        if (string.IsNullOrEmpty(m_RootDirectory) || !Directory.Exists(m_RootDirectory))
        {
            // if root has no children and you SetupDepthsFromParentsAndChildren nullrefs are thrown
            var list = new List<TreeViewItem<int>>() {};
            SetupParentsAndChildrenFromDepths(root, list);
Signature Update

RemoveAssetStoreFiles now accepts TreeViewItem<int>; ensure all call sites (including recursive traversal) pass the correct generic type and no implicit conversions remain, especially where the root may be constructed/returned from another TreeView implementation.

bool RemoveAssetStoreFiles(TreeViewItem<int> root, StringBuilder log)
{
    AssetTreeItem node = root as AssetTreeItem;

    if (node != null && (node.enabled && !node.isMixedState))
Obsolete API

The legacy branch still calls EditorUtility.InstanceIDToObject(int); confirm this API is not considered part of the "all APIs that take in InstanceID" removals for the targeted Unity versions, or add appropriate version-conditional alternative if it becomes an error before UNITY_6000_4_OR_NEWER.

static GameObject GetGameObject(CreateGameObjectHierarchyEventArgs data) => InstanceIDToObject(data.instanceId);
static GameObject GetGameObject(ChangeGameObjectStructureEventArgs data) => InstanceIDToObject(data.instanceId);
static GameObject GetGameObject(ChangeGameObjectStructureHierarchyEventArgs data) => InstanceIDToObject(data.instanceId);

static GameObject InstanceIDToObject(int instanceId)
{
    return UnityEditor.EditorUtility.InstanceIDToObject(instanceId) as GameObject;
}
  • Update review

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

@u-pr-agent
Copy link

u-pr-agent bot commented Jan 13, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent null reference in tree building

BuildRoot() assumes remapObject and remapObject.map are non-null; if the window is
opened before a remap is assigned, this will nullref and break the UI. Guard early
and return an empty root when remapObject/map is missing. This keeps the TreeView
functional and lets the editor continue rendering.

AssetIdRemapUtility/Editor/AssetIdRemapBuilderTreeView.cs [35-44]

 protected override TreeViewItem<int> BuildRoot()
 {
     StringTupleTreeElement root = new StringTupleTreeElement(0, -1, -1, "Root", "", "");
 
     var all = new List<TreeViewItem<int>>();
 
+    if (remapObject == null || remapObject.map == null)
+    {
+        SetupParentsAndChildrenFromDepths(root, all);
+        return root;
+    }
+
     int index = 1;
 
     for (int i = 0, c = remapObject.map.Count; i < c; i++)
Suggestion importance[1-10]: 8

__

Why: BuildRoot() dereferences remapObject.map without a null-check; if remapObject isn't set yet, the editor UI can throw and break rendering. An early guard returning an empty root is a correct and user-facing stability improvement.

Medium
Fix hidden-file filtering logic

path.StartsWith(".") will never match hidden files when path is a full path (it
starts with a drive letter or /). Check the filename instead, otherwise hidden files
like .DS_Store can slip into the tree and later deletion logic. Use
Path.GetFileName(path) (or Path.GetFileName(unixPath)) before checking for leading
..

AssetIdRemapUtility/Editor/AssetTreeView.cs [179-190]

 foreach (string path in Directory.GetFiles(directory, "*", SearchOption.TopDirectoryOnly))
 {
-    if (path.StartsWith(".") || path.EndsWith(".meta"))
+    if (path.EndsWith(".meta"))
         continue;
 
     string unixPath = path.Replace("\\", "/");
+    var fileName = Path.GetFileName(unixPath);
+
+    if (!string.IsNullOrEmpty(fileName) && fileName[0] == '.')
+        continue;
 
     leaf.AddChild(new AssetTreeItem(
             nodeIdIndex++,
             unixPath,
             unixPath.Replace(unixDirectory, "").Trim('/')) { enabled = true });
 }
Suggestion importance[1-10]: 7

__

Why: In PopulateAssetTree, path is a full path so path.StartsWith(".") will not filter dotfiles (eg .DS_Store), which can lead to incorrect items appearing in the tree (and potentially being deleted). Using Path.GetFileName(...) makes the filter behave as intended with low risk.

Medium
  • More suggestions

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

@codecov-github-com
Copy link

codecov-github-com bot commented Jan 13, 2026

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
AssetIdRemapUtility/Editor/AssetIdRemapEditor.cs 0.00% 2 Missing ⚠️
AssetIdRemapUtility/Editor/AssetTreeView.cs 0.00% 2 Missing ⚠️
@@           Coverage Diff           @@
##           master     #645   +/-   ##
=======================================
  Coverage   36.05%   36.05%           
=======================================
  Files         277      277           
  Lines       34899    34899           
=======================================
  Hits        12583    12583           
  Misses      22316    22316           
Flag Coverage Δ
mac_trunk 34.59% <0.00%> (ø)
win_trunk 34.48% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
Editor/EditorCore/HierarchyListener.cs 65.85% <ø> (ø)
AssetIdRemapUtility/Editor/AssetIdRemapEditor.cs 0.00% <0.00%> (ø)
AssetIdRemapUtility/Editor/AssetTreeView.cs 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants