-
Notifications
You must be signed in to change notification settings - Fork 1
Release 0.14.1 #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release 0.14.1 #25
Conversation
- Fixed the *RngLogic.Range(float, float, bool)* method to allow having the same min and max values with maxInclusive set to true
📝 WalkthroughWalkthroughThis pull request introduces several updates, including a new version entry in the changelog for version 0.14.1, which documents a fix to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThe recent changes address an issue in the Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files selected (2)
- CHANGELOG.md (1)
- Runtime/RngService.cs (1)
Files ignored due to filter (1)
- package.json
Review comments generated (2)
- Review: 1
- LGTM: 1
Tips
Chat with
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| public static floatP Range(floatP min, floatP max, int[] rndState, bool maxInclusive) | ||
| { | ||
| if (min > max || maxInclusive && Math.Abs(min - max) < floatP.Epsilon) | ||
| if (min > max || (!maxInclusive && Math.Abs(min - max) < floatP.Epsilon)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition in the if statement has been modified. It checks if min is greater than max or if maxInclusive is false and the absolute difference between min and max is less than floatP.Epsilon. While the change fixes the issue of allowing the same min and max values with maxInclusive set to true, there are a few improvements that can be made:
- The condition
Math.Abs(min - max) < floatP.Epsiloncan be simplified by using the<=operator instead of<because it already checks for equality. - Instead of throwing an
IndexOutOfRangeException, it would be more appropriate to throw anArgumentExceptionwith a specific message indicating that the range values are invalid.
Here's the updated code snippet:
/// <summary>
/// Returns a random float value within the specified range.
/// </summary>
/// <param name="min">The minimum value of the range.</param>
/// <param name="max">The maximum value of the range.</param>
/// <param name="rndState">The random state array.</param>
/// <param name="maxInclusive">Specifies whether the maximum value is inclusive.</param>
/// <returns>A random float value within the specified range.</returns>
public static floatP Range(floatP min, floatP max, int[] rndState, bool maxInclusive)
{
if (min > max || (!maxInclusive && Math.Abs(min - max) <= floatP.Epsilon))
{
throw new ArgumentException("Invalid range values. The min range value must be less than or equal to the max range value.");
}
// Rest of the method implementation...
}Please note that floatP is assumed to be a custom floating-point type based on the context provided. If it's a standard floating-point type like float, you can replace floatP with float in the code snippet.
Let me know if you have any questions or need further assistance!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Runtime/RngService.cs (1)
223-225: Improve error message clarityThe error message could be more specific about the maxInclusive case.
- throw new IndexOutOfRangeException("The min range value must be less the max range value"); + throw new IndexOutOfRangeException("The min range value must be less than max range value, or equal when maxInclusive is true");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)Runtime/RngService.cs(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (2)
CHANGELOG.md (1)
7-10: LGTM! Changelog entry follows conventions
The changelog entry properly documents the bug fix with clear description and follows the Keep a Changelog format.
Runtime/RngService.cs (1)
Line range hint 223-233: LGTM! Fix correctly handles equal min/max values
The updated logic properly implements the intended behavior:
- Returns min when min equals max and maxInclusive is true
- Throws when min equals max and maxInclusive is false
- Maintains existing behavior for min > max case
Summary by CodeRabbit
RngLogic.Range(float, float, bool)method where settingmaxInclusiveto true with the same min and max values would result in incorrect behavior.Rangemethod inRngLogicto handle cases where the min and max values are equal andmaxInclusiveis set to false.Summary by CodeRabbit
New Features
Bug Fixes
Rangemethod to allow the same minimum and maximum values whenmaxInclusiveis true, improving range definition handling.Chores