View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002832 | FSSCP | Pilot data | public | 2013-04-03 10:07 | 2013-04-06 23:58 |
| Reporter | niffiwan | Assigned To | niffiwan | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.6.19 | ||||
| Target Version | 3.7.0 | ||||
| Summary | 0002832: pilotfile conversion doesn't export correct PLR/CSG versions | ||||
| Description | As per the summary, these are currently set to version 0 for both. Therefore the pilot conversion doesn't create up to date pilots. The only tricky thing about solving this is the need to keep most of the pilot conversion constants quarantined from the rest of FSO. | ||||
| Steps To Reproduce | Convert any pilot and examine the PLR and CSG with a hex editor. Of just look at the code: void pilotfile_convert::plr_export() { Assert( cfp != NULL ); // header and version cfwrite_int(0x5f524c50, cfp); cfwrite_ubyte(0, cfp); void pilotfile_convert::csg_export() { Assert( cfp != NULL ); // header and version cfwrite_int(0x5f475343, cfp); cfwrite_ubyte(0, cfp); | ||||
| Additional Information | Note that a workaround is to simply select the converted pilot, enter the mainhall (not 100% sure this is required) then exit FSO without doing anything else. FSO then writes the correct file versions. However, if you do anything else then features added to the pilotfiles since version 0 won't work properly. | ||||
| Tags | No tags attached. | ||||
|
|
oh goddammit. I thought I had caught all the version writing things. Guess I was wrong. |
|
|
mantis2832-svn.patch (3,885 bytes)
Index: code/pilotfile/csg.cpp
===================================================================
--- code/pilotfile/csg.cpp (revision 9621)
+++ code/pilotfile/csg.cpp (working copy)
@@ -25,17 +25,7 @@
#include <sstream>
-static const unsigned int CSG_FILE_ID = 0x5f475343; // "CSG_" in file
-// NOTE: Version should be bumped only for adding/removing sections or section
-// content. It should *NOT* be bumped for limit bumps or anything of
-// that sort!
-//
-// 0 - initial version
-// 1 - re-add recent missions
-static const ubyte CSG_VERSION = 1;
-
-
void pilotfile::csg_read_flags()
{
// tips?
Index: code/pilotfile/csg_convert.cpp
===================================================================
--- code/pilotfile/csg_convert.cpp (revision 9621)
+++ code/pilotfile/csg_convert.cpp (working copy)
@@ -1093,8 +1093,8 @@
Assert( cfp != NULL );
// header and version
- cfwrite_int(0x5f475343, cfp);
- cfwrite_ubyte(0, cfp);
+ cfwrite_int(CSG_FILE_ID, cfp);
+ cfwrite_ubyte(CSG_VERSION, cfp);
// flags and info sections go first
csg_export_flags();
Index: code/pilotfile/pilotfile.h
===================================================================
--- code/pilotfile/pilotfile.h (revision 9621)
+++ code/pilotfile/pilotfile.h (working copy)
@@ -10,6 +10,20 @@
struct player;
+// current pilot constants
+static const unsigned int PLR_FILE_ID = 0x5f524c50; // "PLR_" in file
+static const unsigned int CSG_FILE_ID = 0x5f475343; // "CSG_" in file
+// NOTE: Version should be bumped only for adding/removing sections or section
+// content. It should *NOT* be bumped for limit bumps or anything of
+// that sort!
+// 0 - initial version
+// 1 - Adding support for the player is multi flag
+static const ubyte PLR_VERSION = 1;
+// 0 - initial version
+// 1 - re-add recent missions
+static const ubyte CSG_VERSION = 1;
+
+
class pilotfile {
public:
pilotfile();
Index: code/pilotfile/pilotfile_convert.h
===================================================================
--- code/pilotfile/pilotfile_convert.h (revision 9621)
+++ code/pilotfile/pilotfile_convert.h (working copy)
@@ -26,6 +26,7 @@
#include "mission/missioncampaign.h"
#include "controlconfig/controlsconfig.h"
#include "missionui/redalert.h"
+#include "pilotfile/pilotfile.h"
Index: code/pilotfile/plr.cpp
===================================================================
--- code/pilotfile/plr.cpp (revision 9621)
+++ code/pilotfile/plr.cpp (working copy)
@@ -18,17 +18,6 @@
#include "playerman/managepilot.h"
-static const unsigned int PLR_FILE_ID = 0x5f524c50; // "PLR_" in file
-
-// NOTE: Version should be bumped only for adding/removing sections or section
-// content. It should *NOT* be bumped for limit bumps or anything of
-// that sort!
-//
-// 0 - initial version
-// 1 - Adding support for the player is multi flag
-static const ubyte PLR_VERSION = 1;
-
-
void pilotfile::plr_read_flags()
{
// tips?
Index: code/pilotfile/plr_convert.cpp
===================================================================
--- code/pilotfile/plr_convert.cpp (revision 9621)
+++ code/pilotfile/plr_convert.cpp (working copy)
@@ -816,8 +816,8 @@
Assert( cfp != NULL );
// header and version
- cfwrite_int(0x5f524c50, cfp);
- cfwrite_ubyte(0, cfp);
+ cfwrite_int(PLR_FILE_ID, cfp);
+ cfwrite_ubyte(PLR_VERSION, cfp);
// flags and info sections go first
plr_export_flags();
Index: code/pilotfile/pilotfile_convert.cpp
===================================================================
--- code/pilotfile/pilotfile_convert.cpp (revision 9621)
+++ code/pilotfile/pilotfile_convert.cpp (working copy)
@@ -3,6 +3,7 @@
#include "pilotfile/pilotfile_convert.h"
#include "cfile/cfile.h"
#include "playerman/managepilot.h"
+#include "pilotfile/pilotfile.h"
pilotfile_convert::pilotfile_convert()
|
|
|
Trivial patch attached. Tested and it works as expected. The only question I would like input on is whether the consts should: 1) be put in pilotfile.h, with the possible danger of someone using some const in the conversion process where it shouldn't be used, and thus breaking the conversion process 2) be put in pilotfile_convert.h as separate consts, the obvious downside being that you must update versions in two different files. I think 1) is the best approach, but I'm open to hearing other ideas. |
|
|
pilotfile.h seems to be the most intuitive place for them. |
|
|
I agree. Nice catch and patch. |
|
|
Fix committed to trunk@9624. |
|
fs2open: trunk r9624 2013-04-06 20:46 Ported: N/A Details Diff |
Fix mantis 2832: conversion functions now use certain consts from the new pilot code when exporting *only* |
Affected Issues 0002832 |
|
| mod - /trunk/fs2_open/code/pilotfile/csg.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/pilotfile/csg_convert.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/pilotfile/pilotfile.h | Diff File | ||
| mod - /trunk/fs2_open/code/pilotfile/pilotfile_convert.h | Diff File | ||
| mod - /trunk/fs2_open/code/pilotfile/plr.cpp | Diff File | ||
| mod - /trunk/fs2_open/code/pilotfile/plr_convert.cpp | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-04-03 10:07 | niffiwan | New Issue | |
| 2013-04-03 10:07 | niffiwan | Status | new => assigned |
| 2013-04-03 10:07 | niffiwan | Assigned To | => niffiwan |
| 2013-04-03 10:10 | niffiwan | Additional Information Updated | |
| 2013-04-03 10:11 | niffiwan | Additional Information Updated | |
| 2013-04-05 09:48 | niffiwan | Relationship added | related to 0002815 |
| 2013-04-05 10:47 | The_E | Note Added: 0014899 | |
| 2013-04-06 02:12 | niffiwan | File Added: mantis2832-svn.patch | |
| 2013-04-06 02:15 | niffiwan | Note Added: 0014901 | |
| 2013-04-06 02:15 | niffiwan | Status | assigned => code review |
| 2013-04-06 12:11 | The_E | Note Added: 0014908 | |
| 2013-04-06 18:22 | Zacam | Note Added: 0014909 | |
| 2013-04-06 23:58 | niffiwan | Changeset attached | => fs2open trunk r9624 |
| 2013-04-06 23:58 | niffiwan | Note Added: 0014910 | |
| 2013-04-06 23:58 | niffiwan | Status | code review => resolved |
| 2013-04-06 23:58 | niffiwan | Resolution | open => fixed |
