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

Removed hardcoded skeleton types in actor draw code. #2979

Merged
merged 7 commits into from
Sep 26, 2023

Conversation

NEstelami
Copy link
Contributor

@NEstelami NEstelami commented Jun 10, 2023

This allows custom models to use whatever skeleton type they wish regardless of what the original actor used.

Build Artifacts

@NEstelami NEstelami added the do not merge Not ready or not valid changes label Jun 10, 2023
@NEstelami NEstelami changed the title [WIP] Removed hardcoded skeleton types in actor draw code. Removed hardcoded skeleton types in actor draw code. Jun 22, 2023
@NEstelami NEstelami removed the do not merge Not ready or not valid changes label Jun 22, 2023
@PurpleHato
Copy link
Member

PurpleHato commented Jun 22, 2023

To add more context, this PR is necessary to fix some issues when Custom assets are used over some NPC/Objects that would try to link themselves
image
image

@PurpleHato PurpleHato added the do not merge Not ready or not valid changes label Jun 22, 2023
@NEstelami NEstelami added do not merge Not ready or not valid changes and removed do not merge Not ready or not valid changes labels Jul 21, 2023
@NEstelami NEstelami removed the do not merge Not ready or not valid changes label Jul 21, 2023
@aMannus
Copy link
Contributor

aMannus commented Sep 13, 2023

Considering the scope of the changes in this PR, do we need to organize a testing effort for this? I assume it's good to test this without replaced models as well considering it touches a lot of the vanilla skeleton code as well?

Seems like it's also out of date currently though, so it'll need to be updated to latest develop before any testing can be done.

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

LGTM. left one comment and one question, but nothing that should stop merging. :shipit:

Comment on lines 1254 to 1256
void SkelAnime_DrawOpa(PlayState* play, void** skeleton, Vec3s* jointTable,
OverrideLimbDrawOpa overrideLimbDraw, PostLimbDrawOpa postLimbDraw, void* arg);
void SkelAnime_DrawFlexOpa(PlayState* play, void** skeleton, Vec3s* jointTable, s32 dListCount,
Copy link
Contributor

Choose a reason for hiding this comment

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

could these be moved out of functions.h now that SkelAnime_DrawSkeletonOpa exists/is being used everywhere? it seems that would mean we shouldn't need to reference those outside of z_skelanime.c anymore.

skel.skelAnime->skeleton = newSkel->skeletonData.skeletonHeader.segment;
uintptr_t skelPtr = (uintptr_t)newSkel->GetPointer();
memcpy(&skel.skelAnime->skeletonHeader, &skelPtr, sizeof(uintptr_t)); // Dumb thing that needs to be done because cast is not cooperating
Copy link
Contributor

Choose a reason for hiding this comment

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

cast is not cooperating

curious as to what that means

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's been a good while so I don't remember the specifics, but I believe trying to write that value like normal resulted in some compile-time error.

@garrettjoecox garrettjoecox merged commit ccd05d8 into HarbourMasters:develop Sep 26, 2023
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.

5 participants