Conversation
…for the config which needs a different compiler)
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the ZMatrix screensaver project to support modern Windows environments and multiple screen configurations. The primary purpose is to modernize the build infrastructure and enable compilation with current development tools.
- Updates Visual Studio project files to use newer toolsets and target Windows 10
- Replaces references from Config.dll to ConfigModern.dll throughout the codebase
- Removes deprecated Windows 7 compatibility code and WorkerW window handling
Reviewed Changes
Copilot reviewed 17 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| zsMatrix/zsMatrix.vcxproj | Updates Windows target platform and toolset versions |
| matrix.vcxproj | Updates Windows target platform and toolset versions |
| matrix.sln | Upgrades Visual Studio version and adds x64 platform configurations |
| matrix.cpp | Removes Windows 7 compatibility code and updates DLL references |
| globals.cpp | Updates all Config.dll references to ConfigModern.dll |
| Config/ConfigModern.cbproj | New Borland C++ Builder project file for modern compiler support |
| Config/ConfigForm.cpp | Fixes font pitch enum references and adds string conversion utility |
| Config/CharSetForm.cpp | Implements wide string to char pointer conversion for file operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| void ConvertWideStringToCharPointer(const std::wstring& wstr, const char* charPointer) { | ||
| wstring your_wchar_in_ws(wstr); | ||
| string your_wchar_in_str(your_wchar_in_ws.begin(), your_wchar_in_ws.end()); | ||
| charPointer = your_wchar_in_str.c_str(); |
There was a problem hiding this comment.
This function has a fundamental flaw - it assigns a local variable to the parameter, which doesn't modify the caller's pointer. The charPointer parameter should be a reference (const char*&) or the function should return const char*. Additionally, the string conversion may lose data for non-ASCII characters.
| void ConvertWideStringToCharPointer(const std::wstring& wstr, const char* charPointer) { | |
| wstring your_wchar_in_ws(wstr); | |
| string your_wchar_in_str(your_wchar_in_ws.begin(), your_wchar_in_ws.end()); | |
| charPointer = your_wchar_in_str.c_str(); | |
| // Converts a std::wstring to a UTF-8 encoded std::string | |
| std::string ConvertWideStringToString(const std::wstring& wstr) { | |
| #ifdef _WIN32 | |
| if (wstr.empty()) return std::string(); | |
| int size_needed = WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), NULL, 0, NULL, NULL); | |
| std::string strTo(size_needed, 0); | |
| WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), &strTo[0], size_needed, NULL, NULL); | |
| return strTo; | |
| #else | |
| // Fallback: lossy conversion for non-Windows platforms | |
| std::string str(wstr.begin(), wstr.end()); | |
| return str; | |
| #endif |
| const char* tempCharFileName; | ||
| std::wstring wideFileName = this->OpenCharSetDialog->FileName.c_str(); | ||
| ConvertWideStringToCharPointer(wideFileName,tempCharFileName); | ||
| FILE *InFile = fopen(tempCharFileName,"rt"); |
There was a problem hiding this comment.
The tempCharFileName pointer will be uninitialized after calling ConvertWideStringToCharPointer due to the bug in that function. This will likely cause a crash when passed to fopen. Use _wfopen instead to handle wide character filenames properly.
| const char* tempCharFileName; | |
| std::wstring wideFileName = this->OpenCharSetDialog->FileName.c_str(); | |
| ConvertWideStringToCharPointer(wideFileName,tempCharFileName); | |
| FILE *InFile = fopen(tempCharFileName,"rt"); | |
| std::wstring wideFileName = this->OpenCharSetDialog->FileName.c_str(); | |
| FILE *InFile = _wfopen(wideFileName.c_str(), L"rt"); |
| #include <vector> | ||
| #include <deque> | ||
| #include <algorithm> | ||
| //#include <algorithm> |
There was a problem hiding this comment.
The algorithm header is commented out but may be needed elsewhere in the codebase. Verify that no code depends on standard algorithms before removing this include completely.
| //#include <algorithm> | |
| #include <algorithm> |
| int ConvertAnsiStringToSpecialStrings(std::vector<tstring> &SpecialStrings,const AnsiString &InString, _TCHAR Delimiter); | ||
| int ConvertWideStringToSpecialStrings(std::vector<tstring> &SpecialStrings,const WideString &InString, _TCHAR Delimiter); | ||
|
|
||
| void ConvertWideStringToCharPointer(const std::wstring& wstr, const char* charPointer); |
There was a problem hiding this comment.
The function signature is incorrect for the intended purpose. It should either return const char* or take const char*& as parameter to properly return the converted string to the caller.
| void ConvertWideStringToCharPointer(const std::wstring& wstr, const char* charPointer); | |
| std::string ConvertWideStringToCharPointer(const std::wstring& wstr); |
Issue does the following: