-
Notifications
You must be signed in to change notification settings - Fork 2
SG-39031 comment updated for outdated reference #223
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
base: master
Are you sure you want to change the base?
SG-39031 comment updated for outdated reference #223
Conversation
@eduardoChaucaGallegos can you explain this a little more why we're removing this? Is it patched in tk-core? If so, can you point to where this is patched and is effectively and no-op. Thanks |
@staceyoue, sure! The reason we're removing this code is that QTextCodec.setCodecForCStrings() was deprecated and removed in Qt 5. According to the official Qt documentation on source breaks: Reference: https://doc.qt.io/archives/qt-5.15/sourcebreaks.html#changes-to-qtextcodec "The QTextCodec::codecForCStrings() and QTextCodec::setCodecForCStrings() functions have been removed. This is because Qt now assumes that all C-strings are encoded in UTF-8." In Qt 5 and later (which includes PySide2 and PySide6), all strings are UTF-8 by default, so explicitly setting the codec is no longer necessary or supported. Regarding the patch in tk-core, yes, there is a patcher that handles this: https://github.com/shotgunsoftware/tk-core/blob/master/python/tank/util/pyside2_patcher.py#L76 As you can see in the code, the patched function effectively does nothing. This patcher will be addressed and removed in a separate ticket where we'll analyze all the necessary considerations to avoid any backward compatibility issues. Since we're using PySide2/Qt5+, these methods don't exist anymore, and the original code in our engine was redundant because: It's trying to call methods that don't exist in PySide2/Qt5+ |
@eduardoChaucaGallegos ok makes sense, thanks for this level of detail |
Description
This pull request makes a minor update to the
post_qt_initmethod inengine.pyto clarify the behavior ofQTextCodec.setCodecForCStrings()when using PySide2/PySide6. The comment now explains that the method is effectively a no-op on these versions due to a patched stub. No functional code changes were made.