View Issue Details

IDProjectCategoryView StatusLast Update
0002923FSSCPPilot datapublic2014-06-14 08:41
ReporterGoober5000 Assigned Toniffiwan  
PriorityurgentSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.7.0 
Target Version3.7.1 
Summary0002923: New pilot code has no backout option for all-time stats
DescriptionIf you look at the scoring code, the update_stats method is always called shortly after scoring_do_accept is called in order to save all-time stats. However, there needs to be a corresponding update_stats_backout method that should be called after scoring_backout_accept.
TagsNo tags attached.

Relationships

child of 0002940 resolvedniffiwan multi stats are not saved 

Activities

niffiwan

2013-10-03 20:37

developer   ~0015305

I'm having problems reproducing this issue in-game. Have you been able to to do this? I've basically tried:

Record pilots primary shots fired (from barracks)
Play a test campaign / test mission (just lots of ai-play-dead shivan ships)
Destroy some ships & warp out
Record all-time primary shots in debrief
Click Replay mission button
Destroy different ships this time & warp out
Record all-time primary shots in debrief
Accept mission
Leave campaign & check stats in barracks

It seems to be behaving as you'd expect, i.e. only the accepted stats are recorded. If the stats were not being backed out (or some equivalent process) then I'd expect the barracks stats to be the sum of both plays of the mission, i.e. the "replay" and the "accept" (plus the starting point of course).

(not that I have found out yet WHAT in the code is causing the correct behaviour)

Goober5000

2013-10-04 03:48

administrator   ~0015306

Try playing the mission, but hitting escape in the debriefing followed by "try again later". Even if you win the mission and can proceed to the next one, abort without continuing.

niffiwan

2013-10-12 04:49

developer   ~0015326

I've run some more tests, "hitting escape in the debriefing followed by "try again later" doesn't help, I see the same behaviour (i.e. the 1st stats seem to get tossed correctly)

Having said that, I'm now reasonably sure the "all time stats" in the barracks are *not* in fact all time stats. I think they're campaign-all-time-stats because they change when you switch campaigns.

I'm going to need to do some more digging.

niffiwan

2013-10-21 08:33

developer  

mantis2923-svn.patch (5,712 bytes)   
Index: code/pilotfile/pilotfile.cpp
===================================================================
--- code/pilotfile/pilotfile.cpp	(revision 9970)
+++ code/pilotfile/pilotfile.cpp	(working copy)
@@ -182,6 +182,114 @@
 
 void pilotfile::update_stats_backout(scoring_struct *stats, bool training)
 {
+	int i, j;
+	uint idx;
+	size_t list_size;
+	index_list_t ilist;
+	scoring_special_t *p_stats = NULL;
+
+	if (Game_mode & GM_MULTIPLAYER) {
+		p_stats = &multi_stats;
+	} else {
+		p_stats = &all_time_stats;
+	}
+
+	// medals
+	if (stats->m_medal_earned >= 0) {
+		list_size = p_stats->medals_earned.size();
+
+		j = -1;
+
+		for (idx = 0; idx < list_size; idx++) {
+			if ( p_stats->medals_earned[idx].name.compare(Medals[stats->m_medal_earned].name) == 0 ) {
+				j = idx;
+				break;
+			}
+		}
+
+		if (j >= 0) {
+			p_stats->medals_earned[j].val = MAX(0,p_stats->medals_earned[j].val--);
+		} else {
+			Assertion(true, "Medal '%s' not found, should have been added by pilotfile::update_stats.", Medals[stats->m_medal_earned].name);
+		}
+	}
+
+	// only medals can be awarded in training missions
+	if (training) {
+		return;
+	}
+
+	p_stats->score -= stats->m_score;
+
+	p_stats->assists -= stats->m_assists;
+	p_stats->kill_count -= stats->m_kill_count;
+	p_stats->kill_count_ok -= stats->m_kill_count_ok;
+	p_stats->bonehead_kills -= stats->m_bonehead_kills;
+
+	p_stats->p_shots_fired -= stats->mp_shots_fired;
+	p_stats->p_shots_hit -= stats->mp_shots_hit;
+	p_stats->p_bonehead_hits -= stats->mp_bonehead_hits;
+
+	p_stats->s_shots_fired -= stats->ms_shots_fired;
+	p_stats->s_shots_hit -= stats->ms_shots_hit;
+	p_stats->s_bonehead_hits -= stats->ms_bonehead_hits;
+
+	p_stats->flight_time -= (unsigned int)f2i(Missiontime);
+	p_stats->last_flown = p_stats->last_backup;
+	p_stats->missions_flown--;
+
+	if (stats->m_promotion_earned >= 0) {
+		// deal with a multi-rank promotion mission
+		for (i = 0; i < MAX_FREESPACE2_RANK; ++i) {
+			if (p_stats->score <= Ranks[i].points) {
+				p_stats->rank = i-1;
+				break;
+			}
+		}
+		Assertion (p_stats->rank >= 0, "Rank became negative.");
+	}
+
+	// badges
+	if (stats->m_badge_earned >= 0) {
+		list_size = p_stats->medals_earned.size();
+
+		j = -1;
+
+		for (idx = 0; idx < list_size; idx++) {
+			if ( p_stats->medals_earned[idx].name.compare(Medals[stats->m_badge_earned].name) == 0 ) {
+				j = idx;
+				break;
+			}
+		}
+
+		if (j >= 0) {
+			p_stats->medals_earned[j].val = 0;
+		} else {
+			Assertion (true, "Badge '%s' not found, should have been added by pilotfile::update_stats.", Medals[stats->m_badge_earned].name);
+		}
+	}
+
+	// ship kills
+	for (i = 0; i < Num_ship_classes; i++) {
+		if (stats->m_okKills[i] > 0) {
+			list_size = p_stats->ship_kills.size();
+
+			j = -1;
+
+			for (idx = 0; idx < list_size; idx++) {
+				if ( p_stats->ship_kills[idx].name.compare(Ship_info[i].name) == 0 ) {
+					j = i;
+					break;
+				}
+			}
+
+			if (j >= 0) {
+				p_stats->ship_kills[j].val -= stats->m_okKills[i];
+			} else {
+				Assertion(true, "Ship kills of '%s' not found, should have been added by pilotfile::update_stats.", stats->m_okKills[i]);
+			}
+		}
+	}
 }
 
 /**
Index: code/missionui/missiondebrief.cpp
===================================================================
--- code/missionui/missiondebrief.cpp	(revision 9970)
+++ code/missionui/missiondebrief.cpp	(working copy)
@@ -47,6 +47,7 @@
 #include "network/multi_campaign.h"
 #include "network/multi_endgame.h"
 #include "missionui/chatbox.h"
+#include "pilotfile/pilotfile.h"
 
 
 #define MAX_TOTAL_DEBRIEF_LINES	200
@@ -2109,6 +2110,7 @@
 void debrief_close()
 {
 	int i;
+	scoring_struct *sc;
 
 	Assert(Debrief_inited);
 
@@ -2121,11 +2123,17 @@
 			if(MULTIPLAYER_MASTER){
 				for(i=0; i<MAX_PLAYERS; i++){
 					if(MULTI_CONNECTED(Net_players[i]) && !MULTI_STANDALONE(Net_players[i]) && !MULTI_PERM_OBSERVER(Net_players[i]) && (Net_players[i].m_player != NULL)){
-						scoring_backout_accept(&Net_players[i].m_player->stats);
+						sc = &Net_players[i].m_player->stats;
+						scoring_backout_accept(sc);
+
+						if (Net_player == &Net_players[i]) {
+							Pilot.update_stats_backout( sc );
+						}
 					}
 				}
 			} else {
 				scoring_backout_accept( &Player->stats );
+				Pilot.update_stats_backout( &Player->stats );
 			}
 		}
 	} else {
@@ -2136,6 +2144,7 @@
 				Campaign.next_mission = Campaign.current_mission;
 			}
 			scoring_backout_accept( &Player->stats );
+			Pilot.update_stats_backout( &Player->stats );
 		}
 	}
 
Index: code/network/multi_dogfight.cpp
===================================================================
--- code/network/multi_dogfight.cpp	(revision 9970)
+++ code/network/multi_dogfight.cpp	(working copy)
@@ -30,7 +30,7 @@
 #include "stats/scoring.h"
 #include "mission/missionparse.h"
 #include "iff_defs/iff_defs.h"
-
+#include "pilotfile/pilotfile.h"
 #include "fs2netd/fs2netd_client.h"
 #include "cfile/cfile.h"
 
@@ -296,6 +296,7 @@
 void multi_df_debrief_close()
 {
 	int idx;
+	scoring_struct *sc;
 
 	// shutdown the chatbox
 	chatbox_close();
@@ -307,11 +308,17 @@
 			if(MULTIPLAYER_MASTER){
 				for(idx=0; idx<MAX_PLAYERS; idx++){
 					if(MULTI_CONNECTED(Net_players[idx]) && !MULTI_STANDALONE(Net_players[idx]) && !MULTI_PERM_OBSERVER(Net_players[idx]) && (Net_players[idx].m_player != NULL)){
-						scoring_backout_accept(&Net_players[idx].m_player->stats);
+						sc = &Net_players[idx].m_player->stats;
+						scoring_backout_accept(sc);
+
+						if (Net_player == &Net_players[idx]) {
+							Pilot.update_stats_backout( sc );
+						}
 					}
 				}
 			} else {
 				scoring_backout_accept( &Player->stats );
+				Pilot.update_stats_backout( &Player->stats );
 			}
 		}
 	}
mantis2923-svn.patch (5,712 bytes)   

niffiwan

2013-10-21 08:33

developer  

mantis2923-testcase-svn.patch (400 bytes)   
Index: code/pilotfile/plr.cpp
===================================================================
--- code/pilotfile/plr.cpp	(revision 9970)
+++ code/pilotfile/plr.cpp	(working copy)
@@ -986,6 +986,8 @@
 	// Done!
 	mprintf(("PLR => Saving complete!\n"));
 
+	mprintf(("PLR Dump Stats: AllTime: PriShtF: %s (%u)\n", filename.c_str(), all_time_stats.p_shots_fired));
+
 	plr_close();
 
 	return true;

niffiwan

2013-10-21 08:37

developer   ~0015338

Last edited: 2013-10-21 09:00

Given that the pilot all_time_times (not all campaign stats) are not displayed anywhere, I added a mprintf in order to test my patch (beats hunting through the binary pilot file for the data!)

I managed to reproduce the issue, and the patch seems to solve it my testing. All testing done with a sample campaign that lets you warp out straight away, I just fired some shots, noted how many, then did various combinations of accepted / replayed-and-quit / esc-replay-later. All time stats seem to behave themselves now.

NOTE however, that all time stats are NOT displayed anywhere in game. Where FSO says "all time stats" it should probably say "campaign stats".

(I guess I should test the multi stats similarly...)
(Gah - except that it doesn't seem to be saving any multi stats, even on mission accept & stats "accept" when playing with one player as the server and one as the client... is that normal?)
(I hate recursion. In 3.6.18 multi stats are saved OK even with one player as the server and no clients. Separate bug?)

Goober5000

2013-10-25 23:15

administrator   ~0015347

FUBAR should be able to help you test the multi aspects.

niffiwan

2014-05-25 09:41

developer   ~0015764

Now that I've got a fix for the multi stats issue in 2940, I've retested this patch for multi and it's working as expected there as well.

Code: https://github.com/niffiwan/fs2open.github.com/commit/f84082271168272f734ccdbaa09daa751ca224bc

(note that testing this requires the fix for 2940)

Echelon9

2014-06-14 07:34

developer   ~0015863

Have commented on the Github issue.

niffiwan

2014-06-14 08:35

developer   ~0015865

Thanks to Echelon9, MageKing17 & Zacam for their reviews. Updated Assertions to use false, not true (which will never trigger)

niffiwan

2014-06-14 08:41

developer   ~0015866

Fix committed to trunk@10816.

Related Changesets

fs2open: trunk r10816

2014-06-14 04:59

niffiwan


Ported: N/A

Details Diff
Fix mantis 2923

Backout all_time and multi_stats from the pilotfile stats as
required
Affected Issues
0002923
mod - /trunk/fs2_open/code/network/multi_dogfight.cpp Diff File
mod - /trunk/fs2_open/code/pilotfile/pilotfile.cpp Diff File
mod - /trunk/fs2_open/code/missionui/missiondebrief.cpp Diff File

Issue History

Date Modified Username Field Change
2013-09-22 12:39 Goober5000 New Issue
2013-09-24 03:10 Goober5000 Status new => confirmed
2013-09-24 03:10 Goober5000 Product Version => 3.7.0
2013-09-24 03:10 Goober5000 Target Version => 3.7.1
2013-09-27 02:59 niffiwan Assigned To => niffiwan
2013-09-27 02:59 niffiwan Status confirmed => assigned
2013-10-03 20:37 niffiwan Note Added: 0015305
2013-10-04 03:48 Goober5000 Note Added: 0015306
2013-10-12 04:49 niffiwan Note Added: 0015326
2013-10-21 08:33 niffiwan File Added: mantis2923-svn.patch
2013-10-21 08:33 niffiwan File Added: mantis2923-testcase-svn.patch
2013-10-21 08:37 niffiwan Note Added: 0015338
2013-10-21 08:37 niffiwan Status assigned => code review
2013-10-21 08:53 niffiwan Note Edited: 0015338
2013-10-21 09:00 niffiwan Note Edited: 0015338
2013-10-21 09:04 niffiwan Relationship added child of 0002940
2013-10-25 23:15 Goober5000 Note Added: 0015347
2014-05-25 09:41 niffiwan Note Added: 0015764
2014-06-14 07:34 Echelon9 Note Added: 0015863
2014-06-14 08:35 niffiwan Note Added: 0015865
2014-06-14 08:41 niffiwan Changeset attached => fs2open trunk r10816
2014-06-14 08:41 niffiwan Note Added: 0015866
2014-06-14 08:41 niffiwan Status code review => resolved
2014-06-14 08:41 niffiwan Resolution open => fixed