YimMenu icon indicating copy to clipboard operation
YimMenu copied to clipboard

[Improvement]: Settings not saved to disk are not settings

Open xiaoxiao921 opened this issue 1 year ago • 4 comments

Which feature

https://github.com/YimMenu/YimMenu/commit/4589b87553320d7065be68c72089ef1a60e8c9b9#diff-e5195cb55065018a1152666952dff1dfc77cafd05cd313b57a4bfd2439e8792fR474

Reason

And no, dodging a include is not a valid reason.

Additional context

No response

xiaoxiao921 avatar Jun 27 '24 12:06 xiaoxiao921

The user is not going to see them, but these kind of variables that don't have to be serialized just has no business to be there, in all cases I've seen they can just be declared as just global variable somewhere in a header file or sometimes just in a single compilation cpp unit and not be exposed anywhere else. Most of the time they are not even setting and are just for keeping state across some function calls. The worst offender of all is that whenever the god settings class is modified most if not all of the cpp compilation unit have to be recompiled due to the shift in the settings struct field offsets. Killing all the purpose of incremental compilations. Keeping the fields in there with only the absolute necessary and not putting global variables that are completly unrelated will obviously help with this problem

xiaoxiao921 avatar Jul 04 '24 13:07 xiaoxiao921

The worst offender of all is that whenever the god settings class is modified most if not all of the cpp compilation unit have to be recompiled due to the shift in the settings struct field offsets. Killing all the purpose of incremental compilations. Keeping the fields in there with only the absolute necessary and not putting global variables that are completly unrelated will obviously help with this problem

instead of one big file divide them into separate files. In my fork i have done as follow -

src\core\settings\*.* includes all files that has the configerations to be saved on the disk. src\core\settings.hpp includes all files from above and either save them to or load them from one single settings.json src\core\data\*.* includes all files that need game only state.

Each of the files in src\core\settings\*.* & src\core\data\*.* serves one domain like src\core\data\weapons.hpp will include state related to weapons only. Any changes done to these files will only recompile the files which import them. Here src\core\settings.hpp is only imported by src\main.cpp to load settings from disk in starting and inlialize the state in each of the files in src\core\settings\*.*.

The only thing I dont like about the compilation is that if you save a file without doing any changes (from git point of view) the compiler thinks that the file was changed and it will recompile all the files that depends on it.

lonelybud avatar Jul 04 '24 14:07 lonelybud