View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002923 | FSSCP | Pilot data | public | 2013-09-22 12:39 | 2014-06-14 08:41 |
Reporter | Goober5000 | Assigned To | niffiwan | ||
Priority | urgent | Severity | major | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.7.0 | ||||
Target Version | 3.7.1 | ||||
Summary | 0002923: New pilot code has no backout option for all-time stats | ||||
Description | If 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. | ||||
Tags | No tags attached. | ||||
|
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) |
|
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. |
|
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. |
|
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-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; |
|
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?) |
|
FUBAR should be able to help you test the multi aspects. |
|
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) |
|
Have commented on the Github issue. |
|
Thanks to Echelon9, MageKing17 & Zacam for their reviews. Updated Assertions to use false, not true (which will never trigger) |
|
Fix committed to trunk@10816. |
fs2open: trunk r10816 2014-06-14 04:59 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 |
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 |