Skip to content

Commit

Permalink
address UX comments from design team
Browse files Browse the repository at this point in the history
  • Loading branch information
@glabute
glabute committed Nov 15, 2023
1 parent 15a93d3 commit 8ed285a
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 92 deletions.
Binary file modified com.unity.cinemachine/Documentation~/images/CmCameraInspector.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 3 additions & 3 deletions ...emachine/Editor/Editors/CmCameraEditor.cs → ...Editor/Editors/CinemachineCameraEditor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Unity.Cinemachine.Editor
{
[CustomEditor(typeof(CinemachineCamera))]
[CanEditMultipleObjects]
class CmCameraEditor : UnityEditor.Editor
class CinemachineCameraEditor : UnityEditor.Editor
{
CinemachineCamera Target => target as CinemachineCamera;

Expand Down Expand Up @@ -53,17 +53,17 @@ public override VisualElement CreateInspectorGUI()
this.AddCameraStatus(ux);
this.AddTransitionsSection(ux, new () { serializedObject.FindProperty(() => Target.BlendHint) });
ux.Add(new PropertyField(serializedObject.FindProperty(() => Target.Lens)));
ux.Add(new PropertyField(serializedObject.FindProperty(() => Target.Target)));

ux.AddHeader("Global Settings");
this.AddGlobalControls(ux);

var defaultTargetLabel = new Label() { style = { alignSelf = Align.FlexEnd, opacity = 0.5f }};
var row = ux.AddChild(new InspectorUtility.LabeledRow("<b>Procedural Motion</b>", "", defaultTargetLabel));
var row = ux.AddChild(new InspectorUtility.LabeledRow("<b>Set Procedural Components</b>", "", defaultTargetLabel));
row.focusable = false;
row.style.paddingTop = InspectorUtility.SingleLineHeight / 2;
row.style.paddingBottom = EditorGUIUtility.standardVerticalSpacing;

ux.Add(new PropertyField(serializedObject.FindProperty(() => Target.Target)));
this.AddPipelineDropdowns(ux);

ux.AddSpace();
Expand Down
0 ...ine/Editor/Editors/CmCameraEditor.cs.meta → ...r/Editors/CinemachineCameraEditor.cs.meta
View file
File renamed without changes.
5 changes: 3 additions & 2 deletions com.unity.cinemachine/Editor/Editors/CinemachineFreeLookModifierEditor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ public override VisualElement CreateInspectorGUI()
var ux = new VisualElement();

var invalidSrcMsg = ux.AddChild(
new HelpBox("No applicable components found. Must have one of: "
new HelpBox("<b>This component will be ignored because no applicable target components are present.</b>\n\n"
+ "Applicable target components include: "
+ InspectorUtility.GetAssignableBehaviourNames(
typeof(CinemachineFreeLookModifier.IModifierValueSource)),
HelpBoxMessageType.Warning));
Expand Down Expand Up @@ -80,7 +81,7 @@ public override VisualElement CreatePropertyGUI(SerializedProperty property)
if (property.managedReferenceValue is not CinemachineFreeLookModifier.Modifier m)
return new Label("invalid item");

var warningText = "No applicable components found. Must have one of: "
var warningText = "No applicable targets found. Applicable targets include: "
+ InspectorUtility.GetAssignableBehaviourNames(m.CachedComponentType);

var overlay = new VisualElement { style = { flexDirection = FlexDirection.Row, flexGrow = 1 }};
Expand Down
2 changes: 1 addition & 1 deletion com.unity.cinemachine/Editor/Editors/CinemachineGroupFramingEditor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ void OnDisable()
public override VisualElement CreateInspectorGUI()
{
var ux = new VisualElement();
this.AddMissingCmCameraHelpBox(ux, CmPipelineComponentInspectorUtility.RequiredTargets.Group);
this.AddMissingCmCameraHelpBox(ux, CmPipelineComponentInspectorUtility.RequiredTargets.GroupLookAt);
var groupSizeIsZeroHelp = ux.AddChild(new HelpBox("Group size is zero, cannot frame.", HelpBoxMessageType.Warning));

ux.Add(new PropertyField(serializedObject.FindProperty(() => Target.FramingMode)));
Expand Down
3 changes: 2 additions & 1 deletion com.unity.cinemachine/Editor/Editors/InputAxisControllerEditor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ public override VisualElement CreatePropertyGUI(SerializedProperty property)
list.BindProperty(property);

var isEmptyMessage = ux.AddChild(new HelpBox(
"No applicable components found. Must have one of: "
"<b>This component will be ignored because no applicable target components are present.</b>\n\n"
+ "Applicable target components include: "
+ InspectorUtility.GetAssignableBehaviourNames(typeof(IInputAxisOwner)),
HelpBoxMessageType.Warning));
list.TrackPropertyWithInitialCallback(
Expand Down
66 changes: 32 additions & 34 deletions com.unity.cinemachine/Editor/PropertyDrawers/LensSettingsPropertyDrawer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ System.Reflection.BindingFlags bindingFlags

static List<string> m_PresetOptions;
static List<string> m_PhysicalPresetOptions;
const string k_AddPresetsLabel = "New Preset with these Settings...";
const string k_EditPresetsLabel = "Edit Presets...";
const string k_AddPresetsLabel = "New Palette entry with these Settings...";
const string k_EditPresetsLabel = "Edit Palette...";
const string k_PaletteLabel = "Palette...";
float m_PreviousAspect;

protected bool HideModeOverride { get; set; }
Expand All @@ -61,14 +62,14 @@ void InitPresetOptions()
{
m_PresetOptions ??= new List<string>();
m_PresetOptions.Clear();
var presets = CinemachineLensPresets.InstanceIfExists;
for (int i = 0; presets != null && i < presets.Presets.Count; ++i)
m_PresetOptions.Add(presets.Presets[i].Name);
var palette = CinemachineLensPalette.InstanceIfExists;
for (int i = 0; palette != null && i < palette.Presets.Count; ++i)
m_PresetOptions.Add(palette.Presets[i].Name);
m_PresetOptions.Add("");
m_PresetOptions.Add(k_AddPresetsLabel);
m_PresetOptions.Add(k_EditPresetsLabel);

var physicalPresets = CinemachinePhysicalLensPresets.InstanceIfExists;
var physicalPresets = CinemachinePhysicalLensPalette.InstanceIfExists;
m_PhysicalPresetOptions ??= new List<string>();
m_PhysicalPresetOptions.Clear();
for (int i = 0; physicalPresets != null && i < physicalPresets.Presets.Count; ++i)
Expand Down Expand Up @@ -115,7 +116,7 @@ public override VisualElement CreatePropertyGUI(SerializedProperty property)
{
modeOverrideProperty = property.FindPropertyRelative(() => s_Def.ModeOverride);
modeHelp = foldout.AddChild(
new HelpBox("Lens Mode Override must be enabled in the CM Brain for Mode Override to take effect",
new HelpBox("Lens Mode Override must be enabled in the Cinemachine Brain for Mode Override to take effect",
HelpBoxMessageType.Warning));
foldout.AddChild(new PropertyField(modeOverrideProperty)).TrackPropertyValue(
modeOverrideProperty, (p) => InspectorUtility.RepaintGameView());
Expand Down Expand Up @@ -191,7 +192,7 @@ public FovPropertyControl(SerializedProperty property, bool hideLabel) : base(hi
new FieldMouseDragger<float>(m_Control).SetDragZone(Label);

m_Presets = Contents.AddChild(new PopupField<string>
{ tooltip = "Custom Lens Presets", style = {flexBasis = 20, flexGrow = 1}});
{ tooltip = "Customizable Lens Palette", style = {flexBasis = 20, flexGrow = 1}});
m_Presets.RegisterValueChangedCallback(OnPresetValueChanged);

ShortLabel = new Label("X") { style = { alignSelf = Align.Center, opacity = 0.5f }};
Expand Down Expand Up @@ -264,13 +265,13 @@ void OnLensPropertyChanged(SerializedProperty p)
m_Control.SetValueWithoutNotify(v);

// Sync the presets
var presets = CinemachinePhysicalLensPresets.InstanceIfExists;
var presets = CinemachinePhysicalLensPalette.InstanceIfExists;
var index = presets == null ? -1 : presets.GetMatchingPreset(new ()
{
FocalLength = v,
PhysicalProperties = ReadPhysicalSettings()
});
m_Presets.SetValueWithoutNotify(index < 0 ? string.Empty : presets.Presets[index].Name);
m_Presets.SetValueWithoutNotify(index < 0 ? k_PaletteLabel : presets.Presets[index].Name);
break;
}
case Modes.VFOV:
Expand All @@ -284,9 +285,9 @@ void OnLensPropertyChanged(SerializedProperty p)
m_Control.SetValueWithoutNotify(v);

// Sync the presets
var presets = CinemachineLensPresets.InstanceIfExists;
var presets = CinemachineLensPalette.InstanceIfExists;
var index = presets == null ? -1 : presets.GetMatchingPreset(fovProp.floatValue);
m_Presets.SetValueWithoutNotify(index < 0 ? string.Empty : presets.Presets[index].Name);
m_Presets.SetValueWithoutNotify(index < 0 ? k_PaletteLabel : presets.Presets[index].Name);
break;
}
}
Expand Down Expand Up @@ -366,30 +367,30 @@ void OnPresetValueChanged(ChangeEvent<string> evt)
if (GetLensMode() == Modes.Physical)
{
// Physical presets
var presets = CinemachinePhysicalLensPresets.Instance;
if (presets != null)
var palette = CinemachinePhysicalLensPalette.Instance;
if (palette != null)
{
// Edit the presets assets if desired
if (evt.newValue == k_EditPresetsLabel)
Selection.activeObject = presets;
Selection.activeObject = palette;
else if (evt.newValue == k_AddPresetsLabel)
{
Selection.activeObject = presets;
Undo.RecordObject(presets, "add preset");
presets.Presets.Add(new ()
Selection.activeObject = palette;
Undo.RecordObject(palette, "add palette entry");
palette.Presets.Add(new ()
{
Name = $"{m_Control.value}mm preset {presets.Presets.Count + 1}",
Name = $"{m_Control.value}mm preset {palette.Presets.Count + 1}",
FocalLength = m_Control.value,
PhysicalProperties = ReadPhysicalSettings()
});
}
else
{
// Apply the preset
var index = presets.GetPresetIndex(evt.newValue);
var index = palette.GetPresetIndex(evt.newValue);
if (index >= 0)
{
var v = presets.Presets[index];
var v = palette.Presets[index];
m_LensProperty.FindPropertyRelative(() => s_Def.FieldOfView).floatValue = FocalLengthToFov(v.FocalLength);
WritePhysicalSettings(v.PhysicalProperties);
m_LensProperty.serializedObject.ApplyModifiedProperties();
Expand All @@ -401,30 +402,30 @@ void OnPresetValueChanged(ChangeEvent<string> evt)
else
{
// Nonphysical Presets
var presets = CinemachineLensPresets.Instance;
if (presets != null)
var palette = CinemachineLensPalette.Instance;
if (palette != null)
{
var fovProp = m_LensProperty.FindPropertyRelative(() => s_Def.FieldOfView);

// Edit the presets assets if desired
if (evt.newValue == k_EditPresetsLabel)
Selection.activeObject = presets;
Selection.activeObject = palette;
else if (evt.newValue == k_AddPresetsLabel)
{
Selection.activeObject = presets;
Undo.RecordObject(presets, "add preset");
presets.Presets.Add(new ()
Selection.activeObject = palette;
Undo.RecordObject(palette, "add palette entry");
palette.Presets.Add(new ()
{
Name = $"{fovProp.floatValue} preset {presets.Presets.Count + 1}",
Name = $"{fovProp.floatValue} preset {palette.Presets.Count + 1}",
VerticalFOV = fovProp.floatValue,
});
}
else
{
// Apply the preset
var index = presets.GetPresetIndex(evt.newValue);
var index = palette.GetPresetIndex(evt.newValue);
if (index >= 0)
fovProp.floatValue = presets.Presets[index].VerticalFOV;
fovProp.floatValue = palette.Presets[index].VerticalFOV;
m_LensProperty.serializedObject.ApplyModifiedProperties();
}
}
Expand Down Expand Up @@ -471,15 +472,13 @@ void WritePhysicalSettings(in LensSettings.PhysicalSettings s)
/// IMGUI IMPLEMENTATION (to be removed)
///===========================================================================

static readonly GUIContent EditPresetsLabel = new ("Edit Presets...");
static readonly GUIContent HFOVLabel = new ("Horizontal FOV", "Horizontal Field of View");
static readonly GUIContent VFOVLabel = new ("Vertical FOV", "Vertical Field of View");
static readonly GUIContent FocalLengthLabel = new ("Focal Length", "The length of the lens (in mm)");
static readonly GUIContent OrthoSizeLabel = new ("Ortho Size", "When using an orthographic camera, "
+ "this defines the half-height, in world coordinates, of the camera view.");
static readonly GUIContent s_EmptyContent = new (" ");
static readonly GUIContent AdvancedLabel = new ("Advanced");
static readonly string AdvancedHelpboxMessage = "Lens Mode Override must be enabled in the CM Brain for Mode Override to take effect";
static readonly string AdvancedHelpboxMessage = "Lens Mode Override must be enabled in the Cinemachine Brain for Mode Override to take effect";

static bool s_AdvancedLensExpanded;

Expand All @@ -493,7 +492,6 @@ struct Snapshot
Snapshot m_Snapshot;

const float vSpace= 2;
const float hSpace = 2;

void SnapshotCameraShadowValues(SerializedProperty property)
{
Expand Down
16 changes: 8 additions & 8 deletions .../Editor/Utility/CinemachineLensPresets.cs → .../Editor/Utility/CinemachineLensPalette.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,41 +10,41 @@ namespace Unity.Cinemachine.Editor
/// User-definable named presets for lenses. This is a Singleton asset, available in editor only
/// </summary>
[Serializable]
public sealed class CinemachineLensPresets : ScriptableObject
public sealed class CinemachineLensPalette : ScriptableObject
{
static CinemachineLensPresets s_Instance = null;
static CinemachineLensPalette s_Instance = null;
static bool s_AlreadySearched = false;

/// <summary>Get the singleton instance of this object, or null if it doesn't exist</summary>
public static CinemachineLensPresets InstanceIfExists
public static CinemachineLensPalette InstanceIfExists
{
get
{
if (!s_AlreadySearched)
{
s_AlreadySearched = true;
var guids = AssetDatabase.FindAssets("t:CinemachineLensPresets");
var guids = AssetDatabase.FindAssets("t:CinemachineLensPalette");
for (int i = 0; i < guids.Length && s_Instance == null; ++i)
s_Instance = AssetDatabase.LoadAssetAtPath<CinemachineLensPresets>(
s_Instance = AssetDatabase.LoadAssetAtPath<CinemachineLensPalette>(
AssetDatabase.GUIDToAssetPath(guids[i]));
}
return s_Instance;
}
}

/// <summary>Get the singleton instance of this object. Creates asset if nonexistent</summary>
public static CinemachineLensPresets Instance
public static CinemachineLensPalette Instance
{
get
{
if (InstanceIfExists == null)
{
var newAssetPath = EditorUtility.SaveFilePanelInProject(
"Create Lens Presets asset", "CinemachineLensPresets", "asset",
"Create Lens Palette asset", "CinemachineLensPalette", "asset",
"This editor-only file will contain the lens presets for this project");
if (!string.IsNullOrEmpty(newAssetPath))
{
s_Instance = CreateInstance<CinemachineLensPresets>();
s_Instance = CreateInstance<CinemachineLensPalette>();
// Create some sample presets
s_Instance.Presets = new()
{
Expand Down
0 ...or/Utility/CinemachineLensPresets.cs.meta → ...or/Utility/CinemachineLensPalette.cs.meta
View file
File renamed without changes.
20 changes: 10 additions & 10 deletions ...Utility/CinemachinePhysicalLensPresets.cs → ...Utility/CinemachinePhysicalLensPalette.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,44 +7,44 @@
namespace Unity.Cinemachine.Editor
{
/// <summary>
/// User-definable named presets for lenses. This is a Singleton asset, available in editor only
/// User-definable named presets for physical lenses. This is a Singleton asset, available in editor only
/// </summary>
[Serializable]
public sealed class CinemachinePhysicalLensPresets : ScriptableObject
public sealed class CinemachinePhysicalLensPalette : ScriptableObject
{
static CinemachinePhysicalLensPresets s_Instance = null;
static CinemachinePhysicalLensPalette s_Instance = null;
static bool s_AlreadySearched = false;

/// <summary>Get the singleton instance of this object, or null if it doesn't exist</summary>
public static CinemachinePhysicalLensPresets InstanceIfExists
public static CinemachinePhysicalLensPalette InstanceIfExists
{
get
{
if (!s_AlreadySearched)
{
s_AlreadySearched = true;
var guids = AssetDatabase.FindAssets("t:CinemachinePhysicalLensPresets");
var guids = AssetDatabase.FindAssets("t:CinemachinePhysicalLensPalette");
for (int i = 0; i < guids.Length && s_Instance == null; ++i)
s_Instance = AssetDatabase.LoadAssetAtPath<CinemachinePhysicalLensPresets>(
s_Instance = AssetDatabase.LoadAssetAtPath<CinemachinePhysicalLensPalette>(
AssetDatabase.GUIDToAssetPath(guids[i]));
}
return s_Instance;
}
}

/// <summary>Get the singleton instance of this object. Creates asset if nonexistent</summary>
public static CinemachinePhysicalLensPresets Instance
public static CinemachinePhysicalLensPalette Instance
{
get
{
if (InstanceIfExists == null)
{
var newAssetPath = EditorUtility.SaveFilePanelInProject(
"Create Lens Presets asset", "CinemachinePhysicalLensPresets", "asset",
"This editor-only file will contain the lens presets for this project");
"Create Physical Lens Palette asset", "CinemachinePhysicalLensPalette", "asset",
"This editor-only file will contain the physical lens presets for this project");
if (!string.IsNullOrEmpty(newAssetPath))
{
s_Instance = CreateInstance<CinemachinePhysicalLensPresets>();
s_Instance = CreateInstance<CinemachinePhysicalLensPalette>();

// Create some sample presets
var defaultPhysical = LensSettings.Default.PhysicalProperties;
Expand Down
0 ...ty/CinemachinePhysicalLensPresets.cs.meta → ...ty/CinemachinePhysicalLensPalette.cs.meta
View file
File renamed without changes.
Loading

0 comments on commit 8ed285a

Please sign in to comment.