Rewrite of studio toolbar API with new features; and added GlobalTooltips #85

Merged
ManlyMarco merged 24 commits from wip-toolbar into master 2025-09-16 19:08:50 +00:00
ManlyMarco commented 2025-09-15 16:46:17 +00:00 (Migrated from github.com)
  • Replaced the old CustomToolbarButtons API with ToolbarManager, bringing many improvements and new features.
  • Toolbar buttons now can be drag-and-drop moved and reordered (persists across restarts).
  • Added new GlobalTooltips API for easily adding tooltips to in game and studio.
  • PHAPI got some free improvements as a side effect of refactoring.
- Replaced the old CustomToolbarButtons API with ToolbarManager, bringing many improvements and new features. - Toolbar buttons now can be drag-and-drop moved and reordered (persists across restarts). - Added new GlobalTooltips API for easily adding tooltips to in game and studio. - PHAPI got some free improvements as a side effect of refactoring.
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-09-15 16:49:34 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR implements a comprehensive rewrite of the studio toolbar API with enhanced features and introduces a new GlobalTooltips system for displaying tooltips throughout the game and studio. The rewrite provides better positioning control, drag-and-drop functionality, and persistence of toolbar layouts.

  • Introduces GlobalTooltips API for unified tooltip handling across game and studio interfaces
  • Replaces the old CustomToolbarButtons system with new ToolbarManager and related classes
  • Adds drag-and-drop support and persistent positioning for toolbar buttons

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
GlobalTooltips.cs New utility class providing API for displaying tooltips with hover delays and positioning
ToolbarManager.cs New toolbar management system with thread-safe operations and layout control
ToolbarControlBase.cs Base class for toolbar controls with drag-and-drop and positioning features
ToolbarDataStorage.cs Handles persistent storage of toolbar button positions in config
SimpleToolbarButton.cs Simple button implementation for the new toolbar system
SimpleToolbarToggle.cs Toggle button implementation with visual state management
ToolbarControlAdapter.cs Adapter for stock game toolbar buttons to work with the new system
Obsolete.cs Backward compatibility wrapper for the old CustomToolbarButtons API
Various project files Updates to include new files and reference changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

## Pull Request Overview This PR implements a comprehensive rewrite of the studio toolbar API with enhanced features and introduces a new GlobalTooltips system for displaying tooltips throughout the game and studio. The rewrite provides better positioning control, drag-and-drop functionality, and persistence of toolbar layouts. - Introduces GlobalTooltips API for unified tooltip handling across game and studio interfaces - Replaces the old CustomToolbarButtons system with new ToolbarManager and related classes - Adds drag-and-drop support and persistent positioning for toolbar buttons ### Reviewed Changes Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | GlobalTooltips.cs | New utility class providing API for displaying tooltips with hover delays and positioning | | ToolbarManager.cs | New toolbar management system with thread-safe operations and layout control | | ToolbarControlBase.cs | Base class for toolbar controls with drag-and-drop and positioning features | | ToolbarDataStorage.cs | Handles persistent storage of toolbar button positions in config | | SimpleToolbarButton.cs | Simple button implementation for the new toolbar system | | SimpleToolbarToggle.cs | Toggle button implementation with visual state management | | ToolbarControlAdapter.cs | Adapter for stock game toolbar buttons to work with the new system | | Obsolete.cs | Backward compatibility wrapper for the old CustomToolbarButtons API | | Various project files | Updates to include new files and reference changes | </details> --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/IllusionMods/IllusionModdingAPI/new/master/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>
@ -0,0 +104,4 @@
if (_iconTex == null)
throw new InvalidOperationException($"Icon texture getter for {ButtonID} returned null");
if (_iconTex.width != 32 || _iconTex.height != 32)
KoikatuAPI.Logger.LogWarning($"Icon texture passed to {ButtonID} has wrong size, it should be 32x32 but is {_iconTex.width}x{_iconTex.height}");
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-09-15 16:49:34 +00:00

[nitpick] The warning message should specify that this is just a recommendation rather than a requirement, since the code continues to work with non-32x32 textures. Consider clarifying this in the warning message.

                    KoikatuAPI.Logger.LogWarning($"Icon texture passed to {ButtonID} is {_iconTex.width}x{_iconTex.height}; 32x32 is recommended for best results, but other sizes are supported.");
[nitpick] The warning message should specify that this is just a recommendation rather than a requirement, since the code continues to work with non-32x32 textures. Consider clarifying this in the warning message. ```suggestion KoikatuAPI.Logger.LogWarning($"Icon texture passed to {ButtonID} is {_iconTex.width}x{_iconTex.height}; 32x32 is recommended for best results, but other sizes are supported."); ```
@ -0,0 +151,4 @@
var button = ButtonObject;
if (button) Object.Destroy(button.gameObject);
Object.Destroy(_iconTex);
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-09-15 16:49:34 +00:00

Calling Object.Destroy on _iconTex may destroy textures that were provided by external code and shouldn't be destroyed by this class. Consider only destroying textures that were created internally, or document that provided textures will be destroyed.

Calling Object.Destroy on _iconTex may destroy textures that were provided by external code and shouldn't be destroyed by this class. Consider only destroying textures that were created internally, or document that provided textures will be destroyed.
@ -0,0 +1,107 @@
using System;
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-09-15 16:49:34 +00:00

Using Contains() on a List for duplicate detection has O(n) complexity. Consider using a HashSet to track seen entries for better performance when dealing with many toolbar buttons.

            var seenKeys = new HashSet<string>();

            foreach (var b in buttons.Where(b => b.DesiredRow >= 0 && b.DesiredColumn >= 0))
            {
                var saveKey = GetUniqueName(b);
                if (seenKeys.Contains(saveKey))
                {
                    duplicateIds.Add(saveKey);
                    continue;
                }
                seenKeys.Add(saveKey);
Using Contains() on a List<string> for duplicate detection has O(n) complexity. Consider using a HashSet<string> to track seen entries for better performance when dealing with many toolbar buttons. ```suggestion var seenKeys = new HashSet<string>(); foreach (var b in buttons.Where(b => b.DesiredRow >= 0 && b.DesiredColumn >= 0)) { var saveKey = GetUniqueName(b); if (seenKeys.Contains(saveKey)) { duplicateIds.Add(saveKey); continue; } seenKeys.Add(saveKey); ```
@ -0,0 +1,197 @@
using System;
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-09-15 16:49:33 +00:00

Modifying a list while iterating with RemoveAt() and manual index adjustment can be inefficient for large lists. Consider using a reverse loop or collecting items to remove and processing them separately.

            for (var i = _tooltips.Count - 1; i >= 0; i--)
            {
                var data = _tooltips[i];

                if (data.Target == null || data.Tooltip.IsDestroyed)
                {
                    _tooltips.RemoveAt(i);
Modifying a list while iterating with RemoveAt() and manual index adjustment can be inefficient for large lists. Consider using a reverse loop or collecting items to remove and processing them separately. ```suggestion for (var i = _tooltips.Count - 1; i >= 0; i--) { var data = _tooltips[i]; if (data.Target == null || data.Tooltip.IsDestroyed) { _tooltips.RemoveAt(i); ```
@ -0,0 +59,4 @@
{
_tooltipBackground = new Texture2D(1, 1, TextureFormat.ARGB32, false);
_tooltipBackground.SetPixel(0, 0, new Color(0, 0, 0, 0.65f));
_tooltipBackground.Apply();
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-09-15 16:49:33 +00:00

The tooltip background texture is created every frame in Update() instead of being initialized once. Consider moving this initialization to a static constructor or separate initialization method called once at startup.

The tooltip background texture is created every frame in Update() instead of being initialized once. Consider moving this initialization to a static constructor or separate initialization method called once at startup.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
IllusionMods/IllusionModdingAPI!85
No description provided.