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

SCUMM: Use inline member initialization #3862

Merged
merged 1 commit into from May 13, 2022
Merged

Conversation

orgads
Copy link
Contributor

@orgads orgads commented May 5, 2022

and initialize all members.

and initialize all members.
@orgads orgads requested review from athrxx and bluegr May 5, 2022
@athrxx
Copy link
Member

@athrxx athrxx commented May 5, 2022

What is the purpose of the PR? There is no description.
Initialization is good, of course, if we have cases where it is still missing...
But what about the change to inline initialization. Is it just style preference?
The problem for me is, it is basically impossible for me to tell whether you got all variables right.
So, if we do face possible regressions, it would be good to know why we're actually doing this ;-)

@orgads
Copy link
Contributor Author

@orgads orgads commented May 5, 2022

I wrote a script for this, so the values are all copied from the cpp file.

The main purpose is to prevent missing initializations, and it is also more convenient to have the initializations along with declaration.

@orgads
Copy link
Contributor Author

@orgads orgads commented May 12, 2022

ping

@bluegr
Copy link
Member

@bluegr bluegr commented May 13, 2022

Overall, it's good to have variable declarations together with their initialization. Now that we're using C++ 11, it's straightforward to use such syntax.

Thanks for your work! Looks good. Merging

@bluegr bluegr merged commit 3231a6e into scummvm:master May 13, 2022
8 checks passed
@orgads orgads deleted the scumm-inline-init branch May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants