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

types for hyperbolic space #420

Merged
merged 32 commits into from
Jul 28, 2024
Merged

types for hyperbolic space #420

merged 32 commits into from
Jul 28, 2024

Conversation

strawberrycinnabar
Copy link
Contributor

by #264 adds MIsometry and MVector types

hyperbolic space, replacing na::Vector4 when appropriate
added MIsometry in math.rs representing hyperbolic isometries, replacing na::Matrix4 when appropriate

to compensate for these changes, added inline functions passing self to
respective functions, as well as reimplementing traits

moved some functions in math.rs with MIsometry and MVector arguments to
respective impls
client/src/graphics/draw.rs Outdated Show resolved Hide resolved
common/src/dodeca.rs Outdated Show resolved Hide resolved
common/src/math.rs Outdated Show resolved Hide resolved
common/src/math.rs Outdated Show resolved Hide resolved
common/src/math.rs Outdated Show resolved Hide resolved
common/src/math.rs Outdated Show resolved Hide resolved
client/src/graphics/draw.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@patowen patowen left a comment

Choose a reason for hiding this comment

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

I've reviewed the code more carefully this time. I believe that once my suggestions are dealt with, this PR will be ready to merge.

client/src/local_character_controller.rs Outdated Show resolved Hide resolved
common/src/math.rs Outdated Show resolved Hide resolved
common/src/math.rs Outdated Show resolved Hide resolved
common/src/math.rs Outdated Show resolved Hide resolved
common/src/math.rs Outdated Show resolved Hide resolved
common/src/collision_math.rs Outdated Show resolved Hide resolved
common/src/plane.rs Outdated Show resolved Hide resolved
common/src/plane.rs Outdated Show resolved Hide resolved
common/src/traversal.rs Outdated Show resolved Hide resolved
common/src/traversal.rs Outdated Show resolved Hide resolved
@patowen patowen requested a review from Ralith July 27, 2024 00:05
Copy link
Collaborator

@patowen patowen left a comment

Choose a reason for hiding this comment

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

Looks good to me! Once you run cargo fmt on this, it should be ready to merge.

@patowen patowen merged commit e80465d into Ralith:master Jul 28, 2024
4 checks passed
@patowen
Copy link
Collaborator

patowen commented Jul 28, 2024

Thanks!

@patowen
Copy link
Collaborator

patowen commented Jul 28, 2024

The "package" job for Ubuntu failed for this PR, but that is entirely unrelated to this PR. I believe the root cause is actions/checkout#1809. I hope that a good solution is found for this before it's time to release another version of Hypermine, but otherwise, I'm not sure what the best solution will be that requires the least boring maintenance work. Ideally, these GitHub workflows shouldn't need to be edited very often at all, but this is the second time this year that the package workflows randomly broke.

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.

4 participants