View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002701 | FSSCP | tables | public | 2012-08-21 23:53 | 2012-10-14 19:25 |
| Reporter | MjnMixael | Assigned To | Valathil | ||
| Priority | normal | Severity | crash | Reproducibility | always |
| Status | closed | Resolution | unable to reproduce | ||
| Product Version | 3.6.13 | ||||
| Summary | 0002701: Missing EFF frame causes FSO to CTD | ||||
| Description | I discovered this while working mainhalls, so that's my only example. If in the mainhall.tbl you accidentally specify a sound to play on a frame that doesn't exist (say frame 250, but your animation is only 100 frames long).. FSO will CTD. | ||||
| Steps To Reproduce | Open retail mainhall.tbl and change any of the +Misc Anim Trigger: numbers (except the first one) to some absurd frame number like 500. I'll be attaching a testcase shortly. | ||||
| Additional Information | If FSO needs to terminate, probably best to allow it to do so a little more gracefully and with some useful information attached. | ||||
| Tags | No tags attached. | ||||
|
|
OK, so apparently not. I tried to reproduce with a reliable test case to upload and have been unsuccessful. |
|
|
Also very much a case of Garbage In Garbage Out. Not sure if want to fix. |
|
|
I'm fine with closing this... until I can get something I can upload to reproduce this is likely a waste of time anyway. |
|
|
2701.patch (1,143 bytes)
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp (revision 9134)
+++ code/menuui/mainhallmenu.cpp (working copy)
@@ -1972,9 +1972,24 @@
required_string("+Misc anim trigger:");
int temp_int = 0;
stuff_int(&temp_int);
+
+ //load the ani to get the number of frames
+ int num_frames;
+ int ani = bm_load_either(const_cast<char*>(m->misc_anim_name.at(idx).c_str()), &num_frames);
+ //unload it again
+ bm_unload(ani);
+
for (s_idx=0; s_idx<temp_int; s_idx++) {
m->misc_anim_special_trigger.at(idx).push_back(0);
- stuff_int(&m->misc_anim_special_trigger.at(idx).at(s_idx));
+ int trigger_frame;
+ stuff_int(&trigger_frame);
+
+ if (trigger_frame < num_frames) {
+ m->misc_anim_special_trigger.at(idx).at(s_idx);
+ } else {
+ WarningEx(LOCATION, "Frame number %d is too high for animation %s. The animation only has %d frames.\nTrigger frame has been set to 0.\n",
+ trigger_frame, m->misc_anim_name.at(idx).c_str(), num_frames);
+ }
}
}
|
|
|
Attached a patch for review. |
|
|
Nonononononono if you use bm_load_either you load ALL eff frames into memory and then unload it again. Not a very efficient way of getting the framecount. |
|
|
Neither me nor Mjn can reproduce. Also analysis of the code in question doesn't imply a crash when large trigger frame numbers are encoutered. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2012-08-21 23:53 | MjnMixael | New Issue | |
| 2012-08-22 00:34 | MjnMixael | Note Added: 0013932 | |
| 2012-08-22 14:44 | Valathil | Note Added: 0013936 | |
| 2012-08-22 15:08 | MjnMixael | Note Added: 0013937 | |
| 2012-08-22 16:26 | The_E | File Added: 2701.patch | |
| 2012-08-22 16:26 | The_E | Note Added: 0013938 | |
| 2012-08-22 16:49 | Echelon9 | Status | new => code review |
| 2012-08-22 19:13 | Valathil | Note Added: 0013940 | |
| 2012-10-14 19:24 | Valathil | Assigned To | => Valathil |
| 2012-10-14 19:24 | Valathil | Status | code review => assigned |
| 2012-10-14 19:25 | Valathil | Note Added: 0013981 | |
| 2012-10-14 19:25 | Valathil | Status | assigned => closed |
| 2012-10-14 19:25 | Valathil | Resolution | open => unable to reproduce |