Skip to content

Conversation

@Clubhouse1661
Copy link
Contributor

Fixes #114522

Clarifies the documentation for min() and max() functions to specify that they require at least 2 arguments, rather than the imprecise "any number of arguments" which could incorrectly suggest 0 or 1 arguments are valid.

  • Updated @GlobalScope.xml to state both functions "take at least 2 arguments"

Copilot AI review requested due to automatic review settings January 9, 2026 13:48
@Clubhouse1661 Clubhouse1661 requested a review from a team as a code owner January 9, 2026 13:48

This comment was marked as spam.

Copy link
Member

@Ivorforce Ivorforce left a comment

Choose a reason for hiding this comment

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

The definition is in VariantUtilityFunctions::max, which does appear to check for < 2 and error if so. So this change is correct.

However, it doesn't make much sense to me that the function doesn't handle the single-arg case in the first place. I'd rather change it to do so and change the documentation accordingly.

@Clubhouse1661
Copy link
Contributor Author

Thanks for the review. I agree that handling the single-argument case would be more intuitive, but that would be a behavior change rather than a documentation fix.
Perhaps we should open a separate issue for it.

Otherwise, here's how we could add single-argument support:
core/variant/variant_utility.cpp

Variant VariantUtilityFunctions::max(const Variant **p_args, int p_argcount, Callable::CallError &r_error) {
    if (p_argcount < 1) {  // Changed from < 2
        r_error.error = Callable::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS;
        r_error.expected = 1;  // Changed from 2
        return Variant();
    }
    
    // Validate first argument type
    Variant::Type first_type = p_args[0]->get_type();
    if (first_type != Variant::INT && first_type != Variant::FLOAT) {
        r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
        r_error.argument = 0;
        r_error.expected = Variant::FLOAT;
        return Variant();
    }
    
    // Handle single-argument case
    if (p_argcount == 1) {
        r_error.error = Callable::CallError::CALL_OK;
        return *p_args[0];
    }
    
    // ... rest of existing logic unchanged
}

@akien-mga akien-mga modified the milestones: 4.6, 4.x Jan 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs: Imprecise description for min()

4 participants