ousnius / BodySlide-and-Outfit-Studio

BodySlide and Outfit Studio, a tool to convert, create, and customize outfits and bodies for Bethesda games.
GNU General Public License v3.0
286 stars 63 forks source link

Fixes for wxWidgets-gtk3 3.1.6 #456

Closed sts1skj closed 2 years ago

sts1skj commented 2 years ago
ousnius commented 2 years ago

@sts1skj wxString::ToStdString() is a wide (UTF-16) string on Windows iirc, which is not what I want (codebase expects UTF-8). Can you use wxString::ToUTF8() instead?

sts1skj commented 2 years ago

According to the wxWidgets documentation, wxString::ToStdString() returns a std::string, not a std::wstring. The function wxString::ToStdWstring() returns a std::wstring.

I picked wxString::ToStdString() in an effort to be consistent with the rest of the code. I saw many places in OutfitStudio.cpp where wxString::ToStdString() was used to convert from wxString to std::string. But there are also many places where ToUTF8().data() is used. According to the wxWidgets documentation, ToStdString uses locale encoding, while ToUTF8 uses UTF8 encoding. I don't know what the possibilities are for locale encoding, so I don't understand the difference between these two conversion methods.

One thing is clear from the wxWidgets documentation: when you convert wxString to std::string implicitly, it's the same as calling ToStdString. Since all of the code I changed was doing implicit conversion before, my replacing the implicit conversions with calls to ToStdString should not change the result.

Do you want me to use ToUTF8().data() instead of ToStdString() for the 16 changes in this PR? What about the many existing places where ToStdString() was called before this PR?

ousnius commented 2 years ago

One thing is clear from the wxWidgets documentation: when you convert wxString to std::string implicitly, it's the same as calling ToStdString. Since all of the code I changed was doing implicit conversion before, my replacing the implicit conversions with calls to ToStdString should not change the result.

In that case, the use of ToStdString() is fine for now for this PR. I wasn't fully aware of that.

In the future (separate to this PR), all occurences of ToStdString() where encoding can matter should be replaced by ToUTF8() (or utf8_str()) when converting from wxString to std::string and FromUTF8() when converting from std::string to wxString (from logic to UI) respectively.

Most of the code already does this, I did a big parse back then for Chinese file names (UTF-16 on Windows) and UTF-8 file contents (XML files) back then. It's possible there are important gaps here, though, hence it should be thoroughly looked at once more.

sts1skj commented 2 years ago

This PR only fixes the implicit conversions from wxString to std::string that were generating errors with g++-10 when wxWidgets is built without the "--enable-stl" option. Outfit Studio has many more implicit conversions in both directions between wxString and std::string. I don't know how we could find them all. There could be thousands.