View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002376 | FSSCP | models | public | 2011-01-09 10:41 | 2012-11-28 07:32 |
| Reporter | FUBAR-BDHR | Assigned To | The_E | ||
| Priority | normal | Severity | major | Reproducibility | always |
| Status | closed | Resolution | unable to reproduce | ||
| Product Version | 3.6.13 | ||||
| Summary | 0002376: Cockpit models not rendered in nebula or cause outright crash | ||||
| Description | I've noticed in recent testing that cockpit models do not render at all in a nebula. If you play a mission with the same ship then load a nebula mission you get the following crash: Assert: Polygon_models[num]->id == model_num File: modelread.cpp Line: 2832 <no module>! KiFastSystemCallRet <no module>! WaitForSingleObject + 18 bytes <no module>! SCP_DumpStack + 354 bytes <no module>! WinAssert + 208 bytes <no module>! model_get + 256 bytes <no module>! ship_add_cockpit_display + 105 bytes <no module>! ship_init_cockpit_displays + 283 bytes <no module>! game_enter_state + 1171 bytes <no module>! gameseq_set_state + 310 bytes <no module>! game_process_event + 426 bytes <no module>! gameseq_process_events + 152 bytes <no module>! game_main + 782 bytes <no module>! WinMain + 330 bytes <no module>! __tmainCRTStartup + 358 bytes <no module>! WinMainCRTStartup + 15 bytes <no module>! RegisterWaitForInputIdle + 73 bytes | ||||
| Tags | No tags attached. | ||||
|
|
If you comment out line modelread.cpp:2827 in the current trunk, and when playing with this modified build you hit instead Assert( num < MAX_POLYGON_MODELS ) -- then it's almost 100% going to be because we've hit the limit of Polygon_models[MAX_POLYGON_MODELS]. MAX_POLYGON_MODELS is 128 in standard builds, 300 in Inferno. I think this will only show up in nebulas as the nebula lightning strikes also use Polygon_models array. *sigh* fixed length arrays.... |
|
|
OK that makes no sense. Line 2827 is: int num = model_num % MAX_POLYGON_MODELS; If it comment that out num won't even be defined and the next 2 lines are asserts based on it including the one you listed. Also the assert it is hitting is the last one in that function so it's already passed the num checks. Just to be on the safe side when it asserts num = 5. Just to be sure I bumped MAX_POLYGON_MODELS to 500 for a quick test. no difference. |
|
|
Sorry, what I'm trying to show is that line 2827 is a poor form of boundary case checking to ensure we don't overrun the fixed array Polygon_models[]. That num = 5 (low number) indicates to me model_num is probably MAX_POLYGON_MODELS + 5 at the point of the Assert(). |
|
|
Is there a testing mission to reproduce? |
|
|
This should be reproducible with any ships with a cockpit model in Diaspora. All you need is a mission with default player ship and set full nebula in FRED. |
|
|
2376.patch (4,762 bytes)
Index: model/modelread.cpp
===================================================================
--- model/modelread.cpp (revision 9365)
+++ model/modelread.cpp (working copy)
@@ -54,7 +54,7 @@
// info for special polygon lists
-polymodel *Polygon_models[MAX_POLYGON_MODELS];
+SCP_vector<polymodel*> Polygon_models;
SCP_vector<polymodel_instance*> Polygon_model_instances;
SCP_vector<bsp_collision_tree> Bsp_collision_tree_list;
@@ -125,15 +125,12 @@
// With the basic page in system this can be called from outside of modelread.cpp
void model_unload(int modelnum, int force)
{
- int i, j, num;
+ int i, j;
+ size_t num;
- if ( modelnum >= MAX_POLYGON_MODELS ) {
- num = modelnum % MAX_POLYGON_MODELS;
- } else {
- num = modelnum;
- }
+ num = modelnum;
- if ( (num < 0) || (num >= MAX_POLYGON_MODELS)) {
+ if ( (num < 0) || (num >= Polygon_models.size())) {
return;
}
@@ -286,8 +283,6 @@
void model_free_all()
{
- int i;
-
if ( !model_initted) {
model_init();
return;
@@ -296,7 +291,7 @@
mprintf(( "Freeing all existing models...\n" ));
model_instance_free_all();
- for (i=0;i<MAX_POLYGON_MODELS;i++) {
+ for (size_t i = 0; i<Polygon_models.size(); i++) {
// forcefully unload all loaded models (be careful with this)
model_unload(i, 1);
}
@@ -318,8 +313,6 @@
void model_page_in_start()
{
- int i;
-
if ( !model_initted ) {
model_init();
return;
@@ -327,35 +320,31 @@
mprintf(( "Starting model page in...\n" ));
- for (i=0; i<MAX_POLYGON_MODELS; i++) {
- if (Polygon_models[i] != NULL)
- Polygon_models[i]->used_this_mission = 0;
+ for (SCP_vector<polymodel*>::iterator pmit = Polygon_models.begin(); pmit != Polygon_models.end(); ++pmit) {
+ if ((*pmit) != NULL)
+ (*pmit)->used_this_mission = 0;
}
}
void model_page_in_stop()
{
- int i;
-
Assert( model_initted );
mprintf(( "Stopping model page in...\n" ));
- for (i=0; i<MAX_POLYGON_MODELS; i++) {
- if (Polygon_models[i] == NULL)
+ for (SCP_vector<polymodel*>::iterator pmit = Polygon_models.begin(); pmit != Polygon_models.end(); ++pmit) {
+ if ((*pmit) == NULL)
continue;
- if (Polygon_models[i]->used_this_mission)
+ if ((*pmit)->used_this_mission)
continue;
-
- model_unload(i);
+
+ model_unload(pmit - Polygon_models.begin());
}
}
void model_init()
{
- int i;
-
if ( model_initted ) {
Int3(); // Model_init shouldn't be called twice!
return;
@@ -365,9 +354,7 @@
Model_ram = 0;
#endif
- for (i=0;i<MAX_POLYGON_MODELS;i++) {
- Polygon_models[i] = NULL;
- }
+ Polygon_models.clear();
atexit( model_free_all );
model_initted = 1;
@@ -2408,7 +2395,7 @@
num = -1;
- for (i=0; i< MAX_POLYGON_MODELS; i++) {
+ for (i=0; i< (int)Polygon_models.size(); i++) {
if ( Polygon_models[i] ) {
if (!stricmp(filename, Polygon_models[i]->filename) && !duplicate) {
// Model already loaded; just return.
@@ -2421,6 +2408,9 @@
}
}
+ if (Polygon_models.size() == 0)
+ num = 0;
+
// No empty slot
if ( num == -1 ) {
Error( LOCATION, "Too many models" );
@@ -2431,22 +2421,15 @@
pm = (polymodel *)vm_malloc( sizeof(polymodel) );
Assert( pm != NULL );
+
+ memset(pm, 0, sizeof(polymodel));
- Polygon_models[num] = pm;
-
- memset(pm, 0, sizeof(polymodel));
+ Polygon_models.push_back(pm);
pm->n_paths = 0;
pm->paths = NULL;
- int org_sig = Model_signature;
- Model_signature+=MAX_POLYGON_MODELS;
- if ( Model_signature < org_sig ) {
- Model_signature = 0;
- }
- Assert( (Model_signature % MAX_POLYGON_MODELS) == 0 );
- pm->id = Model_signature + num;
- Assert( (pm->id % MAX_POLYGON_MODELS) == num );
+ pm->id = Polygon_models.size()-1;
extern int Parse_normal_problem_count;
Parse_normal_problem_count = 0;
@@ -2940,17 +2923,10 @@
return NULL;
}
- int num = model_num % MAX_POLYGON_MODELS;
-
- Assertion( num >= 0, "Model id %d is invalid. Please backtrace and investigate.\n", num);
- Assertion( num < MAX_POLYGON_MODELS, "Model id %d is larger than MAX_POLYGON_MODELS (%d). This is impossible, thus we have to conclude that math as we know it has ceased to work.\n", num, MAX_POLYGON_MODELS );
- Assertion( Polygon_models[num], "No model with id %d found. Please backtrace and investigate.\n", num );
- Assertion( Polygon_models[num]->id == model_num, "Index collision between model %s and requested model %d. Please backtrace and investigate.\n", Polygon_models[num]->filename, model_num );
-
- if (num < 0 || num > MAX_POLYGON_MODELS || !Polygon_models[num] || Polygon_models[num]->id != model_num)
+ if (model_num < (int)Polygon_models.size())
+ return Polygon_models[model_num];
+ else
return NULL;
-
- return Polygon_models[num];
}
polymodel_instance* model_get_instance(int model_instance_num)
|
|
|
I've attached a patch that makes Polygon_models dynamic. Please test. |
|
|
oops wrong bug |
|
|
Closing as unable to reproduce. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2011-01-09 10:41 | FUBAR-BDHR | New Issue | |
| 2011-01-10 11:17 | Echelon9 | Note Added: 0012591 | |
| 2011-01-10 22:17 | FUBAR-BDHR | Note Added: 0012592 | |
| 2011-01-10 22:38 | FUBAR-BDHR | Note Edited: 0012592 | |
| 2011-01-11 13:06 | Echelon9 | Note Added: 0012594 | |
| 2012-04-03 13:38 | Echelon9 | Note Added: 0013447 | |
| 2012-04-03 13:38 | Echelon9 | Status | new => feedback |
| 2012-11-20 16:38 | FUBAR-BDHR | Note Added: 0014127 | |
| 2012-11-20 16:38 | FUBAR-BDHR | Status | feedback => new |
| 2012-11-20 20:14 | The_E | File Added: 2376.patch | |
| 2012-11-20 20:15 | The_E | Note Added: 0014130 | |
| 2012-11-20 20:27 | Valathil | Assigned To | => Valathil |
| 2012-11-20 20:27 | Valathil | Status | new => assigned |
| 2012-11-20 20:28 | Valathil | Status | assigned => resolved |
| 2012-11-20 20:28 | Valathil | Resolution | open => fixed |
| 2012-11-20 20:28 | Valathil | Note Added: 0014132 | |
| 2012-11-20 20:28 | Valathil | Status | resolved => feedback |
| 2012-11-20 20:28 | Valathil | Resolution | fixed => reopened |
| 2012-11-20 20:28 | Valathil | Status | feedback => new |
| 2012-11-20 20:30 | Echelon9 | Status | new => code review |
| 2012-11-20 20:31 | Valathil | Assigned To | Valathil => The_E |
| 2012-11-20 20:31 | Valathil | Status | code review => assigned |
| 2012-11-28 07:32 | The_E | Note Added: 0014204 | |
| 2012-11-28 07:32 | The_E | Status | assigned => closed |
| 2012-11-28 07:32 | The_E | Resolution | reopened => unable to reproduce |