introduce `plane_t` struct with `normal` and `dist` members

[illwieckz]

Previously the plane was stored in a vec4_t, with the three first components being the normal, and the last one being the distance.

A lot of code was running the vec3_t specific macros on the vec4_t, it only worked because we use unsafe types.

This change is a step on the road of making safe types like vec2_t, vec3_t, vec4_t and turne macros like VectorCopy into type-safe functions.

This also makes the code more explicit.

Instead of doing:

    vec3_t normal = { 0, 0, 0 };
    vec4_t plane;
    VectorCopy( normal, plane );

We can now do:

    vec3_t in = { 0, 0, 0 };
    plane_t plane;
    VectorCopy( normal, plane.normal );

The day VectorCopy is a type-safe function instead of a macro, it will work.

There already exist a cplane_t struct that carries normal and dist but also more information, we may modify it to use plane_t for normal and dist, this would allow us to do:

    PlaneCopy( cplane.plane, plane )

instead of:

    VectorCopy( cplane.normal, plane.normal );
    plane.dist = cplane.dist;

But for now this is out of topic.

[illwieckz]

Hmm, it looks like there is a mistake in tracing, I verified I can go through the square pipe next to human base in station15 (I assume the pipe is a patch).

[illwieckz]

I fail to spot the bug in the code I wrote.

I wonder if introducing type safety for the plane is uncovering a dormant bug.

[DolceTriade]

I worry it might be more harm than good...

[illwieckz]

I worry it might be more harm than good...

Keeping the current code is really bad, it's full of stupid things like that:

void function1( vec3_t v, float f ) {
  /* code */
}

void function2()
  vec4_t v;
  function1( v, v[3] );
}

Everything is implicit in bad ways, like this:

vec4_t v;
VectorCopy( b, v );

which is a shady way to do:

vec4_t v;
VectorCopy( b.xyz, v.xyz );

All of this is lying to the reader and asking for troubles.

[slipher]

It's broken because you used plane_t, a value type, in function arguments, in places where it needs to be a reference/pointer.

[illwieckz]

Right. This should now be better (on my end it is).

[illwieckz]

On my end the station15 patch collision detection works again.

The unit test seems to be passing again.

This looks ready to me.

[illwieckz]

To avoid confusion when reading my comment and my code, I said:

There already exist a cplane_t struct I have not modified to use plane_t, but I modified the cPlane_t struct to use plane_t (we may look for better names).

[slipher]

using frustum_t = cplane_t[6];

Well that's a tricky typedef. And kinda stupid since cplane_t has a bunch of extra fields that aren't relevant. OK, apparently they're defining the frustum by some bounding planes similarly to a brush. I was mistaken to say that none of the renderer stuff makes sense; the frustum use case is probably fine (though I'd prefer to see a separate commit). I still think a lot of the renderer changes don't make sense so I'll try to make some more specific comments.

[illwieckz]

This looks ready to me.

This improves type saftety and readability, and it removes various abuse of vector function implictly used on subset of larger vectors (like doing VectorCopy( a, b ) where VectorCopy would only copy a.xyz from a vec4_t).

[illwieckz]

When I did this:

• https://github.com/DaemonEngine/Daemon/pull/1125

I first implemented it over this branch, and profiled the game in Orbit from time to time to see if I seen a difference in performances, what I have seen.

But then before opening the PR I rebased on master, and I profiled the game again, and I noticed that after rebasing on master, the game went slower. After I rebased back on this branch, the perf gain I was expecting was restored. The screenshots in #1125 were done with the code rebased on this branch, doing an After screenshot while rebased on master would require to also redo the Before one, because the rest of the code on master looks to be slower.

So, it looks like this code does not only clean-up the code and make it more readable to humans, but it may also help the compiler to grasp the code in input and optimize the generated code and make it faster too.