View Issue Details

IDProjectCategoryView StatusLast Update
0003115FSSCPmodelspublic2021-02-10 04:49
ReporterBlack Wolf Assigned ToGoober5000  
PriorityhighSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Target Version3.8Fixed in Version21.2.0 
Summary0003115: look_at has been removed from the engine
DescriptionLast year, VA mentioned that he had accidentally broken look_at for submodels (http://www.hard-light.net/forums/index.php?topic=84897.msg1713468#msg1713468).

The feature has yet to be repaired and reintegrated with the engine as of the September 17 nightly.
Steps To ReproduceLaunch fighters from the Praetor. Its hydraulic rams should be looking at each other, instead they follow the orientation of the bay doors.
Additional InformationSee thread here: http://www.hard-light.net/forums/index.php?topic=88393.0
TagsNo tags attached.

Activities

Black Wolf

2014-10-01 12:15

reporter  

Praetor.7z (722,861 bytes)

MageKing17

2014-10-09 23:47

developer   ~0016336

Relevant: http://www.hard-light.net/forums/index.php?topic=83628.msg1709474#msg1709474

MageKing17

2014-10-17 22:52

developer  

3115.patch (2,617 bytes)   
Index: code/model/modelinterp.cpp
===================================================================
--- code/model/modelinterp.cpp	(revision 11150)
+++ code/model/modelinterp.cpp	(working copy)
@@ -1952,6 +1952,8 @@
 void model_render(int model_num, matrix *orient, vec3d * pos, uint flags, int objnum, int lighting_skip, int *replacement_textures, const bool is_skybox)
 {
 	int cull = 0;
+
+	model_do_look_at(model_num);
 	// replacement textures - Goober5000
 	model_set_replacement_textures(replacement_textures);
 
Index: code/model/modelread.cpp
===================================================================
--- code/model/modelread.cpp	(revision 11150)
+++ code/model/modelread.cpp	(working copy)
@@ -1259,14 +1259,6 @@
 					}
 				}
 
-				// Sets can_move on submodels which are of a rotating type or which have such a parent somewhere down the hierarchy
-				if ( (pm->submodel[n].movement_type != MOVEMENT_TYPE_NONE)
-					|| strstr(props, "$triggered:") || strstr(props, "$rotate") || strstr(props, "$dumb_rotate:") || strstr(props, "$gun_rotation:") || strstr(props, "$gun_rotation") ) {
-					pm->submodel[n].can_move = true;
-				} else if (pm->submodel[n].parent > -1 && pm->submodel[pm->submodel[n].parent].can_move) {
-					pm->submodel[n].can_move = true;
-				}
-
 				if ( ( p = strstr(props, "$look_at:")) != NULL ) {
 					pm->submodel[n].movement_type = MOVEMENT_TYPE_LOOK_AT;
 					get_user_prop_value(p+9, pm->submodel[n].look_at);
@@ -1283,6 +1275,14 @@
 					pm->submodel[n].dumb_turn_rate = 0.0f;
 				}
 
+				// Sets can_move on submodels which are of a rotating type or which have such a parent somewhere down the hierarchy
+				if ( (pm->submodel[n].movement_type != MOVEMENT_TYPE_NONE)
+					|| strstr(props, "$triggered:") || strstr(props, "$rotate") || strstr(props, "$gun_rotation:") || strstr(props, "$gun_rotation") ) {
+					pm->submodel[n].can_move = true;
+				} else if (pm->submodel[n].parent > -1 && pm->submodel[pm->submodel[n].parent].can_move) {
+					pm->submodel[n].can_move = true;
+				}
+
 				if ( pm->submodel[n].name[0] == '\0' ) {
 					strcpy_s(pm->submodel[n].name, "unknown object name");
 				}
@@ -3472,7 +3472,7 @@
 		}
 
 		if (sm->look_at_num == -2) {
-			Warning( LOCATION, "Invalid submodel name given in $look_at: property in model file <%s>. (%s looking for %s)\n", pm->filename, pm->submodel->name, sm->look_at );
+			Warning( LOCATION, "Invalid submodel name given in $look_at: property in model file <%s>. (%s looking for %s)\n", pm->filename, sm->name, sm->look_at );
 			sm->look_at_num = -1; // Set to -1 to not break stuff
 		}
 	}
3115.patch (2,617 bytes)   

MageKing17

2014-10-17 22:55

developer   ~0016339

Last edited: 2014-10-17 22:55

So I've uploaded a simple patch that adds the missing line from the above link, plus a couple of minor tweaks on the parsing side. This (plus fixing the provided POF to use submodel names instead of numbers) makes the example work, but still has the worrisome potential performance issues brought up in the previously-linked thread.

Goober5000

2015-04-16 04:31

administrator   ~0016643

Assigning this to The_E for review. It would be nice to have this figured out given that the look_at feature was meant for 3.7.0 and then forgotten.

m_m

2015-09-18 12:29

developer   ~0016782

Maybe the performance issues could be reduced by only traversing the model hierarchy for the models that actually use look_at by setting a flag when the model is loaded?

MageKing17

2015-09-18 20:20

developer   ~0016783

As The_E already noted back in 2013: http://www.hard-light.net/forums/index.php?topic=83628.msg1709521#msg1709521

Goober's suggestion immediately after (http://www.hard-light.net/forums/index.php?topic=83628.msg1709547#msg1709547) was to incorporate the look_at stuff into one of the existing traversals, which makes a lot of sense, but at the very least, a model flag would be better than doing the traversal for everything every frame.

m_m

2015-09-23 17:29

developer   ~0016788

I made a PR for this: https://github.com/scp-fs2open/fs2open.github.com/pull/358

Currently untested because I have no test mission. I'm also not sure if my usage of model_look_at is right in model_render_queue.

Goober5000

2016-02-01 03:21

administrator   ~0016811

New PR opened: https://github.com/scp-fs2open/fs2open.github.com/pull/530

Almost, but not quite, ready to be resolved.

Goober5000

2016-03-07 19:49

administrator   ~0016812

I've told chief1983 and The E that I'm bumping this to 3.7.6. The refactoring is complete, but the angle calculation is proving extremely difficult to get working for all models in all situations.

Goober5000

2021-01-14 23:31

administrator   ~0017103

It's been a long journey, but look_at has finally been fixed.
https://github.com/scp-fs2open/fs2open.github.com/pull/3144

The fix won't be merged for another couple of weeks however.

Goober5000

2021-02-10 04:49

administrator   ~0017104

Last edited: 2021-02-10 04:49

Fix merged!

Issue History

Date Modified Username Field Change
2014-10-01 12:15 Black Wolf New Issue
2014-10-01 12:15 Black Wolf File Added: Praetor.7z
2014-10-03 21:53 Goober5000 Priority normal => high
2014-10-03 21:53 Goober5000 Status new => confirmed
2014-10-03 21:53 Goober5000 Target Version => 3.7.2
2014-10-09 23:47 MageKing17 Note Added: 0016336
2014-10-17 22:52 MageKing17 File Added: 3115.patch
2014-10-17 22:55 MageKing17 Note Added: 0016339
2014-10-17 22:55 MageKing17 Note Edited: 0016339
2014-11-21 04:04 Goober5000 Status confirmed => code review
2015-04-16 04:31 Goober5000 Note Added: 0016643
2015-04-16 04:31 Goober5000 Assigned To => The_E
2015-04-16 18:12 The_E Target Version 3.7.2 => 3.7.4
2015-09-18 12:29 m_m Note Added: 0016782
2015-09-18 20:20 MageKing17 Note Added: 0016783
2015-09-23 17:29 m_m Note Added: 0016788
2016-02-01 03:21 Goober5000 Note Added: 0016811
2016-02-01 03:21 Goober5000 Assigned To The_E => Goober5000
2016-03-07 19:49 Goober5000 Note Added: 0016812
2016-03-07 19:49 Goober5000 Target Version 3.7.4 => 3.8
2021-01-14 23:31 Goober5000 Note Added: 0017103
2021-02-10 04:49 Goober5000 Status code review => resolved
2021-02-10 04:49 Goober5000 Resolution open => fixed
2021-02-10 04:49 Goober5000 Fixed in Version => 21.2.0
2021-02-10 04:49 Goober5000 Note Added: 0017104
2021-02-10 04:49 Goober5000 Note Edited: 0017104