2021-04-17 15:13 EDT


View Issue Details Jump to Notes ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0002874FSSCPPilot datapublic2013-08-14 15:32
ReporterYarn 
Assigned Toz64555 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
Platformx64OSWindows 7OS Version
Product Version3.6.19 
Target VersionFixed in Version 
Summary0002874: Pilot conversion not preserving axis mappings
DescriptionConversion from Inferno pilots to the new pilot code is not preserving the mappings for the axis controls, resulting in them being set to their defaults regardless of how the Inferno pilot is configured.
Steps To Reproduce1. Create a new pilot in FSO 3.6.18 or any build that uses the Inferno pilot code.
2. In the control configuration screen, change one or more of the mappings for the last five controls in the Ship category. Save your changes.
3. Exit FSO.
4. Load the pilot created earlier in FSO 3.7.0 or any nightly build that uses the new pilot code.
5. Check the control settings that you changed in step 2.
TagsNo tags attached.
Attached Files
  • patch file icon mantis_2874.patch (1,663 bytes) 2013-08-10 07:59 -
    Index: pilotfile/pilotfile_convert.h
    ===================================================================
    --- pilotfile/pilotfile_convert.h	(revision 9744)
    +++ pilotfile/pilotfile_convert.h	(working copy)
    @@ -29,7 +29,11 @@
     #include "pilotfile/pilotfile.h"
     
     
    +static const unsigned short MAX_JOY_AXES_CONV = 5;
     
    +
     typedef struct index_list_t {
     	SCP_string name;
     	int index;
    Index: pilotfile/plr.cpp
    ===================================================================
    --- pilotfile/plr.cpp	(revision 9744)
    +++ pilotfile/plr.cpp	(working copy)
    @@ -601,7 +601,7 @@
     		}
     	}
     
    -	list_axis = cfread_int(cfp);
    +	list_axis = (int)cfread_ushort(cfp);
     	for (idx = 0; idx < list_axis; idx++) {
     		axi = cfread_int(cfp);
     		inv = cfread_int(cfp);
    @@ -628,7 +628,7 @@
     		cfwrite_short(-1, cfp);
     	}
     
    -	cfwrite_int(NUM_JOY_AXIS_ACTIONS, cfp);
    +	cfwrite_ushort(NUM_JOY_AXIS_ACTIONS, cfp);
     
     	for (idx = 0; idx < NUM_JOY_AXIS_ACTIONS; idx++) {
     		cfwrite_int(Axis_map_to[idx], cfp);
    @@ -895,6 +895,7 @@
     
     		if (offset_pos) {
     			cfseek(cfp, offset_pos, CF_SEEK_CUR);
    +			mprintf(("PLR => WARNING: Advancing to the next section. %i bytes were skipped!\n", offset_pos));
     		}
     	}
     
    Index: pilotfile/plr_convert.cpp
    ===================================================================
    --- pilotfile/plr_convert.cpp	(revision 9744)
    +++ pilotfile/plr_convert.cpp	(working copy)
    @@ -766,7 +769,8 @@
     	}
     
     	// extra joystick stuff
    -	for (idx = 0; idx < 5; idx++) {
    +	cfwrite_ushort( MAX_JOY_AXES_CONV, cfp);
    +	for (idx = 0; idx < MAX_JOY_AXES_CONV; idx++) {
     		cfwrite_int(plr->joy_axis_map_to[idx], cfp);
     		cfwrite_int(plr->joy_invert_axis[idx], cfp);
     	}
    
    patch file icon mantis_2874.patch (1,663 bytes) 2013-08-10 07:59 +
  • patch file icon mantis_2874_1.patch (1,228 bytes) 2013-08-11 08:16 -
    Index: pilotfile/pilotfile_convert.h
    ===================================================================
    --- pilotfile/pilotfile_convert.h	(revision 9744)
    +++ pilotfile/pilotfile_convert.h	(working copy)
    @@ -29,7 +29,11 @@
     #include "pilotfile/pilotfile.h"
     
     
    +static const unsigned short MAX_JOY_AXES_CONV = 5;
     
    +
     typedef struct index_list_t {
     	SCP_string name;
     	int index;
    Index: pilotfile/plr.cpp
    ===================================================================
    --- pilotfile/plr.cpp	(revision 9744)
    +++ pilotfile/plr.cpp	(working copy)
    @@ -895,6 +895,7 @@
     
     		if (offset_pos) {
     			cfseek(cfp, offset_pos, CF_SEEK_CUR);
    +			mprintf(("PLR => WARNING: Advancing to the next section. %i bytes were skipped!\n", offset_pos));
     		}
     	}
     
    Index: pilotfile/plr_convert.cpp
    ===================================================================
    --- pilotfile/plr_convert.cpp	(revision 9744)
    +++ pilotfile/plr_convert.cpp	(working copy)
    @@ -766,7 +769,8 @@
     	}
     
     	// extra joystick stuff
    -	for (idx = 0; idx < 5; idx++) {
    +	cfwrite_int(MAX_JOY_AXES_CONV, cfp);
    +	for (idx = 0; idx < MAX_JOY_AXES_CONV; idx++) {
     		cfwrite_int(plr->joy_axis_map_to[idx], cfp);
     		cfwrite_int(plr->joy_invert_axis[idx], cfp);
     	}
    
    patch file icon mantis_2874_1.patch (1,228 bytes) 2013-08-11 08:16 +

-Relationships
related to 0002901resolvedniffiwan pilot conversion failures 
+Relationships

-Notes

~0015068

z64555 (developer)

Noticed this while going through the controls code... I didn't really see anything in there that would suggest that the analog controls were being saved. Since it's along my alley, I'll see if I can whip up a quick patch soonish.

~0015221

z64555 (developer)

Tracked down the problem to the conversion process. A necessary index of how many joystick axis was not included in the converted .plr file, thereby preventing the reading of the file and forcing the joystick axes to default values.

Attached patch includes a fix to the include the needed index, as well as a new warning when a section was not completely read, which should help track down any future data corruptions of this nature.

~0015227

niffiwan (developer)

Tested & works as expected with x/y/z/Rz axis on my joystick.

I only have one question about the patch, you've changed the PLR from using an int to a ushort to store NUM_JOY_AXIS_ACTIONS, this will probably cause 3.6.19 -> 3.7.0RC2 pilots joystick controls config to be read incorrectly after the patch is applied? Perhaps the file version should be bumped and appropriate version handling added, otherwise just stick with the int? (or just accept the breakage & move on, as the new-pilotfile feature hasn't been in an official release yet :D)

~0015228

z64555 (developer)

Last edited: 2013-08-11 08:19

View 3 revisions

hmm, We probably could keep the int without too much of a loss. It's only 2 bytes more than the ushort, but the embedded engineer in me keeps shaking his head.

edit:
Uploaded another version of the patch that keeps the int instead of using the ushort. Pro's outweigh the con's, and I don't think many people are going to miss the 2 bytes.

~0015229

niffiwan (developer)

Looks good, go ahead & commit :)

~0015237

z64555 (developer)

Fix committed to trunk@9748.
+Notes

+Related Changesets

-Issue History
Date Modified Username Field Change
2013-05-16 03:25 Yarn New Issue
2013-05-18 11:37 z64555 Note Added: 0015068
2013-07-19 22:58 niffiwan Relationship added related to 0002901
2013-08-10 07:59 z64555 File Added: mantis_2874.patch
2013-08-10 07:59 z64555 Note Added: 0015221
2013-08-10 08:00 z64555 Assigned To => z64555
2013-08-10 08:00 z64555 Status new => code review
2013-08-11 06:51 niffiwan Note Added: 0015227
2013-08-11 08:07 z64555 Note Added: 0015228
2013-08-11 08:16 z64555 File Added: mantis_2874_1.patch
2013-08-11 08:18 z64555 Note Edited: 0015228 View Revisions
2013-08-11 08:19 z64555 Note Edited: 0015228 View Revisions
2013-08-11 16:31 niffiwan Note Added: 0015229
2013-08-14 15:32 z64555 Changeset attached => fs2open trunk r9748
2013-08-14 15:32 z64555 Note Added: 0015237
2013-08-14 15:32 z64555 Status code review => resolved
2013-08-14 15:32 z64555 Resolution open => fixed
+Issue History