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

Implement Multiple Polygon Functions #153

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

Conversation

KcRobin9
Copy link
Collaborator

The aim is to understand how the collision table is generated, such that this knowledge can be applied when creating the _HITID.BND from scratch.

The aim is to understand how the collision table is generated, such that this knowledge can be applied when creating the `_HITID.BND` from scratch.
Copy link
Owner

@0x1F9F1 0x1F9F1 left a comment

Choose a reason for hiding this comment

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

Some of these comments apply to multiple places, but I just mentioned it in the first instance.

@@ -248,7 +248,7 @@ class mmBoundTemplate
// ?WinID@mmBoundTemplate@@2HA | mmdyna:bndtmpl2
ARTS_IMPORT static i32 WinID;

private:
public:
Copy link
Owner

Choose a reason for hiding this comment

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

Does this need to be made public?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes.
It's required for mmBoundTemplate::VertPtr and the ->X/Y/Z Dims

f32 mmPolygon::CheckCellXSide(f32 x_plane, f32 z_min, f32 z_max)
{
f32 max_y = -999999.0f;
i32 vert_count = (Flags & 4) != 0 ? 4 : 3;
Copy link
Owner

Choose a reason for hiding this comment

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

Could you replace the manual flag checks with functons? Like IsQuad() and GetNumVerts()

Comment on lines 44 to 47
if (y_int > max_y)
{
max_y = y_int;
}
Copy link
Owner

Choose a reason for hiding this comment

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

Can be replaced with std::max

f32 max_y = -999999.0f;
i32 vert_count = (Flags & 4) ? 4 : 3;
f32 adjusted_sign = (PlaneN.y <= 0.0f) ? 1.0f : -1.0f;
f32 x_plane[4], z_plane[4], const_plane[4], length, inverse_length;
Copy link
Owner

Choose a reason for hiding this comment

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

Try and avoid forward declaring variables when not necssary, i.e length, inverse_length


for (i32 i = 0; i < vert_count; ++i)
{
const Vector3& cur_vert = mmBoundTemplate::VertPtr[VertIndices[i % vert_count]];
Copy link
Owner

Choose a reason for hiding this comment

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

Can remove the modulo here

return max_y;
}

f32 mmPolygon::CheckCorner(f32 x, f32 z, f32* x_plane, f32* z_plane, f32* const_plane)
Copy link
Owner

Choose a reason for hiding this comment

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

I think maybe plane_x, plane_z, and plane_d may be better names for these

Comment on lines 134 to 146
f32 corner_height;

corner_height = CheckCorner(x1, z1, x_plane, z_plane, const_plane);
max_y = std::max(max_y, corner_height);

corner_height = CheckCorner(x1, z2, x_plane, z_plane, const_plane);
max_y = std::max(max_y, corner_height);

corner_height = CheckCorner(x2, z1, x_plane, z_plane, const_plane);
max_y = std::max(max_y, corner_height);

corner_height = CheckCorner(x2, z2, x_plane, z_plane, const_plane);
max_y = std::max(max_y, corner_height);
Copy link
Owner

Choose a reason for hiding this comment

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

You can just inline these into the param for max, not much point assigning them to a variable

Hopefully all points are addressed adequately
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.

None yet

2 participants