Skip to content

Fixed SetMinValue and SetMaxValue not changing min/max values.#13

Merged
lextatic merged 3 commits intomainfrom
bugfix/fixed-typo-in-minValue-assertion
Apr 16, 2026
Merged

Fixed SetMinValue and SetMaxValue not changing min/max values.#13
lextatic merged 3 commits intomainfrom
bugfix/fixed-typo-in-minValue-assertion

Conversation

@lextatic
Copy link
Copy Markdown
Contributor

@lextatic lextatic commented Apr 15, 2026

Fixes #12

Fixed

  • Fixed typo in MinValue assertion.
  • Fixed SetMinValue and SetMaxValue not actually changing min/max values.

Added

  • Added new test cases for those methods.

Thanks @datouzhu125 for reporting!

Copilot AI review requested due to automatic review settings April 15, 2026 22:57
@lextatic lextatic added the fixed General bug fixes label Apr 15, 2026
Copy link
Copy Markdown
Contributor

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

Updates validation messaging in EntityAttribute.SetMinValue to correctly describe the invalid condition when a proposed minimum exceeds the current maximum.

Changes:

  • Fixes the SetMinValue assertion message from “lower” to “greater” to match the newMinValue <= Max condition.
Comments suppressed due to low confidence (1)

Forge/Attributes/EntityAttribute.cs:122

  • SetMinValue(int newMinValue) validates newMinValue but never assigns it to anything; Min is a get-only auto-property and the method clamps BaseValue using the existing Min value. As written, calling this method cannot change the attribute's min bound, which makes the API misleading and likely breaks callers (e.g., code expecting to update Min/Max at runtime). Consider either (a) making Min/Max backed by private fields and updating them here (and clamping BaseValue/CurrentValue against the new bounds), or (b) removing/renaming these methods and adjusting callers/docs if min/max are intended to be immutable.
		Validation.Assert(newMinValue <= Max, "MinValue cannot be greater than MaxValue.");

		var oldValue = CurrentValue;

		BaseValue = Math.Max(BaseValue, Min);

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

@lextatic lextatic changed the title Fixed typo in MinValue assertion. Fixed SetMinValue and SetMaxValue not actually changing min/max values. Apr 16, 2026
@lextatic lextatic changed the title Fixed SetMinValue and SetMaxValue not actually changing min/max values. Fixed SetMinValue and SetMaxValue not changing min/max values. Apr 16, 2026
@lextatic lextatic merged commit 50a523f into main Apr 16, 2026
1 check passed
@lextatic lextatic deleted the bugfix/fixed-typo-in-minValue-assertion branch April 16, 2026 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fixed General bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logical error in min/max validation condition

2 participants