Commit 5e1cecf
authored
feat(library): icon-based row-action buttons on Instantiated tab (#830)
## Summary
Replaces the text-labeled Select / Teleport / Remove buttons on each row
of the Library's Instantiated tab with icon-only colored beveled squares
(80×80). The three buttons preserve their previous semantic hierarchy
via the existing `AcceptButton` (green) / `StandardButton` (blue) /
`CancelButton` (red) prefab styles.
### User-affecting changes
- **Buttons are icon-only.** The existing localization keys
(`library.select`, `library.deselect`, `library.teleportTo`,
`library.remove`) remain in the locale files for future tooltip / a11y
use, so no translations are lost.
- **Buttons are hidden (not just disabled) for scene-mode and embedded
instances.** Previously these rendered as disabled buttons that still
occupied row width and added visual noise. The underlying scene-unload
code paths in the OnClicked handlers are intentionally left in place so
the functionality can be exposed via a different UI surface later — only
the library-window entry point goes away.
### Sprite additions (`com.basis.sdk`)
| Sprite | Source | Status |
|---|---|---|
| `scan-outline.png` | Ionicons | new |
| `link-outline.png` | Ionicons | new, consumed by follow-up
trackerobjects integration PR |
| `unlink-outline.png` | Ionicons | new, consumed by follow-up
trackerobjects integration PR |
| `trash-bin-outline.png` | already on disk | newly registered as
addressable in the Basis UI Assets group |
All sprites match the existing project icon convention: white-stroke
RGBA at 512×512 on transparent background. Source:
https://github.com/ionic-team/ionicons (MIT). PNGs rendered from the
upstream SVGs.
### API additions
Five new entries on `AddressableAssets.Sprites`: `Select`, `TeleportTo`,
`Trash`, `Link`, `Unlink`. `TeleportTo` aliases the existing
`Teleport.png` (was only exposed via `Respawn` before). `Link` and
`Unlink` ship here for the follow-up trackerobjects integration package
PR to consume.
## Required checks
All boxes below must be ticked before this PR can merge. If a check is
genuinely N/A, tick it anyway and explain under **Notes**.
<!-- required-checks-start -->
<!-- Tick the boxes in place — do not edit the line text. The
pr-checklist workflow parses this block; per-PR context goes under
Notes. -->
- [x] **Tested** — I built and ran this locally. The change works in the
editor and (where relevant) in a built player.
- [x] **Transform access is combined and limited** — In hot paths,
transform reads/writes go through `TransformAccessArray` or are
otherwise batched. I have not added per-frame `transform.position` /
`transform.rotation` / `transform.localPosition` calls inside loops.
Whenever I need both position and rotation, I use the combined APIs —
`SetPositionAndRotation` / `SetLocalPositionAndRotation` for writes,
`GetPositionAndRotation` / `GetLocalPositionAndRotation` for reads —
instead of two separate property accesses; the combined call does one
local-to-world matrix traversal instead of two.
- [x] **Addressables used for asset/memory loading** — Any new asset
loads go through Addressables. No new `Resources.Load`, no direct asset
references that pull large content into memory on scene load.
- [x] **No new `GetComponent` / `AddComponent` where avoidable** — Where
unavoidable, the result is cached on a field, and any `GetComponent<T>`
is replaced with `TryGetComponent<T>(out var x)` — bare `GetComponent`
will be denied. `TryGetComponent` is the modern API (Unity 2019.2+) and
skips the Editor-only GC allocation `GetComponent` causes when a
component is missing: Unity wraps the `null` return in a managed "fake
null" object so its overloaded `==` operator can still detect destroyed
C++ objects, and constructing that wrapper allocates; `TryGetComponent`
returns a `bool` plus `out` parameter and never builds the wrapper. None
of these calls run inside `Update`, `LateUpdate`, `FixedUpdate`, jobs,
or other per-frame code paths.
- [x] **Per-frame work is scheduled through `BasisEventDriver`** — Any
new per-frame work hooks into `BasisEventDriver` rather than adding
standalone `Update` / `LateUpdate` / `FixedUpdate` callbacks on a
MonoBehaviour.
- [x] **Considered jobification** — I asked whether this work can be
moved to a Unity Job (Burst-compiled where possible). If it can, it is.
If it cannot, the reason is in **Notes**.
- [x] **No needless `{ get; set; }` properties or access lockdowns** —
Public fields are fine; Basis is meant to be read and modified freely,
so don't wall things off `private`/`internal` without a real reason.
Don't wrap a field in `{ get; set; }` when the accessors do nothing —
property accessors have a real performance cost vs direct field access,
and the lead maintainer prefers plain fields (or a method / setter-only
property when only the setter needs logic) over a noop-getter pair. For
`.Instance` singletons, callers reassigning `Type.Instance` is allowed;
if that would break your code, log a warning or throw — don't block the
assignment. Locking down access is not your call.
- [x] **Camera access goes through `BasisLocalCameraDriver`** — Code
that needs the local camera (transform, projection, rig data, etc.)
pulls it from `BasisLocalCameraDriver` rather than looking one up
itself. Don't roll a separate camera discovery path.
- [x] **Logging uses `BasisDebug`** — All new logging calls go through
`BasisDebug.Log` / `BasisDebug.LogWarning` / `BasisDebug.LogError` (with
an appropriate `LogTag`) instead of `UnityEngine.Debug.Log` /
`Debug.LogWarning` / `Debug.LogError`. `BasisDebug` routes through
Basis's tagged, color-coded logger and respects the project-wide
`LoggingDisabled` toggle so logging can be killed at runtime; bare
`Debug.Log` calls bypass that and will be denied.
- [x] **No scene-wide discovery for dependencies** — New code is
architected so it does not need `FindObjectOfType` / `FindObjectsOfType`
/ `GameObject.Find` / `FindGameObjectsWithTag` to locate what it depends
on. References are wired in — registered through an existing
manager/driver, injected at init, or passed in by the caller — rather
than discovered by scanning the scene at runtime. If a scene scan is
genuinely unavoidable, justify it under **Notes**.
- [x] **No allocations in hot paths** — Per-frame code (Update /
LateUpdate / FixedUpdate, simulation loops, jobs, anything called once
per frame or more) does not allocate. No `new` on reference types, no
LINQ, no `string` concatenation/interpolation, no boxing, no `foreach`
over interface-typed collections. Allocate once at init and reuse the
buffer.
- [x] **No debugging in hot paths** — No log calls of any kind on
per-frame paths, including `BasisDebug`. Hot-path logging floods the
console and incurs cost on every frame regardless of whether the message
is filtered out downstream. If a hot-path log is needed while iterating,
gate it behind `#if UNITY_EDITOR` and remove (or leave gated) before
merge.
- [x] **Hot-path collection access is optimized** — Cache `.Count`
(lists) / `.Length` (arrays) into a local `int` before the loop instead
of re-reading the property each iteration. Prefer `T[]` (with a separate
length int when the array is over-sized) over `List<T>` where the data
is hot — Unity's mono BCL doesn't expose
`CollectionsMarshal.AsSpan(List<T>)`, so a list can't be fed into
`Span<T>` / unsafe paths cleanly. Where the perf justifies it, drop into
`Span<T>` / `ref` locals / `Unsafe.As` / `unsafe` pointer code to skip
bounds checks and copies, and call out the invariants you're relying on
under **Notes** so reviewers can sanity-check them.
<!-- required-checks-end -->
## Testing details
Tick the platforms you actually tested on. Leave the rest unticked —
these are informational and do not block merge.
- [x] Windows
- [ ] Linux
- [ ] Android
- [ ] iOS
- [ ] macOS
Input / control mode coverage:
- [ ] Tested in VR (note headset under **Notes**)
- [x] Tested in desktop / non-VR mode
- [ ] Tested with phone controls (mobile touch input)
- [ ] N/A — change does not touch player/XR/input code
Where applicable, confirm these flows still work after your changes:
- [ ] Hot-switching (desktop ↔ VR mode swap at runtime)
- [ ] Avatar swapping
- [ ] Server swapping (joining / leaving / changing servers)
- [x] N/A — change does not touch any of the above
## Notes
- **Required-check N/A justifications.** This is a UI-only change:
row-button restyling plus a few new addressable sprite entries. No
transform access, no per-frame work, no jobification candidates, no
camera access, no new logging, no scene-wide discovery, no hot-path
code, no `GetComponent`/`AddComponent` additions, no `{ get; set; }`
lockdowns. Existing `TryGetComponent<Button>` calls in
`LibraryProvider.CreateListEntry` are retained unchanged. The
required-checks boxes are ticked because the project's checklist rule is
to tick + explain N/A items here; they aren't claims that the diff
exercised those code paths.
- **Verified live in the editor.** Buttons were iterated visually across
multiple in-game reviews — caption crowding, icon stroke colour (white
to match the existing Ionicon set), and the colored-bevel vs Hotbar
layout were all tightened based on what rendered correctly on a real
avatar + prop spawn.
- **Scene/embedded unload code paths intact.** The `OnClicked` handlers
for Teleport and Remove still contain their original `case
SpawnMode.Scene` branches and the `case SpawnMethod.Embedded`
fall-through label — the early return at the top of `CreateListEntry`
simply prevents this UI surface from ever reaching them. Those code
paths predate this PR and may want to be surfaced via a different UI (a
moderator panel, perhaps) in a follow-up; deliberately not removed here.
- **Follow-up trackerobjects PR** depends on this one merging first so
that `AddressableAssets.Sprites.Link` / `.Unlink` resolve at compile
time. Its branch is `feat/trackerobjects` on this fork; the icon-swap
commit on that branch is held back until this lands.9 files changed
Lines changed: 432 additions & 18 deletions
File tree
- Basis
- Assets/AddressableAssetsData/AssetGroups
- Packages
- com.basis.framework/BasisUI
- Addressables
- Menus/Library
- com.basis.sdk/Textures/Runtime
Lines changed: 20 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
116 | 116 | | |
117 | 117 | | |
118 | 118 | | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
119 | 124 | | |
120 | 125 | | |
121 | 126 | | |
| |||
179 | 184 | | |
180 | 185 | | |
181 | 186 | | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
182 | 192 | | |
183 | 193 | | |
184 | 194 | | |
| |||
248 | 258 | | |
249 | 259 | | |
250 | 260 | | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
251 | 266 | | |
252 | 267 | | |
253 | 268 | | |
| |||
263 | 278 | | |
264 | 279 | | |
265 | 280 | | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
266 | 286 | | |
267 | 287 | | |
268 | 288 | | |
| |||
Lines changed: 7 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
43 | 50 | | |
44 | 51 | | |
45 | 52 | | |
| |||
Lines changed: 15 additions & 18 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1944 | 1944 | | |
1945 | 1945 | | |
1946 | 1946 | | |
1947 | | - | |
1948 | | - | |
1949 | | - | |
1950 | | - | |
1951 | | - | |
1952 | | - | |
| 1947 | + | |
| 1948 | + | |
| 1949 | + | |
1953 | 1950 | | |
1954 | | - | |
1955 | | - | |
| 1951 | + | |
1956 | 1952 | | |
1957 | 1953 | | |
| 1954 | + | |
| 1955 | + | |
| 1956 | + | |
| 1957 | + | |
| 1958 | + | |
1958 | 1959 | | |
1959 | 1960 | | |
1960 | 1961 | | |
| |||
1975 | 1976 | | |
1976 | 1977 | | |
1977 | 1978 | | |
1978 | | - | |
1979 | | - | |
1980 | | - | |
1981 | | - | |
1982 | | - | |
1983 | | - | |
1984 | | - | |
1985 | | - | |
| 1979 | + | |
| 1980 | + | |
| 1981 | + | |
1986 | 1982 | | |
1987 | 1983 | | |
1988 | 1984 | | |
| |||
2013 | 2009 | | |
2014 | 2010 | | |
2015 | 2011 | | |
2016 | | - | |
2017 | | - | |
| 2012 | + | |
| 2013 | + | |
| 2014 | + | |
2018 | 2015 | | |
2019 | 2016 | | |
2020 | 2017 | | |
| |||
Lines changed: 130 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 130 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
0 commit comments