diff --git a/CHANGELOG.md b/CHANGELOG.md index c90300f..c366dc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All package updates & migration steps will be listed in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [4.3.0] - 2024-11-22 +### Added +- `Warning` and `Error` severity levels for `ValidatorError` +- Updated UI to display and filter warnings & errors. +- Some missing coverage. +- Built-in warnings for slow validation execution. +### Changed +- Only mark data generation as an error for `ValidaitonError` with severity error. +- Missing validators are considered warnings. This reduces the noisiness of compilation errors causing this. +- Do not force open the Validation window for warnings. + ## [4.2.1] - 2024-11-19 ### Fixed - Issue where nested structs in scriptable object drawers have glitching UI. diff --git a/Editor/Common/Util/ParameterDebug.cs b/Editor/Common/Util/ParameterDebug.cs index 9a7dc18..b2f0d60 100644 --- a/Editor/Common/Util/ParameterDebug.cs +++ b/Editor/Common/Util/ParameterDebug.cs @@ -22,6 +22,12 @@ public static void LogVerbose(string log) /// String to log public static void LogError(string log) => Debug.LogError(log); + /// + /// Warning logs to the console. + /// + /// String to log + public static void LogWarning(string log) => Debug.LogWarning(log); + /// /// Logs to the console. /// diff --git a/Editor/DataGeneration/Operation/DataOperationContext.cs b/Editor/DataGeneration/Operation/DataOperationContext.cs index 592ead1..be1f4e3 100644 --- a/Editor/DataGeneration/Operation/DataOperationContext.cs +++ b/Editor/DataGeneration/Operation/DataOperationContext.cs @@ -3,6 +3,7 @@ using PocketGems.Parameters.Common.Models.Editor; using PocketGems.Parameters.Common.Operation.Editor; using PocketGems.Parameters.DataGeneration.LocalCSV.Editor; +using PocketGems.Parameters.Validation; namespace PocketGems.Parameters.DataGeneration.Operation.Editor { @@ -14,6 +15,7 @@ public DataOperationContext() InfoCSVFileCache = new InfoCSVFileCache(GeneratedLocalCSVDirectory, true); StructCSVFileCache = new StructCSVFileCache(GeneratedLocalCSVDirectory, true); ScriptableObjectMetadatas = new Dictionary>(); + AllValidationErrors = new(); } public GenerateDataType GenerateDataType { get; set; } @@ -36,5 +38,7 @@ public DataOperationContext() #endif public Dictionary> ScriptableObjectMetadatas { get; } public List GeneratedFilePaths { get; } + + public List AllValidationErrors { get; } } } diff --git a/Editor/DataGeneration/Operation/IDataOperationContext.cs b/Editor/DataGeneration/Operation/IDataOperationContext.cs index a393f12..36a9247 100644 --- a/Editor/DataGeneration/Operation/IDataOperationContext.cs +++ b/Editor/DataGeneration/Operation/IDataOperationContext.cs @@ -2,6 +2,7 @@ using PocketGems.Parameters.Common.Models.Editor; using PocketGems.Parameters.Common.Operation.Editor; using PocketGems.Parameters.DataGeneration.LocalCSV.Editor; +using PocketGems.Parameters.Validation; namespace PocketGems.Parameters.DataGeneration.Operation.Editor { @@ -41,5 +42,10 @@ public interface IDataOperationContext : ICommonOperationContext /// Asset files that were generated through this data generation /// List GeneratedFilePaths { get; } + + /// + /// Validation errors & warnings + /// + List AllValidationErrors { get; } } } diff --git a/Editor/DataGeneration/Operations/DataValidationOperation.cs b/Editor/DataGeneration/Operations/DataValidationOperation.cs index 80fff4e..d948ca0 100644 --- a/Editor/DataGeneration/Operations/DataValidationOperation.cs +++ b/Editor/DataGeneration/Operations/DataValidationOperation.cs @@ -25,13 +25,23 @@ public override void Execute(IDataOperationContext context) // validate assets var assetErrors = AssetValidator.ValidateScriptableObjects(context.ScriptableObjectMetadatas); for (int i = 0; i < assetErrors?.Count; i++) - Error(assetErrors[i]); + { + var validationError = assetErrors[i]; + context.AllValidationErrors.Add(validationError); + if (validationError.ErrorSeverity == ValidationError.Severity.Error) + Error(assetErrors[i]); + } // validate parameters IParameterManager parameterManager = EditorParams.ParameterManager; var parameterErrors = InvokeParamsValidation(context, parameterManager); for (int i = 0; i < parameterErrors?.Count; i++) - Error(parameterErrors[i]); + { + var validationError = parameterErrors[i]; + context.AllValidationErrors.Add(validationError); + if (validationError.ErrorSeverity == ValidationError.Severity.Error) + Error(parameterErrors[i]); + } } /// diff --git a/Editor/Editor/ParameterScriptableObjectInspector.cs b/Editor/Editor/ParameterScriptableObjectInspector.cs index f6182e6..2631d48 100644 --- a/Editor/Editor/ParameterScriptableObjectInspector.cs +++ b/Editor/Editor/ParameterScriptableObjectInspector.cs @@ -85,17 +85,38 @@ protected void DrawParameterToolGUI() EditorGUILayout.Space(); } + #region hack for backwards compatibility + + private static string warningMessageHolder = null; + + [Obsolete] + private void InternalDrawProperty(SerializedProperty property, string errorMessage = null, string warningMessage = null) + { + warningMessageHolder = warningMessage; + DrawProperty(property, errorMessage); + } + + [Obsolete] + protected virtual void DrawProperty(SerializedProperty property, string errorMessage = null) + { + DrawProperty(property, errorMessage, warningMessageHolder); + } + + #endregion + /// /// Allow for subclasses to draw custom visualizers for the property /// /// property to render /// error message to show if any - protected virtual void DrawProperty(SerializedProperty property, string errorMessage = null) + protected virtual void DrawProperty(SerializedProperty property, string errorMessage = null, string warningMessage = null) { var propLabel = new GUIContent(property.displayName); EditorGUILayout.PropertyField(property, propLabel, true); if (errorMessage != null) EditorGUILayout.HelpBox(errorMessage, MessageType.Error); + if (warningMessage != null) + EditorGUILayout.HelpBox(warningMessage, MessageType.Warning); } public override void OnInspectorGUI() @@ -147,8 +168,8 @@ public override void OnInspectorGUI() errors.Add(error); } - // collect properties & errors - List<(SerializedProperty, string)> properties = new(); + // collect properties, errors & warnings + List<(SerializedProperty, string, string)> properties = new(); // call to ensure the values are up to date with the target in case it was modified in another inspector/drawer serializedObject.Update(); var serializedProp = serializedObject.GetIterator(); @@ -159,17 +180,28 @@ public override void OnInspectorGUI() // don't draw class file if (serializedProp.name == "m_Script") continue; var interfacePropertyGetterName = GetPropertyGetterName(serializedProp); - string errorMessage = null; + StringBuilder errorMessage = null; + StringBuilder warningMessage = null; if (_propertyToError.ContainsKey(interfacePropertyGetterName)) { - StringBuilder s = new StringBuilder(); - s.Append($"{interfacePropertyGetterName} Validation Error(s):"); var propertyErrors = _propertyToError[interfacePropertyGetterName]; _propertyToError.Remove(interfacePropertyGetterName); for (int i = 0; i < propertyErrors.Count; i++) { + StringBuilder s; var error = propertyErrors[i]; - s.AppendLine(); + if (error.ErrorSeverity == ValidationError.Severity.Error) + { + errorMessage ??= new(); + s = errorMessage; + } + else + { + warningMessage ??= new(); + s = warningMessage; + } + if (s.Length != 0) + s.AppendLine(); s.Append(" "); if (!string.IsNullOrEmpty(error.StructKeyPath)) { @@ -190,21 +222,32 @@ public override void OnInspectorGUI() } s.Append($"{error.Message}"); } - - errorMessage = s.ToString(); } - properties.Add((serializedProp.Copy(), errorMessage)); + properties.Add((serializedProp.Copy(), errorMessage?.ToString(), warningMessage?.ToString())); } // display missing maps (shouldn't ever happen but just in case...) if (_propertyToError.Count != 0) { - StringBuilder s = new StringBuilder("Unexpected Errors:"); + StringBuilder errorMessage = null; + StringBuilder warningMessage = null; foreach (var kvp in _propertyToError) { var errors = kvp.Value; for (int i = 0; i < errors.Count; i++) { + StringBuilder s; + var error = errors[i]; + if (error.ErrorSeverity == ValidationError.Severity.Error) + { + errorMessage ??= new("Unexpected Error(s):"); + s = errorMessage; + } + else + { + warningMessage ??= new("Unexpected Warning(s):"); + s = warningMessage; + } s.AppendLine(); if (!string.IsNullOrEmpty(errors[i].InfoProperty)) s.Append($"{errors[i].InfoProperty}: "); @@ -212,7 +255,10 @@ public override void OnInspectorGUI() } } EditorGUILayout.Space(); - EditorGUILayout.HelpBox(s.ToString(), MessageType.Error); + if (errorMessage != null) + EditorGUILayout.HelpBox(errorMessage.ToString(), MessageType.Error); + if (warningMessage != null) + EditorGUILayout.HelpBox(warningMessage.ToString(), MessageType.Warning); EditorGUILayout.Space(); } @@ -227,6 +273,7 @@ public override void OnInspectorGUI() { serializedProp = properties[i].Item1; var errorMessage = properties[i].Item2; + var warningMessage = properties[i].Item3; // don't draw class file if (serializedProp.name == "m_Script") continue; @@ -255,7 +302,7 @@ public override void OnInspectorGUI() // draw property if (foldOutAttribute == null || isFoldoutOpen) - DrawProperty(serializedProp, errorMessage); + InternalDrawProperty(serializedProp, errorMessage, warningMessage); } scope?.Dispose(); diff --git a/Editor/Editor/SerializableDateTimeDrawer.cs b/Editor/Editor/SerializableDateTimeDrawer.cs index 1fa2e7c..9013f56 100644 --- a/Editor/Editor/SerializableDateTimeDrawer.cs +++ b/Editor/Editor/SerializableDateTimeDrawer.cs @@ -58,7 +58,7 @@ int DrawComponent(string componentLabel, int componentDigits, int componentValue var second = DrawComponent("s", 2, dateTime.Second); dateTime = new DateTime(year, month, day, hour, minute, second); } - catch (Exception _) + catch { // ignored } diff --git a/Editor/Editor/SerializableTimeSpanDrawer.cs b/Editor/Editor/SerializableTimeSpanDrawer.cs index c07b43e..9b63d33 100644 --- a/Editor/Editor/SerializableTimeSpanDrawer.cs +++ b/Editor/Editor/SerializableTimeSpanDrawer.cs @@ -57,7 +57,7 @@ float DrawComponent(string componentLabel, int componentDigits, int componentVal newTimeSpan += TimeSpan.FromSeconds(DrawComponent("s", 2, timeSpan.Seconds)); newTimeSpan += TimeSpan.FromMilliseconds(DrawComponent("ms", 3, timeSpan.Milliseconds)); } - catch (Exception e) + catch { // ignored } diff --git a/Editor/Editor/Validation/ValidationTreeView.cs b/Editor/Editor/Validation/ValidationTreeView.cs index 20f5c51..f11de10 100644 --- a/Editor/Editor/Validation/ValidationTreeView.cs +++ b/Editor/Editor/Validation/ValidationTreeView.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using PocketGems.Parameters.Editor.Validation.TreeDataModel.Editor; +using PocketGems.Parameters.Validation; using UnityEditor; using UnityEditor.IMGUI.Controls; using UnityEngine; @@ -134,7 +135,12 @@ void CellGUI(Rect cellRect, TreeViewItem item, Columns co case Columns.Icon: { if (!string.IsNullOrEmpty(item.data.ValidationError?.Message)) - GUI.DrawTexture(cellRect, _errorIcon, ScaleMode.ScaleToFit); + { + var icon = _errorIcon; + if (item.data.ValidationError.ErrorSeverity == ValidationError.Severity.Warning) + icon = _warningIcon; + GUI.DrawTexture(cellRect, icon, ScaleMode.ScaleToFit); + } } break; case Columns.PropertyName: diff --git a/Editor/Editor/Validation/ValidationWindow.cs b/Editor/Editor/Validation/ValidationWindow.cs index c1cd20c..74e61ef 100644 --- a/Editor/Editor/Validation/ValidationWindow.cs +++ b/Editor/Editor/Validation/ValidationWindow.cs @@ -33,6 +33,8 @@ private enum ValidationState // models [NonSerialized] private IReadOnlyList _errors; + [NonSerialized] private int _errorCount; + [NonSerialized] private int _warningCount; [NonSerialized] private ValidationState _validationState = ValidationState.NoData; [NonSerialized] private long _executionMillis; @@ -43,6 +45,8 @@ private enum ValidationState [SerializeField] MultiColumnHeaderState _multiColumnHeaderState; SearchField _searchField; ValidationTreeView _treeView; + private bool _showErrors = true; + private bool _showWarnings = true; // red flickering notifier private bool _disableNextFlicker; @@ -51,6 +55,11 @@ private enum ValidationState private Stopwatch _flickerStopwatch = new Stopwatch(); private const float s_flickerDuration = 0.25f; + // view + private Texture2D _errorIcon; + private Texture2D _warningIcon; + private Texture2D _infoIcon; + [MenuItem(MenuItemConstants.ValidationWindowPath, false, MenuItemConstants.ValidationWindowPriority)] public static void OpenWindow() { @@ -72,6 +81,18 @@ internal void UpdateValidationResult(IReadOnlyList errors, long { _isDirty = true; _errors = errors; + _errorCount = 0; + _warningCount = 0; + if (_errors?.Count > 0) + { + foreach (var error in _errors) + { + if (error.ErrorSeverity == ValidationError.Severity.Error) + _errorCount++; + else + _warningCount++; + } + } _executionMillis = executionMillis; _validationState = errors?.Count > 0 ? ValidationState.NoErrors @@ -113,6 +134,8 @@ private void ClearValidationResults() ValidationWindowSerializer.SerializeToStorage(null); _isDirty = true; _errors = null; + _errorCount = 0; + _warningCount = 0; _validationState = ValidationState.NoData; } @@ -143,6 +166,11 @@ private void InitIfNeeded() _searchField = new SearchField(); _searchField.downOrUpArrowKeyPressed += _treeView.SetFocusAndEnsureSelectedItem; + // found texture examples in AssetSettingsAnalyzeTreeView + _errorIcon = EditorGUIUtility.FindTexture("console.errorIcon"); + _warningIcon = EditorGUIUtility.FindTexture("console.warnicon"); + _infoIcon = EditorGUIUtility.FindTexture("console.infoIcon"); + _isInit = true; } } @@ -214,9 +242,38 @@ private IList CreateTreeElements() var rootElement = new ValidationTreeElement(ValidationTreeElement.TreeElementType.Root, "root", idCount++); elements.Add(rootElement); + List filteredErrors = null; if (_errors != null && _errors.Count > 0) { - List sortedErrors = new List(_errors); + foreach (var error in _errors) + { + if ((_showErrors && error.ErrorSeverity == ValidationError.Severity.Error) || + (_showWarnings && error.ErrorSeverity == ValidationError.Severity.Warning)) + { + if (filteredErrors == null) + filteredErrors = new(); + filteredErrors.Add(error); + } + } + } + + if (!_showWarnings && _warningCount > 0) + { + string status = _warningCount == 1 ? "1 warning is hidden." : $"{_warningCount} warnings are hidden."; + var parentElement = new ValidationTreeElement(ValidationTreeElement.TreeElementType.General, status, idCount++); + elements.Add(parentElement); + } + + if (!_showErrors && _errorCount > 0) + { + string status = _errorCount == 1 ? "1 error is hidden." : $"{_errorCount} errors are hidden."; + var parentElement = new ValidationTreeElement(ValidationTreeElement.TreeElementType.General, status, idCount++); + elements.Add(parentElement); + } + + if (filteredErrors != null && filteredErrors.Count > 0) + { + List sortedErrors = new(filteredErrors); int ErrorCompare(ValidationError a, ValidationError b) { if (a.InfoType != b.InfoType) @@ -273,7 +330,8 @@ int ErrorCompare(ValidationError a, ValidationError b) elements.Add(element); } } - else + + if (_errors == null || _errors.Count == 0) { string status = _validationState == ValidationState.NoData ? "No Validation Results" : "No Errors"; var parentElement = new ValidationTreeElement(ValidationTreeElement.TreeElementType.General, status, idCount++); @@ -324,7 +382,34 @@ private void ToolBar(Rect rect) { ClearValidationResults(); } - var searchRect = new Rect(rect.x + 290, rect.y, rect.width - 290, rect.height); + + // show error & warning toggle on the very right + var errorButton = new GUIContent(_errorCount.ToString(), _errorIcon); + Vector2 errorButtonSize = EditorStyles.toolbarButton.CalcSize(errorButton); + var errorButtonRect = new Rect( + rect.x + rect.width - errorButtonSize.x, + rect.y, + errorButtonSize.x, + errorButtonSize.y); + var newShowErrors = GUI.Toggle(errorButtonRect, _showErrors, errorButton, EditorStyles.toolbarButton); + _isDirty |= newShowErrors != _showErrors; + _showErrors = newShowErrors; + + var warningButton = new GUIContent(_warningCount.ToString(), _warningIcon); + Vector2 warningButtonSize = EditorStyles.toolbarButton.CalcSize(warningButton); + var warningButtonRect = new Rect( + rect.x + rect.width - warningButtonSize.x - errorButtonSize.x, + rect.y, + warningButtonSize.x, + warningButtonSize.y); + var newShowWarnings = GUI.Toggle(warningButtonRect, _showWarnings, warningButton, EditorStyles.toolbarButton); + _isDirty |= newShowWarnings != _showWarnings; + _showWarnings = newShowWarnings; + var searchRect = new Rect( + rect.x + 290, + rect.y, + rect.width - 290 - errorButtonSize.x - warningButtonSize.x - 5, + rect.height); _treeView.searchString = _searchField.OnGUI(searchRect, _treeView.searchString); } @@ -362,8 +447,6 @@ private void BottomToolBar(Rect rect) { _treeView.CollapseAll(); } - if (_validationState != ValidationState.NoData) - GUILayout.Label($"Total Errors: {_errors?.Count ?? 0}"); GUILayout.FlexibleSpace(); if (_validationState != ValidationState.NoData && _executionMillis != 0) GUILayout.Label($"Validation Execution Duration: {_executionMillis}ms"); diff --git a/Editor/EditorParameterDataManager.cs b/Editor/EditorParameterDataManager.cs index 73a4cac..ca170d4 100644 --- a/Editor/EditorParameterDataManager.cs +++ b/Editor/EditorParameterDataManager.cs @@ -452,6 +452,7 @@ public static bool GenerateData(GenerateDataType generateType, * output/display results *************************/ DisplayExecutionResults(executor); + DisplayValidationResults(executor, context); s_runningGenerateData = false; ParameterDebug.LogVerbose($"{nameof(GenerateData)} duration: {generateDataDuration}ms"); @@ -536,32 +537,55 @@ private static void DisplayExecutionResults(OperationExecutor executor) $"There are {errorMessages.Count} parameter errors. See console for error logs.", "Okay"); - if (Application.isBatchMode) { for (int i = 0; i < validationErrors.Count; i++) ParameterDebug.LogError(validationErrors[i].ToString()); } - else if (validationErrors.Count > 0) + } + } + + private static void DisplayValidationResults(OperationExecutor executor, IDataOperationContext context) + { + if (executor.ExecutorState == ExecutorState.Error) + { + bool hasErrorSeverity = false; + foreach (var validationError in context.AllValidationErrors) + { + if (validationError.ErrorSeverity == ValidationError.Severity.Error) + { + hasErrorSeverity = true; + break; + } + } + + if (hasErrorSeverity) { ParameterDebug.LogError("Parameter errors: see Parameter Validation window"); - ValidationWindow.SerializeToStorage(validationErrors); + // only force open the window on validation errors not warnings + ValidationWindow.SerializeToStorage(context.AllValidationErrors); var window = ValidationWindow.GetWindow(true); - window.UpdateValidationResult(validationErrors, executor.ExecuteMilliseconds); + window.UpdateValidationResult(context.AllValidationErrors, executor.ExecuteMilliseconds); } } if (executor.ExecutorState == ExecutorState.Finished) { - if (typeof(T) == typeof(IDataOperationContext) && !executor.ShortCircuited && - ParameterPrefs.AutoValidateDataOnAssetChange) + if (!executor.ShortCircuited && ParameterPrefs.AutoValidateDataOnAssetChange) { - ValidationWindow.SerializeToStorage(null); + if (context.AllValidationErrors.Count > 0) + { + ParameterDebug.LogWarning("Parameter warnings: see Parameter Validation window"); + } + + // update the window with the latest results if it happens to be open + // if it got this far, the only errors there may have are warnings + ValidationWindow.SerializeToStorage(context.AllValidationErrors); if (ValidationWindow.HasOpenInstance()) { var window = ValidationWindow.GetWindow(false); - window.UpdateValidationResult(null, executor.ExecuteMilliseconds); + window.UpdateValidationResult(context.AllValidationErrors, executor.ExecuteMilliseconds); } } } diff --git a/README/DataValidation.md b/README/DataValidation.md index 0750601..1d8c092 100644 --- a/README/DataValidation.md +++ b/README/DataValidation.md @@ -116,21 +116,22 @@ namespace Parameters.Validation ### ValidateInfo `ValidateInfo` will be called for every row of data that conforms to `ICurrencyInfo`. More complex checks that cannot be enforced via attributes can be written here. -Upon finding an error `Error` should be called with the `nameof()` the property and the error message. The system already knows the `Identifier` of the `info` so there is no need to incooporate that as part of the error message. +Upon finding an error or warning `Error` or `Warn` should be called respectively with the `nameof()` the property and the error message. The system already knows the `Identifier` of the `info` so there is no need to incorporate that as part of the error message. ```C# protected override void ValidateInfo(IParameterManager parameterManager, ICurrencyInfo info) { - if (info.IsPremiumCurrency && string.IsNullOrEmpty(info.PremiumCurrencyIconSprite.AssetGUID)) - Error(nameof(ICurrencyInfo.PremiumCurrencyIconSprite), "required for premium currency"); + if (info.IsPremiumCurrency && string.IsNullOrEmpty(info.PremiumCurrencyIconSprite.AssetGUID)) + Error(nameof(ICurrencyInfo.PremiumCurrencyIconSprite), "required for premium currency"); - // add other checks here + if (info.MaxCurrency < 100) + Warn(nameof(ICurrencyInfo.MaxCurrency), "might be too low of a max"); } ``` ### ValidateParameters ValidateParameters will be called once. This can be used for more holistic checks. -Upon error, `Error` can be called with the error message. +Upon error or warning, `Error` or `Warn` can be called respectively with a message. ```C# protected override void ValidateParameters(IParameterManager parameterManager) { @@ -142,7 +143,8 @@ protected override void ValidateParameters(IParameterManager parameterManager) else seenColors.Add(info.CurrencyColor); } - // add other checks here + if (parameterManager.Get().ToList().Count > 1000) + Warn("There may be too many currencies in the game consider reducing"); } ``` @@ -155,9 +157,7 @@ In this validation, the code doesn't know about the context of where the data is ```C# protected override void ValidateStruct(IParameterManager parameterManager, IRewardStruct structObj) { - if (structObj.IsPremiumCurrency && string.IsNullOrEmpty(structObj.PremiumCurrencyIconSprite.AssetGUID)) - Error(nameof(IRewardStruct.PremiumCurrencyIconSprite), "required for premium currency"); - - // add other checks here + if (structObj.IsPremiumCurrency && string.IsNullOrEmpty(structObj.PremiumCurrencyIconSprite.AssetGUID)) + Error(nameof(IRewardStruct.PremiumCurrencyIconSprite), "required for premium currency"); } ``` \ No newline at end of file diff --git a/Runtime/Validation/BaseDataValidator.cs b/Runtime/Validation/BaseDataValidator.cs index 15519c9..444d196 100644 --- a/Runtime/Validation/BaseDataValidator.cs +++ b/Runtime/Validation/BaseDataValidator.cs @@ -50,6 +50,29 @@ protected void Error(string message) _errors.Add(error); } + /// + /// Log a warning during validation. + /// + /// property name with the warning + /// user facing message about the warning + protected void Warn(string propertyName, string message) + { + var error = new ValidationError(typeof(T), _currentIdentifier, propertyName, message, + severity: ValidationError.Severity.Warning); + _errors.Add(error); + } + + /// + /// General warning during validation. + /// + /// user facing message about the warning + protected void Warn(string message) + { + var error = new ValidationError(typeof(T), _currentIdentifier, null, message, + severity: ValidationError.Severity.Warning); + _errors.Add(error); + } + void IDataValidator.ValidateInfo(IParameterManager parameterManager, IBaseInfo info) { _currentIdentifier = info.Identifier; diff --git a/Runtime/Validation/ParameterManagerValidator.cs b/Runtime/Validation/ParameterManagerValidator.cs index 3205e3e..d029ebe 100644 --- a/Runtime/Validation/ParameterManagerValidator.cs +++ b/Runtime/Validation/ParameterManagerValidator.cs @@ -1,11 +1,12 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Reflection; using PocketGems.Parameters.Interface; using PocketGems.Parameters.Validation.Attributes; -using UnityEngine; +using Debug = UnityEngine.Debug; namespace PocketGems.Parameters.Validation { @@ -93,6 +94,10 @@ public void Validate() where T : class, IBaseInfo private readonly IParameterManager _parameterManager; private readonly List _errors; + // constants + private const int SlowExecutingValidatorInfoMillis = 10; + private const int SlowExecutingValidatorMillis = 100; + // built in attributes private readonly Func[] _builtInAttributesCreators; private readonly IValidationAttribute[] _builtInAttributesInstancePool; @@ -105,6 +110,7 @@ public void Validate() where T : class, IBaseInfo private void ValidateInfoDataValidators(List infos, bool recurseInterfaces, bool callValidateParameters) where T : class, IBaseInfo { + Stopwatch stopwatch = new(); IReadOnlyList interfacesType; if (recurseInterfaces) interfacesType = ParameterManagerValidatorCache.GetAllInterfaceTypes(typeof(T)); @@ -120,7 +126,8 @@ private void ValidateInfoDataValidators(List infos, bool recurseInterfaces var dataValidators = ParameterManagerValidatorCache.GetDataValidatorMap(); if (!dataValidators.ContainsKey(interfaceType)) { - var error = new ValidationError(typeof(T), null, null, $"Cannot find validator for {interfaceType}"); + var error = new ValidationError(typeof(T), null, null, $"Cannot find validator for {interfaceType}", + severity: ValidationError.Severity.Warning); _errors.Add(error); continue; } @@ -135,6 +142,7 @@ private void ValidateInfoDataValidators(List infos, bool recurseInterfaces _dataValidatorVerifiedParameterManagerSet.Add(dataValidator); try { + stopwatch.Restart(); dataValidator.ValidateParameters(_parameterManager); } catch (Exception e) @@ -143,6 +151,15 @@ private void ValidateInfoDataValidators(List infos, bool recurseInterfaces _errors.Add(error); Debug.LogError(e); } + + stopwatch.Stop(); + if (stopwatch.ElapsedMilliseconds >= SlowExecutingValidatorMillis) + { + var error = new ValidationError(typeof(T), null, null, + $"{dataValidator.GetType().Name}.{nameof(dataValidator.ValidateParameters)} took {stopwatch.ElapsedMilliseconds}ms to run validation.", + severity:ValidationError.Severity.Warning); + _errors.Add(error); + } } // get verified set @@ -161,6 +178,7 @@ private void ValidateInfoDataValidators(List infos, bool recurseInterfaces verifiedInfos.Add(info); try { + stopwatch.Restart(); dataValidator.ValidateInfo(_parameterManager, info); } catch (Exception e) @@ -170,6 +188,14 @@ private void ValidateInfoDataValidators(List infos, bool recurseInterfaces _errors.Add(error); Debug.LogError(e); } + + stopwatch.Stop(); + if (stopwatch.ElapsedMilliseconds >= SlowExecutingValidatorInfoMillis) + { + var error = new ValidationError(typeof(T), info.Identifier, null, $"Took {stopwatch.ElapsedMilliseconds}ms to run info validation.", + severity: ValidationError.Severity.Warning); + _errors.Add(error); + } } } } @@ -187,7 +213,8 @@ private void ValidateStructDataValidators(Type structType, List string.IsNullOrEmpty(_message) ? null : _message; public string StructKeyPath => string.IsNullOrEmpty(_structKeyPath) ? null : _structKeyPath; public string StructProperty => string.IsNullOrEmpty(_structProperty) ? null : _structProperty; + public Severity ErrorSeverity => _severity; [NonSerialized] private Type _infoType; [NonSerialized] private bool _loadedType; @@ -40,9 +47,10 @@ public Type InfoType [SerializeField] private string _message; [SerializeField] private string _structKeyPath; [SerializeField] private string _structProperty; + [SerializeField] private Severity _severity; public ValidationError(Type infoType, string infoIdentifier, string infoProperty, string message, - string structKeyPath = null, string structProperty = null) + string structKeyPath = null, string structProperty = null, Severity severity = Severity.Error) { _infoType = infoType; _loadedType = true; @@ -50,9 +58,10 @@ public ValidationError(Type infoType, string infoIdentifier, string infoProperty _typeString = infoType.AssemblyQualifiedName; _infoIdentifier = infoIdentifier; _infoProperty = infoProperty; + _message = message; _structKeyPath = structKeyPath; _structProperty = structProperty; - _message = message; + _severity = severity; } public override string ToString() diff --git a/Tests/Editor/CodeGeneration/Operation/CodeOperationContextTest.cs b/Tests/Editor/CodeGeneration/Operation/CodeOperationContextTest.cs index faa435a..84bb46e 100644 --- a/Tests/Editor/CodeGeneration/Operation/CodeOperationContextTest.cs +++ b/Tests/Editor/CodeGeneration/Operation/CodeOperationContextTest.cs @@ -14,6 +14,7 @@ public void Coverage() Assert.IsNotEmpty(context.SchemaFilePath); Assert.IsNotEmpty(context.GeneratedCodeDir); Assert.IsNotEmpty(context.GeneratedCodeScriptableObjectsDir); + Assert.IsNotEmpty(context.GeneratedCodeStructsDir); Assert.IsNotEmpty(context.GeneratedCodeFlatBufferClassesDir); Assert.IsNotEmpty(context.GeneratedCodeFlatBufferStructsDir); Assert.IsNotEmpty(context.GeneratedCodeFlatBufferBuilderDir); diff --git a/Tests/Editor/Common/Util/ParameterDebugTest.cs b/Tests/Editor/Common/Util/ParameterDebugTest.cs index 8a8716e..58a8e26 100644 --- a/Tests/Editor/Common/Util/ParameterDebugTest.cs +++ b/Tests/Editor/Common/Util/ParameterDebugTest.cs @@ -29,6 +29,13 @@ public void Log() LogAssert.Expect(LogType.Log, _logMessage); } + [Test] + public void LogWarning() + { + ParameterDebug.LogWarning(_logMessage); + LogAssert.Expect(LogType.Warning, _logMessage); + } + [Test] public void LogError() { diff --git a/Tests/Editor/DataGeneration/Operation/DataOperationContextTest.cs b/Tests/Editor/DataGeneration/Operation/DataOperationContextTest.cs index 6893fb4..025cd77 100644 --- a/Tests/Editor/DataGeneration/Operation/DataOperationContextTest.cs +++ b/Tests/Editor/DataGeneration/Operation/DataOperationContextTest.cs @@ -31,6 +31,7 @@ public void Coverage() #endif Assert.AreEqual(0, context.ScriptableObjectMetadatas.Count); Assert.AreEqual(0, context.GeneratedFilePaths.Count); + Assert.AreEqual(0, context.AllValidationErrors.Count); } } } diff --git a/Tests/Runtime/TestCode/TestBaseDataValidator.cs b/Tests/Runtime/TestCode/TestBaseDataValidator.cs index 5e677a1..0492696 100644 --- a/Tests/Runtime/TestCode/TestBaseDataValidator.cs +++ b/Tests/Runtime/TestCode/TestBaseDataValidator.cs @@ -5,17 +5,21 @@ namespace PocketGems.Parameters.Validation public class TestBaseDataValidator : BaseDataValidator where T : class, IBaseInfo { public const string PropertyName = "some property name"; - public const string ErrorMessage1 = "some message"; - public const string ErrorMessage2 = "some other message"; + public const string ErrorMessage1 = "some error"; + public const string ErrorMessage2 = "some other error"; + public const string WarningMessage1 = "some warning"; + public const string WarningMessage2 = "some other warning"; protected override void ValidateInfo(IParameterManager parameterManager, T info) { Error(PropertyName, ErrorMessage1); + Warn(PropertyName, WarningMessage1); } protected override void ValidateParameters(IParameterManager parameterManager) { Error(ErrorMessage2); + Warn(WarningMessage2); } } } diff --git a/Tests/Runtime/Validation/BaseDataValidatorTest.cs b/Tests/Runtime/Validation/BaseDataValidatorTest.cs index c60cb3d..6a375b6 100644 --- a/Tests/Runtime/Validation/BaseDataValidatorTest.cs +++ b/Tests/Runtime/Validation/BaseDataValidatorTest.cs @@ -16,12 +16,21 @@ public void ValidateInfo() validator.ValidateInfo(null, mock); var errors = validator.Errors; - Assert.AreEqual(1, errors.Count); + Assert.AreEqual(2, errors.Count); + var error = errors[0]; Assert.AreEqual(typeof(IMySpecialInfo), error.InfoType); Assert.AreEqual(infoIdentifier, error.InfoIdentifier); + Assert.AreEqual(ValidationError.Severity.Error, error.ErrorSeverity); Assert.AreEqual(TestBaseDataValidator.PropertyName, error.InfoProperty); Assert.AreEqual(TestBaseDataValidator.ErrorMessage1, error.Message); + + var warning = errors[1]; + Assert.AreEqual(typeof(IMySpecialInfo), warning.InfoType); + Assert.AreEqual(infoIdentifier, warning.InfoIdentifier); + Assert.AreEqual(ValidationError.Severity.Warning, warning.ErrorSeverity); + Assert.AreEqual(TestBaseDataValidator.PropertyName, warning.InfoProperty); + Assert.AreEqual(TestBaseDataValidator.WarningMessage1, warning.Message); } [Test] @@ -31,12 +40,21 @@ public void ValidateParameters() validator.ValidateParameters(null); var errors = validator.Errors; - Assert.AreEqual(1, errors.Count); + Assert.AreEqual(2, errors.Count); + var error = errors[0]; Assert.AreEqual(typeof(IMySpecialInfo), error.InfoType); Assert.IsNull(error.InfoIdentifier); Assert.IsNull(error.InfoProperty); + Assert.AreEqual(ValidationError.Severity.Error, error.ErrorSeverity); Assert.AreEqual(TestBaseDataValidator.ErrorMessage2, error.Message); + + var warning = errors[1]; + Assert.AreEqual(typeof(IMySpecialInfo), warning.InfoType); + Assert.IsNull(warning.InfoIdentifier); + Assert.IsNull(warning.InfoProperty); + Assert.AreEqual(ValidationError.Severity.Warning, warning.ErrorSeverity); + Assert.AreEqual(TestBaseDataValidator.WarningMessage2, warning.Message); } } } diff --git a/package.json b/package.json index 26c3ca8..5672466 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "com.pocketgems.scriptableobject.flatbuffer", - "version": "4.2.1", + "version": "4.3.0", "displayName": "Scriptable Object - FlatBuffer", "description": "Seamless syncing between Scriptable Objects and CSVs. Scriptable Object data built to Google FlatBuffers for optimal runtime loading & access.", "unity": "2021.3",