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

New iniReader implementation #409

Merged
merged 6 commits into from
Jan 17, 2023
Merged

New iniReader implementation #409

merged 6 commits into from
Jan 17, 2023

Conversation

nipkownix
Copy link
Owner

@nipkownix nipkownix commented Dec 28, 2022

@emoose This should fix part of #405, as mentioned here: #405 (comment)

Figured it was time we made our own .ini reader to fit our needs instead of fighting the existing one.
This is a bit simplified if compared to the original one, but I only focused on what we are actually using for now.

Our extended CmdIniReader is now part of this too, so everything feels a bit cleaner.

Never tried doing this before, so I figured I should get your input before a potential merge. Everything seems to be working fine, though.

Edit: Was looking for a way to stop using WritePrivateProfileString since that is terribly slow, outdated, unsupported, and the only reason we had a separate thread to write settings. Came across simpleini, which can write back the .ini preserving comments and doesn't touch any of the outdated xxPrivateProfileString funcs. Seems pretty quick as well, so I removed the thread stuff we were using.

Should probably triple-check if everything is saving fine, though ._.

@emoose
Copy link
Collaborator

emoose commented Jan 17, 2023

Seems to work fine for me, settings + trainer stuff seemed to save fine at least, and cmd line args all looked okay, debug tool stuff like area jump still saved fine also.

I noticed this branch mentions "New refresh rate = 70" in console (I'm still using the OC'd monitor stuff I mentioned at #419 (comment)) - not sure if that means this is actually using that mode to display though, but IIRC I didn't see that message with the other branch.

@nipkownix
Copy link
Owner Author

Ah, nice! Many thanks for taking a look.
I suppose there were no hiccups when saving settings? Guess we should be good to merge it.
Will most likely have to fix the other PRs though.. :p

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.

2 participants