Skip to content
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

Expose Vector* component-wise and scalar min/max to scripting #80223

Merged
merged 1 commit into from
May 2, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Aug 3, 2023

Added for convenience, which was mentioned in the PR changing the behavior of the generic min/max

If the naming is considered confusing we can use component_max/min just to make it more clear (though more verbose)

Will open a godot-cpp equivalent as well
Done: godotengine/godot-cpp#1197

@MewPurPur
Copy link
Contributor

Regarding the resolved comment above, shouldn't documentation examples, on the contrary, avoid static typing? So no mini() and minf() for example?

@AThousandShips
Copy link
Member Author

AThousandShips commented Aug 3, 2023

Here the typing is static because of the vector types, no, and the specific versions have better performance so would recommend using them whenever reasonable? But can switch back if desired

@AThousandShips
Copy link
Member Author

Added note to the documentation in line with #84656

@raulsntos
Copy link
Member

Unfortunately the Min/Max ones don't fit in the integer vector versions because they already have Min/Max as constants

Luckily those members have been added in 4.2 (#81819) which is still unreleased so we have a chance to rename them. I would name them MinValue and MaxValue to match the int names. Then we can use the Min and Max names for the methods which seems consistent with the methods in Mathf.

@AThousandShips
Copy link
Member Author

Great! Was considering that, will make the changes here and see what ends up happening

@AThousandShips
Copy link
Member Author

AThousandShips commented Nov 9, 2023

Resolved it for the CI here, but would be maybe good to get a dedicated one for C# done for 4.2 anyway

I'll go ahead and make one

@AThousandShips

This comment was marked as outdated.

@AThousandShips AThousandShips marked this pull request as draft May 2, 2024 08:30
@AThousandShips AThousandShips changed the title Expose Vector* component-wise min/max to scripting Expose Vector* component-wise min/max and scalar mini/maxi to scripting May 2, 2024
@AThousandShips AThousandShips changed the title Expose Vector* component-wise min/max and scalar mini/maxi to scripting Expose Vector* component-wise and scalar min/max to scripting May 2, 2024
@AThousandShips AThousandShips marked this pull request as ready for review May 2, 2024 11:03
@akien-mga akien-mga modified the milestones: 4.x, 4.3 May 2, 2024
@akien-mga akien-mga merged commit 396ce1a into godotengine:master May 2, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the vec_elem branch May 2, 2024 12:05
@AThousandShips
Copy link
Member Author

Thank you!

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.

Expose Vector* component-wise min/max to scripting
4 participants