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

Reuse code to remove overloads via templates #1319

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ZXShady
Copy link

@ZXShady ZXShady commented Nov 1, 2024

THIS PR IS NOT COMPLETE,

I currently now have split it into commits to see if they are fine to be metrged, and I had some issues in local commits with internal compiler errors that I will yet fix.

First of all I want to thank the authors for making this amazing library.

but one thing I really hated is the insanely long error messages for simple mistakes like glm::ivec3() + 1.0f gets me a 210 line error message like this

error_before.txt

but after removing unnecessary overloads I got it down to this (I still could reduce the unneeded overloads even more)

error_after.txt

it also significantly reduced the amount of boiler plate code written in the header files and leaves less room for mistakes.
like unary operator+ for mat was repeated 16 times!! while it had the same definition every time. 16x less code

this pr breaks code though like

--glm::ivec3(0); // no longer compiles

I made it a free function that takes a non const lvalue so that no longer compiles (and it should tbh just write ivec3() -1)

@ZXShady ZXShady force-pushed the remove_unneccessary_overloads branch 5 times, most recently from fddc0eb to bb563dc Compare November 3, 2024 19:04
@ZXShady ZXShady closed this Mar 7, 2025
@ZXShady ZXShady force-pushed the remove_unneccessary_overloads branch from f9186b8 to 2d4c4b4 Compare March 7, 2025 18:22
@ZXShady ZXShady reopened this Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant