caseif / SkyMM-NX

Simple mod manager for Skyrim: Switch Edition.
MIT License
16 stars 4 forks source link

ini_helper.cpp not writing the correct suffixed bsa's to Skyrim.ini #14

Open SundayReds opened 3 years ago

SundayReds commented 3 years ago

I found this out when I was tweaking the application and I thought you might want to know.

Expected: According to Skyrim-NX-Toolkit, under Skyrim.ini 's sResourceArchiveList=, only meshes, animations, and sounds-suffixed bsa's are needed along with the non-suffixed bsa's

Actual: Almost everything is added. I've found - Voices.bsa, - Textures.bsa, and - Animations.bsa in mine. EDIT: Apologies, - Animations is supposed to be there, but the other two aren't

The funny thing is that the voices, textures and animations are getting added properly to their other locations in Skyrim.ini and Skyrim_en.ini, and they all rely on the same writeFileList() function in ini_help.cpp to get it done.

The g_archive_types_1 passed however seems to be defined properly (it only contain meshes, animations, sounds and empty suffixes), so honestly I'm really stumped at why this is happening without tracing out the entire code in detail. Maybe you could take a look.

KP2048 commented 3 years ago

Are voices and sounds the same?

KP2048 commented 3 years ago

Ah it should be sounds not voices?

KP2048 commented 3 years ago

It seems 2 and 3 are being put into 1 for some reason?

KP2048 commented 3 years ago

What version are you using? I haven’t had this issue ever

KP2048 commented 3 years ago

Oh! the g_archive_types_1 has a “” in it, maybe it’s putting everything in it because all the bsa files fall under it?

SundayReds commented 3 years ago

@Witherking25 you're absolutely right. I was thinking the same thing last night since that seems to be the only abnormality among the three lists, and i only just had time to test it out on repl it.

the problem is with the following line - specifically, the std::string::find() used to compare suffixes:

https://github.com/caseif/SkyMM-NX/blob/5674a29eabb0017c6c41267c913d493ff0af9817/src/ini_helper.cpp#L205

Here's the sandboxed code where it returns a wrong result when compared against an empty suffix: third case. I think a simple fix would be to use the == operator to compare the suffixes. ie. if (suffix_pair.first == expected_suffix) {

There's another line within the same file that concerns me as well:

https://github.com/caseif/SkyMM-NX/blob/5674a29eabb0017c6c41267c913d493ff0af9817/src/ini_helper.cpp#L141

From cppreference, this is the description for std::string:find_last_of(const basic_string& str, size_type pos = npos):

Finds the last character equal to one of characters in the given character sequence. Return Value: Position of the found character or npos if no such character is found.

It seems to me that the intended purpose is to compare if the strings are equal, which is not the same as the above function and could possible give much more false positives (and probably some false negatives as well) than intended.

@caseif : are you able to confirm this?

KP2048 commented 3 years ago

i can confirm that replacing both of those with == fixes the issue

KP2048 commented 3 years ago

11 fixes this