search

diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.08k stars 795 forks source link

Rename stores.cpp global variables #7425

Closed kphoenix137 closed 1 month ago

kphoenix137 commented 1 month ago

Precursor to https://github.com/diasurgical/devilutionX/pull/7424 to make review of this linked PR easier.

StephenCWills commented 1 month ago

I see that the time demo is failing, but I think it's failing as a result of #7417 being merged.

Timedemo isn't failing on master.

kphoenix137 commented 1 month ago

I see that the time demo is failing, but I think it's failing as a result of #7417 being merged.

Timedemo isn't failing on master.

I saw it fail running the tests locally. Not sure why it's not failing on master.

AJenbo commented 1 month ago

Maybe you can PascalCase the variables that shouldn't end up in a class and fix the plural forms of containers :)

kphoenix137 commented 1 month ago

Maybe you can PascalCase the variables that shouldn't end up in a class and fix the plural forms of containers :)

I can just correct the variable cases in the next PR I open, although it might not be productive to open a PR with just the class addition itself since that would involve a mass copy and paste of things into the class, which would put the codebase in a gross state if the refactor isn't merged soon after. I think this PR itself might be enough to make reviewing the refactor easier.

AJenbo commented 1 month ago

you already did it, as fare as i can see :)