View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002814 | FSSCP | Pilot data | public | 2013-03-18 14:18 | 2013-03-30 21:26 |
Reporter | MjnMixael | Assigned To | niffiwan | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Platform | PC | OS | Windows | OS Version | Win7 |
Product Version | 3.6.19 | ||||
Target Version | 3.7.0 | ||||
Summary | 0002814: FSO does not delete .csg files when deleting pilots | ||||
Description | Pretty straightforward, this one. FSO is not deleting .csg files related to pilots when those pilots are deleted in the pilot select screen. | ||||
Steps To Reproduce | 1) Create a new pilot. Load a campaign or 2 or 3 to ensure multiple .csg files exists. May need to do this across a couple mods. 2) At the Pilot Select screen, delete that pilot. 3) Exit FSO and check your data/players folder. You'll see that pilot's .plr file is gone, but many PILOT.CAMPAIGN.csg files remain. | ||||
Tags | No tags attached. | ||||
|
I wonder if it deletes the currently active campaign .csg, or if it doesn't delete any. Regardless, should be a fairly simple fix. |
|
I think that the problem is that the function that FSO uses to find all the .csg files ignores any files that are longer than 31 characters (i.e. >= MAX_FILENAME_LEN). i.e. using "tbd" as a pilot name resulted in all .csg files being removed. Using "quitealongname1234567890" as the pilot name resulted in none of the .csg files being deleted. Not sure of the best way to fix this issue, as the function that finds the .csg files is "cf_get_file_list" and that's used for heaps of other stuff as well. Maybe give the flag an extra param to control its behaviour. Or write a new find-the-csg-files function. Or change the names assigned to .csg files :) I'll have to give it some more thought. (note: looks like this problem predates the new pilot code) |
|
I took a quick look and it seems that there are a few different file lengths used. One area uses _MAX_FNAME which is 256 characters, another uses MAX_PATH_LEN at 128. Then there is one that uses MAX_FILENAME_LEN + 2 for 34 characters (probably the one you were looking at). Looks like there needs to be some uniformity to what the size can be. Also just noticed there is a second cf_get_file_list function that appears to take a 128 length (brain is still not up to par). Could the call just be to the wrong function? |
|
There are two cf_get_file_list functions, one puts the filenames into a SCP_vector, the other uses a C-style array. Unfortunately, both do the same ignore-more-than-31-char-files thing... if ( strlen(find.name) >= MAX_FILENAME_LEN ) continue; Re: the 128 length, were you referring to this? char filespec[MAX_PATH_LEN]; |
|
Highly possible. The thing I can't help wondering about this is how the game can read the list for use if it can't read it for deleting. Seems it should be able to delete in the same way it converts, creates, loads unless it's deleting the pilot file before it tries to delete the .csg files and its stored in there. |
|
tl;dr I might have a patch ready tonight Eh - those functions just make a choice to ignore those files. i.e. they both use OS calls to read the files from disk, those calls return all the files matching the specification (in this case, PILOT.*.csg), and then FSO tosses out the ones that are too long. I suspect that the pilot conversion process will have the same issue because it uses the same function call (cf_get_file_list). I think I'm going to fix this by removing the filename length check from the SCP_vector version of cf_get_file_list. This is only used in a few places, all to do with the new pilot code. Once I've removed the check, I'll change "mission_campaign_delete_all_savefiles" to use the SCP_vector version. Regarding the removal of the filename length check, I can't see this being a problem. With an array you could overrun the array if you're not limiting the length of the filename, but not with a vector. And the array version of cf_get_file_list gets around this anyway by dynamically allocating the memory required to hold the string. So it'd perhaps come back to a memory use issue, and from what I can find the max filename length of NTFS/ext4/HPFS+ is 255 chars, so no big deal there. |
|
Can you please try this patch? It should remove all .csg files (no matter how long) for a pilot. |
|
Tested and it didn't work. Still deleted the .plr, but not any .csg files. |
|
Hmmmm. Did you delete the pilot from the initial selection screen, or from the barracks? |
|
Did both just to be sure. |
|
Well, I'm confused. I double checked on my PC and the patch does remove all .csg files. I've double checked the code for all places where the string ".csg" exists and there's nowhere else that could delete the campaign files. So, I guess that the #if defined _WIN32 section in cf_get_file_list must behave differently to the Linux/OSX section and therefore I might need someone else to help resolve this. FYI, I've been mostly testing with ST:R (since it has effectively 5 campaigns available) One more request though, with a plain trunk build, can you check if the .csg files are or are not deleted when a pilot has a name only 1 character long? |
|
It didn't even remove 'd.str.csg'. |
|
I found a difference between the two versions of cf_get_file_list. The "temp" patch I attached *may* fix the issue since it makes the SCP_string version more like the other version, and I'm assuming that this isn't broken in the other version... (but I can't test/check it, so I might be completely wrong) edit: nope - tested by chief1983 and that didn't work... |
|
mantis2814-mk2-svn.patch (2,390 bytes)
Index: code/mission/missioncampaign.cpp =================================================================== --- code/mission/missioncampaign.cpp (revision 9612) +++ code/mission/missioncampaign.cpp (working copy) @@ -814,9 +814,10 @@ void mission_campaign_delete_all_savefiles( char *pilot_name ) { int dir_type, num_files, i; - char *names[MAX_CAMPAIGNS], file_spec[MAX_FILENAME_LEN + 2], *ext; + char file_spec[MAX_FILENAME_LEN + 2], *ext; char filename[1024]; int (*filter_save)(char *filename); + SCP_vector<SCP_string> names; ext = NOX(".csg"); dir_type = CF_TYPE_PLAYERS; @@ -827,14 +828,13 @@ // be. I have to save any file filters filter_save = Get_file_list_filter; Get_file_list_filter = NULL; - num_files = cf_get_file_list(MAX_CAMPAIGNS, names, dir_type, file_spec); + num_files = cf_get_file_list(names, dir_type, const_cast<char *>(file_spec)); Get_file_list_filter = filter_save; for (i=0; i<num_files; i++) { - strcpy_s(filename, names[i]); + strcpy_s(filename, names[i].c_str()); strcat_s(filename, ext); cf_delete(filename, dir_type); - vm_free(names[i]); } } Index: code/cfile/cfilesystem.cpp =================================================================== --- code/cfile/cfilesystem.cpp (revision 9612) +++ code/cfile/cfilesystem.cpp (working copy) @@ -1447,12 +1447,9 @@ find_handle = _findfirst( filespec, &find ); if (find_handle != -1) { do { - if (strcmp(strstr(filter, "."), strstr(find.name,".")) != 0) + if (strcmp(strrchr(filter, '.'), strrchr(find.name,'.')) != 0) continue; - if ( strlen(find.name) >= MAX_FILENAME_LEN ) - continue; - if (!(find.attrib & _A_SUBDIR)) { if ( !Get_file_list_filter || (*Get_file_list_filter)(find.name) ) { if ( check_duplicates && cf_file_already_in_list(list, find.name) ) { @@ -1488,9 +1485,6 @@ dirp = opendir (filespec); if ( dirp ) { while ((dir = readdir (dirp)) != NULL) { - if ( strlen(dir->d_name) >= MAX_FILENAME_LEN ) { - continue; - } if (fnmatch(filter, dir->d_name, 0) != 0) continue; @@ -1650,7 +1644,7 @@ if (num_files >= max) break; - if (strcmp(strstr(filter, "."), strstr(find.name,".")) != 0) + if (strcmp(strrchr(filter, '.'), strrchr(find.name,'.')) != 0) continue; if ( strlen(find.name) >= MAX_FILENAME_LEN ) |
|
I think I've found the issue in windows. It was assuming that there was only a single dot in all filenames. I've made some minor changes to check for the last dot (not the 1st) in the file checks. Can you please give the latest patch a try & let me know if it works? |
|
That patch totally fixes it. Nice work. |
|
Fix committed to trunk@9613. |
fs2open: trunk r9613 2013-03-30 18:13 Ported: N/A Details Diff |
Fix for mantis 2814: don't check filename lengths when deleting .csg files Also change win32 to check for last . in filenames, not the 1st |
Affected Issues 0002814 |
|
mod - /trunk/fs2_open/code/cfile/cfilesystem.cpp | Diff File | ||
mod - /trunk/fs2_open/code/mission/missioncampaign.cpp | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-03-18 14:18 | MjnMixael | New Issue | |
2013-03-18 20:31 | niffiwan | Note Added: 0014782 | |
2013-03-18 20:31 | niffiwan | Assigned To | => niffiwan |
2013-03-18 20:31 | niffiwan | Status | new => assigned |
2013-03-19 11:13 | niffiwan | Note Added: 0014784 | |
2013-03-20 08:12 | FUBAR-BDHR | Note Added: 0014785 | |
2013-03-20 08:24 | FUBAR-BDHR | Note Edited: 0014785 | |
2013-03-20 09:20 | niffiwan | Note Added: 0014786 | |
2013-03-20 17:01 | FUBAR-BDHR | Note Added: 0014787 | |
2013-03-20 20:45 | niffiwan | Note Added: 0014788 | |
2013-03-21 08:16 | niffiwan | File Added: mantis2814-svn.patch | |
2013-03-21 08:28 | niffiwan | Note Added: 0014792 | |
2013-03-21 08:28 | niffiwan | Status | assigned => feedback |
2013-03-22 19:23 | MjnMixael | Note Added: 0014800 | |
2013-03-22 19:23 | MjnMixael | Status | feedback => assigned |
2013-03-22 22:14 | niffiwan | Note Added: 0014801 | |
2013-03-22 23:13 | MjnMixael | Note Added: 0014802 | |
2013-03-23 10:54 | niffiwan | Note Added: 0014814 | |
2013-03-23 14:15 | MjnMixael | Note Added: 0014815 | |
2013-03-24 01:41 | niffiwan | File Added: mantis2814-temp-svn.patch | |
2013-03-24 01:47 | niffiwan | Note Added: 0014817 | |
2013-03-24 01:49 | niffiwan | File Deleted: mantis2814-temp-svn.patch | |
2013-03-24 01:49 | niffiwan | File Added: mantis2814-temp-svn.patch | |
2013-03-24 03:20 | niffiwan | Note Edited: 0014817 | |
2013-03-24 03:20 | niffiwan | File Deleted: mantis2814-temp-svn.patch | |
2013-03-30 09:21 | niffiwan | File Added: mantis2814-mk2-svn.patch | |
2013-03-30 09:22 | niffiwan | Note Added: 0014861 | |
2013-03-30 09:22 | niffiwan | Status | assigned => feedback |
2013-03-30 09:22 | niffiwan | File Deleted: mantis2814-svn.patch | |
2013-03-30 09:28 | niffiwan | Note Edited: 0014861 | |
2013-03-30 15:01 | MjnMixael | Note Added: 0014862 | |
2013-03-30 15:01 | MjnMixael | Status | feedback => assigned |
2013-03-30 21:26 | niffiwan | Changeset attached | => fs2open trunk r9613 |
2013-03-30 21:26 | niffiwan | Note Added: 0014863 | |
2013-03-30 21:26 | niffiwan | Status | assigned => resolved |
2013-03-30 21:26 | niffiwan | Resolution | open => fixed |