Skip to content

Conversation

@VALERA771
Copy link

Description

Describe the changes
Adds config validators alongside with some new features that wasn't presented in previos PRs

What is the current behavior? (You can also link to an open issue here)

What is the new behavior? (if this is a feature change)

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

Other information:


Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentations

Submission checklist

  • I have checked the project can be compiled
  • I have tested my changes and it worked as expected

Patches (if there are any changes related to Harmony patches)

  • I have checked no IL patching errors in the console

Other

  • Still requires more testing

Copy link
Collaborator

@Someone-193 Someone-193 left a comment

Choose a reason for hiding this comment

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

Just some minor docs things, I don't really understand how this fully works so I don't know if it'll work, so if it works according to you @VALERA771 and you fix any docs issues I found, I think it should be good to go

// </copyright>
// -----------------------------------------------------------------------

using System.Reflection;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move to after "using System.Linq"

foreach (PropertyInfo propertyInfo in config.GetType().GetProperties().Where(x => x.GetMethod != null && x.SetMethod != null))
plugin.ValidateType(config, propertyInfo, ref validated);

Log.Info($"Config has successfully passed {validated} validations!");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe only print this if validated is > 0 and also include the plugins name too? So like if a plugin has validation, print "MyPlugin's Config has passed 3 validations"

Comment on lines +12 to +25
/// <summary>
/// Check if value is 0 or less.
/// </summary>
[AttributeUsage(AttributeTargets.Property)]
public class NonPositiveAttribute : LessOrEqualAttribute
{
/// <summary>
/// Initializes a new instance of the <see cref="NonPositiveAttribute"/> class.
/// </summary>
public NonPositiveAttribute()
: base(0)
{
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe call this NegativeAttribute or IsNegativeAttribute? NonPositive is a bit convoluted

Comment on lines +12 to +25
/// <summary>
/// Checks if value is 0 or greater.
/// </summary>
[AttributeUsage(AttributeTargets.Property)]
public class NonNegativeAttribute : GreaterOrEqualAttribute
{
/// <summary>
/// Initializes a new instance of the <see cref="NonNegativeAttribute"/> class.
/// </summary>
public NonNegativeAttribute()
: base(0)
{
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe call this PositiveAttribute or IsPositiveAttribute? NonNegative is a bit convoluted

}

/// <summary>
/// Gets the minimum value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be a maximum value if this attribute checks if a given value in a config is less than the value provided to the attribute?

}

/// <summary>
/// Gets the minimum value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this be a maximum value if this attribute checks if a given value in a config is less than the value provided to the attribute?

@VALERA771
Copy link
Author

Basically this PR introduces attributes that perform checks on config load. If check is not passed - default value is used. I'll fix docs and do some tests on week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants