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

Improve Vec4Base sub-class functionality and Vec4 directional vector method name change #2072

Merged
merged 2 commits into from
Aug 4, 2024

Conversation

rh101
Copy link
Contributor

@rh101 rh101 commented Aug 4, 2024

Describe your changes

Move methods from Vec4 to Vec4Base, specifically set(x,y,z,w) and set(float*). This allows their usage in Color4F and other sub-classes of Vec4Base.

Remove trivial copy constructor method, since it does not need to be explicitly implemented.

Rename Vec4::set(const Vec4& p1, const Vec4& p2) method to Vec4::setDirection to reflect its actual purpose.

If anyone currently uses the Vec4::set(const Vec4& p1, const Vec4& p2), then it is a simple rename to Vec4::setDirection(const Vec4& p1, const Vec4& p2). The set name was a bit misleading, and this change renames it to what the method actually does, which is to calculate the directional vector from point p1 to point p2. The compiler will throw an error regarding this, so there is no danger that this goes unnoticed.

Note that I did try to add the AX_DEPRECATED_ATTRIBUTE specifier to the the Vec4::set(const Vec4& p1, const Vec4& p2) method, but the compiler would throw errors when attempting to use the other set methods that were moved to Vec4Base, since they were being hidden by the Vec4::set method.

Issue ticket number and link

Checklist before requesting a review

For each PR

  • Add Copyright if it missed:
    - "Copyright (c) 2019-present Axmol Engine contributors (see AUTHORS.md)."

  • I have performed a self-review of my code.

    Optional:

    • I have checked readme and add important infos to this PR.
    • I have added/adapted some tests too.

For core/new feature PR

  • I have checked readme and add important infos to this PR.
  • I have added thorough tests.

…ubclasses of Vec4Base

Rename Vec4::set method to setDirection to reflect its actual purpose
Remove trivial copy constructor method since it does not need to be explicitly implemented
@halx99 halx99 merged commit 3a46201 into axmolengine:dev Aug 4, 2024
15 checks passed
@rh101 rh101 deleted the improve-vec4-base branch August 4, 2024 12:05
@aismann
Copy link
Contributor

aismann commented Aug 4, 2024

@rh101
Is the title correct?
...Enhanced Color4F ...

@rh101
Copy link
Contributor Author

rh101 commented Aug 4, 2024

Is the title correct?
...Enhanced Color4F ...

Fair point, as initially there was changes to the Color4F class, but they I reverted them before creating this PR, since they were not required. I'll fix the title to be more appropriate, since the enhancement to Color4F was more a side-effect of the change.

@rh101 rh101 changed the title Enhance Color4F and method naming change Improve Vec4Base sub-class functionality and Vec4 directional vector method name change Aug 4, 2024
@halx99 halx99 added this to the 2.1.5 milestone Aug 6, 2024
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.

3 participants