Replayerui improvements#204
Conversation
| using UnityEngine; | ||
|
|
||
| namespace BeatLeader.Replayer.Binding { | ||
| internal static class ReplayControlsActions { |
There was a problem hiding this comment.
Very weird decision, hotkeys are built around zenject, so you can simply create a separate service to unify logic
| /// <summary> | ||
| /// Whether seeking/speed control via the VR controller thumbsticks is enabled. | ||
| /// </summary> | ||
| public bool Enabled { get; set; } = true; |
There was a problem hiding this comment.
Let's rename it to VrControlsEnabled for clarity
| public bool CurvatureEnabled { get; set; } | ||
| public bool AttachToHand { get; set; } | ||
| public bool AttachToRightHand { get; set; } | ||
| public SerializablePose HandOffset { get; set; } = new( |
There was a problem hiding this comment.
Remove default values, they're already applied in ConfigDefaults
| public KeyCode PauseHotkey { get; set; } | ||
| public KeyCode RewindForwardHotkey { get; set; } | ||
| public KeyCode RewindBackwardHotkey { get; set; } | ||
| public KeyCode SpeedUpHotkey { get; set; } = KeyCode.Plus; |
| } | ||
| } | ||
|
|
||
| internal class SettingsControlsTabs { |
There was a problem hiding this comment.
Split into separate components and stop using AI for everything
| public void Setup(FloatingScreen screen, Camera camera, ReplayerFloatingUISettings settings) { | ||
| public void Setup( | ||
| FloatingScreen screen, | ||
| ToolbarWithSettings mainUi, |
There was a problem hiding this comment.
Instead of directly relying on that specific type, let's use events e.g. MainUiStateChangedEvent
| _screen = screen; | ||
| _mainUi = mainUi; | ||
| _settings = settings; | ||
| screen.gameObject.GetOrAddComponent<FloatingScreenHandFollower>().Setup(settings, menuControllersManager); |
There was a problem hiding this comment.
This is a ui component, you shouldn't initialize unrelated scripts here
| } | ||
| } | ||
|
|
||
| private void LateUpdate() { |
There was a problem hiding this comment.
Why checking state in each frame?
| var handTransform = GetHandTransform(settings); | ||
| if (handTransform == null) return; | ||
|
|
||
| if (!_attached || transform.parent != handTransform) { |
There was a problem hiding this comment.
Alongside the update cycle this is very expensive as it creates a native wrapper on each Transform instance equality comparison
| using UnityEngine; | ||
|
|
||
| namespace BeatLeader.UI.Replayer { | ||
| internal class FloatingScreenHandFollower : MonoBehaviour { |
There was a problem hiding this comment.
After refining apply in ReplayerFloatingUIBinder
2 participants
No description provided.