Skip to content

Conversation

@DryreL
Copy link

@DryreL DryreL commented Jan 3, 2026

Changes

  • Fixed LayoutUtility for Unity 6 WindowLayout API changes
  • Updated GameViewUtils to remove NET_4_6 directive
  • Fixed GUILayout Begin/End mismatch in VideoEditorWelcomeWindow
  • Added MP4 resolution validation (even dimensions required)
  • Added dark/light mode text color support for all Editor scripts

Testing

  • Editor Play button successfully worked without any issues
  • Verified Welcome Window displays correctly in both themes
  • All Editor drawers now support light/dark modes

Copilot AI review requested due to automatic review settings January 3, 2026 12:23
@DryreL DryreL mentioned this pull request Jan 3, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds Unity 6 compatibility support for a Video Editor Template by addressing API changes in Unity 6 and adding dark/light mode theme support for editor UI components.

  • Updated WindowLayout API reflection to handle Unity 6's namespace changes with fallback support
  • Removed NET_4_6 preprocessor directives in favor of runtime constructor detection
  • Added MP4 format validation requiring even dimensions for H.264 codec compatibility
  • Implemented dark/light mode text color support across all custom editor scripts

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
VideoEditorWelcomeWindow.cs Fixed GUILayout Begin/End mismatch, added MP4 dimension validation with user warning, corrected YouTube link bug, and implemented theme-aware text colors
LayoutUtility.cs Updated WindowLayout type reflection for Unity 6 CoreModule with fallback, added null safety checks for method invocation
GameViewUtils.cs Removed NET_4_6 directive, replaced with runtime constructor detection for Unity 6 compatibility, added MakeEven helper for MP4 dimension validation
VideoScriptPlayableAssetEditor.cs Added dark/light mode text color support with runtime theme detection
TransformTweenDrawer.cs Added GUI.color modifications for light mode label visibility
TimeDilationDrawer.cs Added GUI.color modifications for light mode label visibility
TextSwitcherDrawer.cs Added GUI.color modifications for light mode label visibility
ScreenFaderDrawer.cs Added GUI.color modifications for light mode label visibility
LightControlDrawer.cs Added GUI.color modifications for light mode label visibility

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +24 to +28
Color originalLabelColor = GUI.color;
if (!EditorGUIUtility.isProSkin)
{
GUI.color = new Color(0, 0, 0, 1); // Black for light mode
}
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using GUI.color to control label text color is incorrect. GUI.color is a tint that affects all GUI elements including backgrounds, borders, and textures. For controlling text color specifically in PropertyDrawers, you should use the GUIStyle.normal.textColor property or leave the default styling which Unity handles automatically. This implementation may cause unintended visual effects on other GUI elements.

Copilot uses AI. Check for mistakes.
Comment on lines +15 to +27
// Support dark/light mode for label text
Color originalLabelColor = GUI.color;
if (!EditorGUIUtility.isProSkin)
{
GUI.color = new Color(0, 0, 0, 1); // Black for light mode
}

SerializedProperty timeScaleProp = property.FindPropertyRelative("timeScale");

Rect singleFieldRect = new Rect(position.x, position.y, position.width, EditorGUIUtility.singleLineHeight);
EditorGUI.PropertyField(singleFieldRect, timeScaleProp);

GUI.color = originalLabelColor;
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using GUI.color to control label text color is incorrect. GUI.color is a tint that affects all GUI elements including backgrounds, borders, and textures. For controlling text color specifically in PropertyDrawers, you should use the GUIStyle.normal.textColor property or leave the default styling which Unity handles automatically. This implementation may cause unintended visual effects on other GUI elements.

Suggested change
// Support dark/light mode for label text
Color originalLabelColor = GUI.color;
if (!EditorGUIUtility.isProSkin)
{
GUI.color = new Color(0, 0, 0, 1); // Black for light mode
}
SerializedProperty timeScaleProp = property.FindPropertyRelative("timeScale");
Rect singleFieldRect = new Rect(position.x, position.y, position.width, EditorGUIUtility.singleLineHeight);
EditorGUI.PropertyField(singleFieldRect, timeScaleProp);
GUI.color = originalLabelColor;
SerializedProperty timeScaleProp = property.FindPropertyRelative("timeScale");
Rect singleFieldRect = new Rect(position.x, position.y, position.width, EditorGUIUtility.singleLineHeight);
EditorGUI.PropertyField(singleFieldRect, timeScaleProp);

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +22
Color originalLabelColor = GUI.color;
if (!EditorGUIUtility.isProSkin)
{
GUI.color = new Color(0, 0, 0, 1); // Black for light mode
}
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using GUI.color to control label text color is incorrect. GUI.color is a tint that affects all GUI elements including backgrounds, borders, and textures. For controlling text color specifically in PropertyDrawers, you should use the GUIStyle.normal.textColor property or leave the default styling which Unity handles automatically. This implementation may cause unintended visual effects on other GUI elements.

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +29
// Support dark/light mode for label text
Color originalLabelColor = GUI.color;
if (!EditorGUIUtility.isProSkin)
{
GUI.color = new Color(0, 0, 0, 1); // Black for light mode
}

SerializedProperty colorProp = property.FindPropertyRelative("color");

Rect singleFieldRect = new Rect(position.x, position.y, position.width, EditorGUIUtility.singleLineHeight);
EditorGUI.PropertyField(singleFieldRect, colorProp);

GUI.color = originalLabelColor;
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using GUI.color to control label text color is incorrect. GUI.color is a tint that affects all GUI elements including backgrounds, borders, and textures. For controlling text color specifically in PropertyDrawers, you should use the GUIStyle.normal.textColor property or leave the default styling which Unity handles automatically. This implementation may cause unintended visual effects on other GUI elements.

Suggested change
// Support dark/light mode for label text
Color originalLabelColor = GUI.color;
if (!EditorGUIUtility.isProSkin)
{
GUI.color = new Color(0, 0, 0, 1); // Black for light mode
}
SerializedProperty colorProp = property.FindPropertyRelative("color");
Rect singleFieldRect = new Rect(position.x, position.y, position.width, EditorGUIUtility.singleLineHeight);
EditorGUI.PropertyField(singleFieldRect, colorProp);
GUI.color = originalLabelColor;
SerializedProperty colorProp = property.FindPropertyRelative("color");
Rect singleFieldRect = new Rect(position.x, position.y, position.width, EditorGUIUtility.singleLineHeight);
EditorGUI.PropertyField(singleFieldRect, colorProp);

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +21
Color originalLabelColor = GUI.color;
if (!EditorGUIUtility.isProSkin)
{
GUI.color = new Color(0, 0, 0, 1); // Black for light mode
}
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using GUI.color to control label text color is incorrect. GUI.color is a tint that affects all GUI elements including backgrounds, borders, and textures. For controlling text color specifically in PropertyDrawers, you should use the GUIStyle.normal.textColor property or leave the default styling which Unity handles automatically. This implementation may cause unintended visual effects on other GUI elements.

Copilot uses AI. Check for mistakes.
Comment on lines +68 to +78
// MP4 format requires even dimensions
int adjustedWidth = (width % 2 == 0) ? width : width + 1;
int adjustedHeight = (height % 2 == 0) ? height : height + 1;

if (adjustedWidth != width || adjustedHeight != height)
{
width = adjustedWidth;
height = adjustedHeight;
UnityEngine.Debug.LogWarning($"Resolution adjusted to even dimensions for MP4 compatibility: {width}x{height}");
}

Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MP4 resolution validation is being performed twice in different locations (here and in GameViewUtils.AddSetSize). This creates inconsistent behavior: users might see a warning about adjusted dimensions from this code, but GameViewUtils.AddSetSize will adjust again silently. The validation should be performed in only one location to avoid confusion and ensure consistent behavior throughout the application.

Suggested change
// MP4 format requires even dimensions
int adjustedWidth = (width % 2 == 0) ? width : width + 1;
int adjustedHeight = (height % 2 == 0) ? height : height + 1;
if (adjustedWidth != width || adjustedHeight != height)
{
width = adjustedWidth;
height = adjustedHeight;
UnityEngine.Debug.LogWarning($"Resolution adjusted to even dimensions for MP4 compatibility: {width}x{height}");
}

Copilot uses AI. Check for mistakes.
{
newGvsType = typeof(Editor).Assembly.GetType("UnityEditor.GameViewSizeType.FixedResolution");
// Fallback for older Unity versions with int parameter
ctor = gvsType.GetConstructor(new Type[] { typeof(int), typeof(int), typeof(int), typeof(string) });
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback branch may fail silently if the constructor is not found in older Unity versions. After the fallback constructor lookup, there's no null check before invoking it. If both constructor lookups fail, this will throw a NullReferenceException. Add a null check and appropriate error handling after the fallback constructor lookup.

Suggested change
ctor = gvsType.GetConstructor(new Type[] { typeof(int), typeof(int), typeof(int), typeof(string) });
ctor = gvsType.GetConstructor(new Type[] { typeof(int), typeof(int), typeof(int), typeof(string) });
if (ctor == null)
{
Debug.LogError("GameViewUtils.AddCustomSize: Unable to find a suitable UnityEditor.GameViewSize constructor for this Unity version.");
return;
}

Copilot uses AI. Check for mistakes.
Comment on lines +27 to 32
// Ensure label color is set for dark/light mode compatibility
if (lStyle.normal.textColor == Color.black)
{
lStyle.normal.textColor = EditorGUIUtility.isProSkin ? Color.white : Color.black;
}

Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition will always be false in light mode. The GetLabelStyle() method already sets the textColor based on the editor skin (line 89), so checking if it equals Color.black and then setting it again is redundant and won't work as expected. This check should be removed since the text color is already properly set in GetLabelStyle().

Suggested change
// Ensure label color is set for dark/light mode compatibility
if (lStyle.normal.textColor == Color.black)
{
lStyle.normal.textColor = EditorGUIUtility.isProSkin ? Color.white : Color.black;
}

Copilot uses AI. Check for mistakes.
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.

1 participant