Skip to content

Conversation

@someone2639
Copy link
Collaborator

@someone2639 someone2639 commented Aug 27, 2024

camera.c 11148 -> 3178 2441 lines

See README for details on file structure

TODO's for the PR:
Decide on whether it's worth adding a file for camera shake and camera sound (Decided not worth by popular vote)

@gheskett gheskett added enhancement New feature or request cleanup Removal of useless or bloat code/features labels Aug 28, 2024
@gheskett gheskett added this to the 3.0 milestone Aug 28, 2024
@gheskett gheskett added the monkaS monkaS label Aug 28, 2024
@someone2639
Copy link
Collaborator Author

I believe this PR is ready for initial review. As always, the README has the intended file structure in case there are comments on that.

Things to consider:

  • Many enums and structs are still in camera.h, due to being a bit involved to separate into their uses (it would pollute the PR's changed files too)
  • The functions in Image 1 might warrant a new file, camera_sound.c
  • The functions in Image 2 might warrant a new file, camera_shake.c, or integration into the existing camera_math.c
  • I have put a function only used by the behind_mario mode in its respective file, but it might be worth placing it in camera_math.c instead.

Image 1:
image

Image 2:
image

@someone2639 someone2639 marked this pull request as ready for review August 29, 2024 17:57
@someone2639 someone2639 requested a review from gheskett as a code owner August 29, 2024 17:57
@someone2639 someone2639 changed the title Split camera.c (Tracking) Split camera.c Aug 29, 2024
@arthurtilly
Copy link
Collaborator

i see a TODO in the PR description so either do it or update the description if its outdated...

@someone2639
Copy link
Collaborator Author

updated

@@ -0,0 +1,19 @@
# Camera

The camera system in SM64 is notoriously complex, with the original file exceeding 11000 lines of code. HackerSM64 separates the camera code into more relevant and granular files. An index of changes is provided below.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure how I feel about a readme in the middle of source files lol

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

tbf this is a big enough change to warrant one (at the very least to prevent some where camera issues from people who dont use the server)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated to remove this; it's still in the history so I can put it into the wiki probably


return cutscene;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

is all this crap just moved here from camera.c?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This seemed like a better place for it within the existing files than camera.c itself, though if a custom file can hold all this, then I can do that

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah thats fine, just was unclear

@farisawan-2000 farisawan-2000 requested a review from Reonu as a code owner April 23, 2025 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Removal of useless or bloat code/features enhancement New feature or request monkaS monkaS

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

3 participants