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.

Sign up for GitHub

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

Jump to bottom

Cleanup LoadGameLevel() #7467

Merged
merged 2 commits into from
Oct 9, 2024
Merged

Cleanup LoadGameLevel() #7467

merged 2 commits into from
Oct 9, 2024

Conversation

kphoenix137
Copy link
Collaborator

@kphoenix137 kphoenix137 commented Oct 9, 2024 •
edited
Loading

This PR breaks up LoadGameLevel() into several functions, to make it more readable. Originally, this function is obscenely long and hard to follow. I was extremely careful to not rearrange any function calls to avoid introducing any bugs.

AJenbo
AJenbo approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@AJenbo AJenbo left a comment

Choose a reason for hiding this comment

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

Probably all of this should be moved to a different file to avoid all the LoadGame* prefixing. Also I'm not 100% happy about the functions that are named after the bool that is checked to see if it should be called or not. But still this looks like a good start.

@AJenbo AJenbo merged commit 1812479 into diasurgical:master Oct 9, 2024
23 checks passed
This was referenced Oct 9, 2024
Closed
Closed
@AJenbo
Copy link
Member

AJenbo commented Oct 9, 2024

Lol so you didn't actually leave things unchanged.

Please seperat Clean ups and bug fixes, it's hard to review when they are mixed together and it makes it hard to write the Change log later

@kphoenix137
Copy link
Collaborator Author

Lol so you didn't actually leave things unchanged.

Please seperat Clean ups and bug fixes, it's hard to review when they are mixed together and it makes it hard to write the Change log later

I'm sorry. I wrote this one up making an assumption the other PR would be merged first. I should have drafted this PR until that one was reviewed.

@kphoenix137 kphoenix137 deleted the cleanup-LoadGameLevel branch October 9, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AJenbo AJenbo AJenbo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kphoenix137 @AJenbo