Create vector math structs#345
Conversation
CloneDeath
requested review from
NeilFerguson,
Z80coder,
dlaydon,
gnedjati,
igfoo and
matt-gretton-dann
as code owners
CloneDeath
force-pushed
the
create-vector-math-structs
branch
from
ca4dfe9 to
f7efa10
Compare
|
@matt-gretton-dann Do you think you'll have a chance to look at this PR today? |
NeilFerguson
left a comment
NeilFerguson
left a comment
There was a problem hiding this comment.
Despite my fondness for pure C, these look fine, assuming regression tests are OK.
Yup, all the tests currently pass. I've purposely only introduced the classes and replaced the member pairs, in order to minimize the chance of any errors being introduced. |
CloneDeath
force-pushed
the
create-vector-math-structs
branch
from
f7efa10 to
369a2df
Compare
|
@matt-gretton-dann Given Neil's approval, do you think you can merge this, so I can start working on the next part? |
CloneDeath
force-pushed
the
create-vector-math-structs
branch
from
369a2df to
276eea7
Compare
|
@weshinsley Would you mind taking a look at this PR for merge? It's the first step in refactoring out the vector math, which is why the changes are pretty limited. |
|
Got 2 or 3 I'm looking at/testing at the moment - will do this next... very quick picky thing - can we have a newline at the end of your new files? (Also, I think another merge has just nudged Model.h... sorry) |
CloneDeath
force-pushed
the
create-vector-math-structs
branch
from
276eea7 to
b845f55
Compare
…prevent circular dependencies due to their interaction with Dist.h
…dependency due to Dist.h
CloneDeath
force-pushed
the
create-vector-math-structs
branch
from
19324ad to
b2fecc5
Compare
|
@weshinsley No problem. Merge conflict fixed, and newlines added. 👍 |
|
Ok, on the basis of Neil's approval, tests passing, and identical report 9 results, this looks good to merge. |
3 participants
My other PR #325 was getting very large, and I decided to split it up into parts. Future pull requests (after this is merged) will focus on utilizing this refactoring, ie by simplifying a lot of vector math.
This PR mainly focuses on introducing the Vector2, Size, and BoundingBox classes, and replacing several member pairs with their usage.
@matt-gretton-dann I held back on pulling structs out of Model except for the bare minimum.
The ones I pulled out were simply due to circular dependencies in the header files, caused by Diff.h.
Ideally, I'd like to remove the dependency of Diff on several Model classes (as I did in #325) but I'll do that in a separate PR.