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

Additional labeling and clarifying of Goddard code, particularly renderer.c #487

Open
wants to merge 6 commits into
base: develop/3.0.0
Choose a base branch
from

Conversation

Tubular-Bells
Copy link
Contributor

This pull request renames many Goddard functions and variables to be more descriptive. A few functions and variables do not have a discernible function, as they are only used as part of incomplete, unused functions. While I could have left these as-is, I took the liberty of giving them names anyway, if only for improving the human readability of the code. I have also added more UNUSED tags where appropriate. Lastly, some comments were left explaining the functionality of previously undocumented functions, at least where I felt reasonably confident I understood what the function was doing.

No changes to the behavior of the Goddard code have been made. In the case of gd_dl_rot_matrix (previously func_8019F2C4), minor changes to how the function uses its arguments have been made for the sake of clarity. gd_dl_rot_matrix is only called during a specific switch statement in the draw_shape function in draw_objects.c. Previously, the values 120, 121, and 122 were passed to represent the X, Y, and Z axes, respectively. Confusingly, 120 was then subtracted from this passed value in the function, meaning that it was essentially passing 0, 1, and 2 with extra steps. This change should have no effect on the behavior of the program.

This is by no means a comprehensive PR. Hopefully, further labeling will follow.

@gheskett gheskett added the documentation Improvements or additions to documentation label Sep 21, 2022
@gheskett gheskett added this to the 2.1 milestone Sep 21, 2022
@gheskett gheskett removed the request for review from Mr-Wiseguy September 29, 2022 03:49
Copy link
Collaborator

@gheskett gheskett left a comment

Choose a reason for hiding this comment

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

These seem like good changes, obviously this code still needs a lot more cleanup but this is a good start.

src/goddard/renderer.c Outdated Show resolved Hide resolved
src/goddard/renderer.c Outdated Show resolved Hide resolved
src/goddard/renderer.c Outdated Show resolved Hide resolved
@gheskett gheskett modified the milestones: 2.1, 3.1 Mar 6, 2023
@gheskett gheskett changed the base branch from develop/2.1.0 to develop/3.0.0 September 27, 2023 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

3 participants