-
Notifications
You must be signed in to change notification settings - Fork 269
Remove ddio_MakePath() function usage from code #714
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: main
Are you sure you want to change the base?
Remove ddio_MakePath() function usage from code #714
Conversation
Fix comparison type mismatches.
Change objhandle and ithandle to int32_t as all referrers assigns and compares int types.
# Conflicts: # Descent3/ambient.cpp
Change signature of local GetFileNameFromPlayerAndID() and related functions, minor fixes.`
Simplify code, remove ddio_SplitPath() usage.
fd0b883 to
1ac163f
Compare
|
|
||
| // Set output size to minimum of trimmed string length and buffer size minus 1 | ||
| out_size = (end - src) < destlen - 1 ? (end - src) : destlen - 1; | ||
| out_size = static_cast<size_t>(end - src) < destlen - 1 ? (end - src) : destlen - 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider providing the exact compiler warning message in the commit log so later onlookers can establish that the chosen solution was indeed the right choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few forward-compatibility suggestions for cpp20
| LOG_INFO.printf("Compiling: %s[%s] to %d", AddOnDataTables[tf].Addon_tracklocks[i].name, | ||
| (curr_tablefile == 1) ? TableFilename : AddOnDataTables[curr_tablefile - 2].AddOnTableFilename, | ||
| page_pos); | ||
| (curr_tablefile == 1) ? TableFilename.u8string().c_str() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| (curr_tablefile == 1) ? TableFilename.u8string().c_str() | |
| (curr_tablefile == 1) ? (const char*)TableFilename.u8string().c_str() |
(cf #712)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everyone should strive to reduce casts, not add more ones, especially not the old C-style cast. C++ has four much better ones. (hence #712)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why we need const char * cast here, .c_str() already have const CharT* qualifier in retern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With C++20 and later, .c_str() on a std::u8string returns const char8_t*, so we need a cast to get a const char* here to be compatible with later C++ standards (see error #706 (comment)), either the C-style one or C++-style as suggested by @jengelh / #712
| rend_Flip(); | ||
|
|
||
| int ret = zfile.ExtractFile(ze, output_filename); | ||
| int ret = zfile.ExtractFile(ze, output_filename.u8string().c_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| int ret = zfile.ExtractFile(ze, output_filename.u8string().c_str()); | |
| int ret = zfile.ExtractFile(ze, (const char*)output_filename.u8string().c_str()); |
| if (strnicmp(szcrc, file + (strlen(file) - 9), 9) != 0) { | ||
| LOG_WARNING.printf("Bad CRC on file %s! It must be corrupt! File will not be used, and is being deleted!", p); | ||
| ddio_DeleteFile(p); | ||
| std::vector<std::string> parts = StringSplit(p.stem().u8string(), "_"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| std::vector<std::string> parts = StringSplit(p.stem().u8string(), "_"); | |
| std::vector<std::string> parts = StringSplit(p.stem().string(), "_"); |
or use std::u8string? C++20 strings are quite a ride... https://stackoverflow.com/questions/55556200/convert-between-stdu8string-and-stdstring
| else | ||
| strcpy(newfile, file); | ||
| std::filesystem::path FixFilenameCase(const std::filesystem::path &filename) { | ||
| std::string local_file = filename.filename().u8string(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| std::string local_file = filename.filename().u8string(); | |
| std::string local_file = filename.filename().string(); |
or use std::u8string
Pull Request Type
Description
Remove
ddio_MakePath()from Descent3 code. Reduceddio_SplitPath()usage.ddio_MakePath()is still in use in Editor, cleanup will be finished in next run.Related Issues
Screenshots (if applicable)
Checklist
Additional Comments