Create matrix.h and matrix_stack.h

[Dragorn421]

Create matrix_stack.h with all the stack-based matrix manipulation functions, and matrix.h with the rest of the matrix functions (besides the gu ones which should be in libultra's gu.h (and currently are in functions.h))

[fig02]

I'd think it makes more sense for sys_matrix and skin_matrix to be separate headers. They are different systems that aren't really compatible with each other, being that one manipulates the matrix stack and the other does not.

Take for example eff_ss_kirakira, which opts to use solely skinmatrix operations instead of the matrix stack. It should probably just be able to include the specific matrix system it wants to use on its own

[Dragorn421]

What do we gain from two separate headers?

I think we should split headers by feature rather than make basically one .h per .c, otherwise we'd end up with separate headers for the same thing (like matrix operations here) or a single header with way too much going on (z_actor.c)

Do note that sys_matrix.c is not only operations on the matrix stack, it also has operations that don't involve the stack:

Mtx* Matrix_MtxFToMtx(MtxF* src, Mtx* dest);
void Matrix_MtxFCopy(MtxF* dest, MtxF* src);
void Matrix_MtxToMtxF(Mtx* src, MtxF* dest);
void Matrix_MultVec3fExt(Vec3f* src, Vec3f* dest, MtxF* mf);
void Matrix_Transpose(MtxF* mf);
void Matrix_MtxFToYXZRotS(MtxF* mf, Vec3s* rotDest, s32 flag);
void Matrix_MtxFToZYXRotS(MtxF* mf, Vec3s* rotDest, s32 flag);
MtxF* Matrix_CheckFloats(MtxF* mf, const char* file, int line);
void Matrix_SetTranslateScaleMtx2(Mtx* mtx, f32 scaleX, f32 scaleY, f32 scaleZ, f32 translateX, f32 translateY,
                                  f32 translateZ);

I would be ok splitting the header between "matrix_stack.h" and "matrix_stackless.h" (?), but those Matrix_ functions should then still go with SkinMatrix_ functions into "matrix_stackless.h" imo

[fig02]

Yeah I am not suggesting one header per c file, but one header per system. These are completely different systems that dont work with each other. Whereas yes, z_actor.c contains a lot of different systems in it, so not all of it will be in the same header.

We gain the granularity to allow the programmer to pick which system they want to include into their file. I dont think you should have to include a bunch of unnecessary things because you want to work with the concept of "matrices" in general.

--

Sure, we could look into some scenario where the matrix stack operations in sys_matrix belong to a new system, and you would have functions like MatrixStack_Init, MatrixStack_Push, MatrixStack_Pop... etc. This would just be a huge improvement anyway, and let the user know point blank that they are messing with the stack.

In this scenario we could also look into changing the SkinMatrix_ system to just Matrix_, along with the other generic matrix operations you mention in sys_matrix.c. This aligns with how things are used anyway-- skin matrix functions clearly started out as helpers for the skin skeleton system, but other devs found it useful outside of that system. It makes sense for us to name it generically to suit that.

[fig02]

Though then we will have to resolve overlaps such as Matrix_MtxFToMtx and SkinMatrix_MtxFToNewMtx. it may get messy 😅. But this is kinda my point of them being seperate systems with their own implementations.

[cadmic]

I also think it should be sys_matrix.h and skin_matrix.h. To it seems like "matrix.h" and "matrix_stack.h" is the same thing, except with much worse names, because the names don't match the subsystems they correspond to

[cadmic]

I mean it's evidence but not the main reason, if the two files worked better with each other (or other factors like both being named sys_ or z_) then I would consider them more the same system. The main reason is that the code does not treat sys_matrix and skin_matrix as a unified whole.

For example, Matrix_TranslateRotateZYX and SkinMatrix_SetTranslateRotateZYX theoretically do the same thing, but actually are pretty different. Not just matrix stack vs no stack, but also different parameter types and different implementations that share no code and probably have different rounding behavior (though I haven't checked). So I think it would be confusing to mix them too closely

[cadmic]

Now if the goal isn't usability, what is it?

Documentation and reverse-engineering I guess? Like I want to give readers the impression of "oh huh, there's two different matrix subsystems and they're often completely different, that's cool/horrifying/good-to-know".

I don't think it's less usable, if you're modding then you should probably just use sys_matrix and forget about skin_matrix entirely

[fig02]

Think im personally bowing out of this conversation for now, cause I'd just be repeating myself. I've said what I want to say.
Would love to hear more thoughts than just us 3, if anyone else wants to give their opinion.

And lastly I'll say that if we don't come to a conclusion on this now, that we should go back to 1 header until we do decide

[Dragorn421]

Documentation and reverse-engineering I guess? Like I want to give readers the impression of "oh huh, there's two different matrix subsystems and they're often completely different, that's cool/horrifying/good-to-know\

Alright that's reasonable to me (though I dislike splitting the systems based on the .c. During development I could see a dev adding a matrix function they need to either file, whichever they come across first)
I'll switch to sys_matrix.h + skin_matrix.h

[AngheloAlf]

Personally I prefer if the headers were separated by their systems.
In this specific case the systems would be named after their C files, yes. That's because it happens to be organized this way. But this doesn't apply to most systems, like all the different systems present on z_actor.c.

So I agree with you dragorn that we should split headers by system, this just happens to more or less match the corresponding C files. Since there's currently no consensus on how to name the headers I think we should name them after the C files since that's the most obvious way, we can always rename them later.

Since you are concerned about usability then you could add a comment at the top of the header explaining the system a bit and how it differentiates to the other matrix header.

tl;dr: I prefer sys_matrix.h + skin_matrix.h and docs comment on each of them.

[fig02]

like I mentioned in discord, not sold on the exact split of things here. but we can get back to this this at a later time.

I would almost prefer reverting back to the generic matrix.h as our temporary solution. but whatever others want

[cadmic]

include guard name doesn't match

[cadmic]

ditto

[mzxrules]

PR #1972 also conflicts with this because I implemented my own sys_matrix.h that mimics MM's own sys_matrix.h

I'm fine with dropping the inclusion of sys_matrix.h in #1972 in favor of this pr, though I would prefer the two headers stay mostly consistent between games.

[Dragorn421]

I don't know where to take this