Skip to content

Commit 1832fa0

Browse files
committed
fix a FRED2 crash
Fix a crash caused in part by some missing UI calls in FRED2 and in part by subtle object life cycle issues. Depending on whether FRED receives the focus during mission load (which can happen if the autosave dialog appears) it could access props that were in the process of being cleaned up but were still referenced. This isn't specifically a props issue; it could happen to any FSO object stored in a SCP_vector, since the vector can be cleared. The quick mission init is to prevent a whole bunch of unnecessary and risky processing during a transient loading state, but it has the nice side effect of making mission list loading noticeably faster.
1 parent 4fbfa84 commit 1832fa0

13 files changed

Lines changed: 45 additions & 30 deletions

File tree

code/jumpnode/jumpnode.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,5 +562,6 @@ void jumpnode_render_all()
562562
*/
563563
void jumpnode_level_close()
564564
{
565+
// Clear all jump nodes. Note that this can happen either before or after objects are cleaned up.
565566
Jump_nodes.clear();
566567
}

code/mission/missionparse.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6551,7 +6551,7 @@ bool parse_mission(mission *pm, int flags)
65516551
Warned_about_team_out_of_range = false;
65526552

65536553
reset_parse();
6554-
mission_init(pm);
6554+
mission_init(pm, (flags & MPF_ONLY_MISSION_INFO) != 0);
65556555

65566556
parse_mission_info(pm);
65576557

@@ -7107,7 +7107,7 @@ void mission::Reset()
71077107
/**
71087108
* Initialize the mission and related data structures.
71097109
*/
7110-
void mission_init(mission *pm)
7110+
void mission_init(mission *pm, bool quick_init)
71117111
{
71127112
pm->Reset();
71137113

@@ -7119,6 +7119,11 @@ void mission_init(mission *pm)
71197119
Mission_all_attack = 0;
71207120
Num_teams = 1; // assume 1
71217121

7122+
// sometimes we don't need to run through the entire initialization,
7123+
// e.g. if we're just checking mission info
7124+
if (quick_init)
7125+
return;
7126+
71227127
init_sexp();
71237128
mission_goals_and_events_init();
71247129

code/mission/missionparse.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ extern p_object *Arriving_support_ship;
575575
extern char Neb2_texture_name[MAX_FILENAME_LEN];
576576

577577

578-
void mission_init(mission *pm);
578+
void mission_init(mission *pm, bool quick_init = false);
579579
bool parse_main(const char *mission_name, int flags = 0);
580580
p_object *mission_parse_get_arrival_ship(ushort net_signature);
581581
p_object *mission_parse_get_arrival_ship(const char *name);

code/object/object.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -672,9 +672,6 @@ void obj_delete_all()
672672
obj_delete(i);
673673
}
674674

675-
// If we've removed all objects then we can safely clear the Props vector
676-
Props.clear();
677-
678675
mprintf(("Cleanup: Deleted %i objects\n", counter));
679676
}
680677

code/object/waypoint.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ void waypoint_list::set_name(const char *name)
140140
//********************FUNCTIONS********************
141141
void waypoint_level_close()
142142
{
143+
// Clear all waypoint lists and all their waypoints. Note that this can happen either before or after objects are cleaned up.
143144
Waypoint_lists.clear();
144145
}
145146

code/prop/prop.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -823,19 +823,14 @@ void prop_render(object* obj, model_draw_list* scene)
823823
}
824824
}*/
825825

826-
void props_level_init() {
826+
void props_level_init()
827+
{
827828
Props.clear();
828829
}
829830

830831
void props_level_close()
831832
{
832-
for (auto& opt_prop : Props) {
833-
if (opt_prop.has_value()) {
834-
prop_delete(&Objects[opt_prop->objnum]);
835-
}
836-
}
837-
838-
// Clear all props and empty prop slots
833+
// Clear all props and empty prop slots. Note that this can happen either before or after objects are cleaned up.
839834
Props.clear();
840835
}
841836

fred2/freddoc.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,7 @@ bool CFREDDoc::autoload() {
108108
return 0;
109109
fclose(fp);
110110

111-
if (Briefing_dialog) {
112-
// clean things up first
113-
Briefing_dialog->icon_select(-1);
114-
}
111+
clean_up_selections();
115112

116113
// Load Backup.002
117114
r = load_mission(name, MPF_FAST_RELOAD);
@@ -482,10 +479,6 @@ void CFREDDoc::OnFileImportFSM() {
482479
if (*dest_directory == '\0')
483480
return;
484481

485-
// clean things up first
486-
if (Briefing_dialog)
487-
Briefing_dialog->icon_select(-1);
488-
489482
clear_mission(true);
490483

491484
int num_files = 0;
@@ -611,8 +604,8 @@ BOOL CFREDDoc::OnNewDocument() {
611604

612605
BOOL CFREDDoc::OnOpenDocument(LPCTSTR pathname)
613606
{
614-
if (Briefing_dialog)
615-
Briefing_dialog->icon_select(-1); // clean things up first
607+
// don't process any objects if the window focus is lost and reacquired
608+
clean_up_selections();
616609

617610
auto sep_ch = strrchr(pathname, '\\');
618611
auto filename = (sep_ch != nullptr) ? (sep_ch + 1) : pathname;

fred2/fredview.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,11 +1250,10 @@ void CFREDView::OnSetFocus(CWnd* pOldWnd)
12501250
Update_wing = 0;
12511251
}
12521252

1253-
/* if (Wing_editor_dialog.verify() == -1)
1254-
return; // abort
1255-
1256-
if (Ship_editor_dialog.verify() == -1)
1257-
return; // abort*/
1253+
if (Update_prop) {
1254+
Prop_editor_dialog.initialize_data(1);
1255+
Update_prop = 0;
1256+
}
12581257

12591258
if (update_dialog_boxes()) {
12601259
nprintf(("Fred routing", "OnSetFocus() returned (error occured)\n"));
@@ -1398,7 +1397,7 @@ void select_objects()
13981397
}
13991398
}
14001399

1401-
Update_ship = Update_wing = 1;
1400+
Update_ship = Update_wing = Update_prop = 1;
14021401
}
14031402

14041403
LRESULT CFREDView::OnMenuPopupShips(WPARAM wParam, LPARAM lParam)

fred2/management.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,7 @@ void clear_mission(bool fast_reload)
865865
CTime t;
866866

867867
// clean up everything we need to before we reset back to defaults.
868+
clean_up_selections();
868869
if (Briefing_dialog){
869870
Briefing_dialog->reset_editor();
870871
}
@@ -1545,6 +1546,14 @@ void unmark_all()
15451546
}
15461547
}
15471548

1549+
void clean_up_selections()
1550+
{
1551+
if (Briefing_dialog)
1552+
Briefing_dialog->icon_select(-1);
1553+
1554+
unmark_all();
1555+
}
1556+
15481557
void clear_menu(CMenu *ptr)
15491558
{
15501559
int count;

fred2/management.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ void clear_menu(CMenu* ptr);
9090
void generate_wing_popup_menu(CMenu* mptr, int first_id, int state);
9191
void generate_ship_popup_menu(CMenu* mptr, int first_id, int state, int filter = 0);
9292
int string_lookup(const CString& str1, char* strlist[], int max);
93+
void clean_up_selections();
9394
int update_dialog_boxes();
9495
void set_cur_wing(int wing);
9596
int gray_menu_tree(CMenu* base);

0 commit comments

Comments
 (0)