Skip to content

Conversation

@dtugend
Copy link
Contributor

@dtugend dtugend commented Aug 24, 2014

This is merely to keep the VC++ Debug builds happy and addresses cases
where variables were being used without being initialized. There should be
no visible changes apart from non-functional side effects to the users of
Release builds.
Thanks to @fabiosarts for finding and reporting these problems.
Thanks to @LevShisterov for suggesting how to fix geiger.cpp files, which I
followed closely except using <= 800 instead of < 800.

This is merely to keep the VC++ Debug builds happy and addresses cases
where variables where being used without being initalized. There should be
no visible changes apart from non-functional side effects to the users of
Release builds.
Thanks to @fabiosarts for finding and reporting these problems.
Thanks to @LevShisterov for suggeting how to fix geiger.cpp files, which I
followed closely except using <= 800 instead of < 800.
alfred-valve added a commit that referenced this pull request Aug 25, 2014
@alfred-valve alfred-valve merged commit 0a1528c into ValveSoftware:master Aug 25, 2014
@alfred-valve
Copy link
Contributor

Looks good

@elnurvl
Copy link

elnurvl commented May 21, 2025

Hi @dtugend , not sure if there was a good reason to not initialize the variables for all conditions back then, but the debug build crashes when a new level message appears on screen. I have created #3925 to initialize the variables as soon as they are created so it is not exclusive for some conditions. Let me know if there is anything wrong with doing that.

I could not reopen #1536 , but the issue has not yet been fully resolved.

@dtugend
Copy link
Contributor Author

dtugend commented May 21, 2025

Hi @dtugend , not sure if there was a good reason to not initialize the variables for all conditions back then, but the debug build crashes when a new level message appears on screen. I have created #3925 to initialize the variables as soon as they are created so it is not exclusive for some conditions. Let me know if there is anything wrong with doing that.

I could not reopen #1536 , but the issue has not yet been fully resolved.

@elnurvl You are blaming this on the wrong person, if you make effort to read the changes of this pull request, you'll see it was fixed, including the geiger.

They basically reverted this (and potentially others) with this commit:
0fc8913

0fc8913#diff-9203423ac932f9aa4e788c44e510c924891af4ba2f6453abdf028a24add6d412L71-L74

0fc8913#diff-44162cb21d9bc2023c9771f9b6631ad7b9a112e4bc98633d7b5fb442ee59dc46L150

Maybe they had reasons, maybe not I don't know.

@dtugend dtugend mentioned this pull request May 21, 2025
@elnurvl
Copy link

elnurvl commented May 21, 2025

@dtugend , sorry, my bad. I had no intention of blaming anyone, just was curious on what I am missing. You are right, I see you also proposed the same fix here and it got merged before being reverted for some reason. Maybe it was the closing of the referred issue made me jumping on a conclusion without checking the change logs.

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.

3 participants