View Issue Details

IDProjectCategoryView StatusLast Update
0002733FSSCPSEXPspublic2012-12-12 13:27
ReporterFUBAR-BDHR Assigned ToThe_E  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.6.15 
Summary0002733: Add-nav-waypoint 4th argument
DescriptionWell the first thing is that the 4th argument is undocumented. I found this while checking for sexps affected by 2048. As I have no idea what it's even trying to do I do not know what is should be doing with what is supplied in that 4th argument and if it should be applied to parse objects or wings that haven't arrived yet.
Steps To ReproduceSearch sexp.cpp for add_nav_waypoint and check the description in FRED (well I guess you could just check that in sexp.cpp as well)
TagsNo tags attached.

Activities

Goober5000

2012-11-22 21:33

administrator   ~0014151

Hoping Kazan is available to fill in help text for this...

The_E

2012-12-05 18:58

administrator   ~0014311

Grabbing this because Kazan does not seem to be inclined to show up.

While I'm at it, I'm also going to rewrite the sexp help for ALL of his navpoint sexps, given that he didn't see fit to follow the established conventions AT ALL.

Kazan

2012-12-05 20:44

developer   ~0014316

it's been so long i don't even have a code checkout of ANYTHING on my personal hardware. link me to a source browse of the file and i'll see if i can remember.

Goober5000

2012-12-05 22:41

administrator   ~0014319

https://svn.icculus.org/fs2open/trunk/fs2_open/code/parse/sexp.cpp?limit_changes=100&view=markup

Kazan

2012-12-05 23:13

developer   ~0014320

I didn't add the fourth parameter.. but from code context and the diff that added it ( https://svn.icculus.org/fs2open/trunk/fs2_open/code/parse/sexp.cpp?limit_changes=100&r1=5584&r2=5598 ) it appears to be related to making waypoints function in multiplayer the comment indicates that the fourth argument is "ShipWingTeam"

the change is Kara's
Revision 5598 - (view) (download) (as text) - [select for diffs]
Modified Tue Sep 15 13:47:38 2009 UTC (3 years, 2 months ago) by karajorma

The_E

2012-12-09 22:17

administrator   ~0014392

Fix committed to trunk@9418.

FUBAR-BDHR

2012-12-09 23:30

developer   ~0014394

Reopening this as while the fix addresses the description of the 4th argument we still don't know what it's supposed to be or how to handle it. For instance the sexp takes OPF_SHIP_WING_POINT as an argument but looks for OSWPT_TYPE_TEAM in the optional argument and does nothing(at best) if passed a point.

What we really need is to find someone that is actually using this to tell us what is is supposed to be doing.

Goober5000

2012-12-10 04:04

administrator   ~0014401

Yeah, that little detail is why I went looking for Kazan... >.>

Though if Karajorma added that argument, he ought to be able to fill out the sexp help. Assigning to him.

The_E

2012-12-10 17:47

administrator  

2733.patch (1,639 bytes)   
Index: code/autopilot/autopilot.cpp
===================================================================
--- code/autopilot/autopilot.cpp	(revision 9416)
+++ code/autopilot/autopilot.cpp	(working copy)
@@ -1447,7 +1447,7 @@
 	// Create the NavPoint struct
 	NavPoint tnav;
 
-	strncpy(tnav.m_NavName, Nav, 32);
+	strncpy(tnav.m_NavName, Nav, TOKEN_LENGTH);
 	tnav.flags = NP_WAYPOINT | flags;
 
 	Assert(!(tnav.flags & NP_SHIP));
Index: code/parse/sexp.cpp
===================================================================
--- code/parse/sexp.cpp	(revision 9418)
+++ code/parse/sexp.cpp	(working copy)
@@ -18292,12 +18292,13 @@
 				if (oswpt.team == Player_ship->team) {
 					add_for_this_player = true; 
 				}
+				break;
 
 			case OSWPT_TYPE_SHIP:
 				if (oswpt.shipp == Player_ship) {
 					add_for_this_player = true; 
 				}
-
+				break;
 			case OSWPT_TYPE_WING:
 				for ( i = 0; i < oswpt.wingp->current_count; i++) {
 					if (Ships[oswpt.wingp->ship_index[i]].objnum == Player_ship->objnum) {
@@ -18305,15 +18306,14 @@
 					}
 				}
 
-			// for all other oswpt types we simply return
+
+			// for all other oswpt types we simply ignore this
 			default:
-				return;
+				break;
 		}
 	}
-
-	if (add_for_this_player) {		
-		AddNav_Waypoint(nav, WP_path, vert, 0);
-	}
+	
+	AddNav_Waypoint(nav, WP_path, vert, add_for_this_player ? 0 : NP_HIDDEN);
 }
 
 
@@ -26405,7 +26405,7 @@
 			else if (argnum==2)
 				return OPF_POSITIVE;
 			else 
-				return OPF_SHIP_WING_POINT;
+				return OPF_SHIP_WING_TEAM;
 
 		case OP_NAV_ADD_SHIP:		//kazan
 			if (argnum==0)
2733.patch (1,639 bytes)   

The_E

2012-12-10 17:48

administrator   ~0014410

Attached a patch that restores the intended behaviour (at least as far as I can tell what it should be).

karajorma

2012-12-12 09:55

administrator   ~0014426

The E seems to have a handle on what the fix should have done. The idea is that you can say the new nav point should only be visible to certain players, wings or teams.

I'll have a look in a bit and see if his patch solves the issue in a couple of hours.

karajorma

2012-12-12 12:46

administrator   ~0014427

Yeah, The_E's patch fixes the problem. I patched the code to solve the multiplayer issue and then no one ever got back to me on whether it worked or not. Which is why such poor code managed to survive that long.

The_E

2012-12-12 13:27

administrator   ~0014428

Fix committed to svn in revision 9422

Related Changesets

fs2open: trunk r9418

2012-12-09 17:47

The_E


Ported: N/A

Details Diff
Fix for Mantis 2733: Adds documentation for the 4th argument for the add-nav-waypoint sexp
Affected Issues
0002733
mod - /trunk/fs2_open/code/parse/sexp.cpp Diff File

Issue History

Date Modified Username Field Change
2012-11-22 19:47 FUBAR-BDHR New Issue
2012-11-22 21:33 Goober5000 Note Added: 0014151
2012-11-22 21:33 Goober5000 Assigned To => Kazan
2012-11-22 21:33 Goober5000 Status new => assigned
2012-12-05 18:56 The_E Assigned To Kazan => The_E
2012-12-05 18:58 The_E Note Added: 0014311
2012-12-05 20:44 Kazan Note Added: 0014316
2012-12-05 22:41 Goober5000 Note Added: 0014319
2012-12-05 23:13 Kazan Note Added: 0014320
2012-12-09 22:17 The_E Changeset attached => fs2open trunk r9418
2012-12-09 22:17 The_E Note Added: 0014392
2012-12-09 22:17 The_E Status assigned => resolved
2012-12-09 22:17 The_E Resolution open => fixed
2012-12-09 23:30 FUBAR-BDHR Note Added: 0014394
2012-12-09 23:30 FUBAR-BDHR Status resolved => feedback
2012-12-09 23:30 FUBAR-BDHR Resolution fixed => reopened
2012-12-10 04:04 Goober5000 Note Added: 0014401
2012-12-10 04:04 Goober5000 Assigned To The_E => karajorma
2012-12-10 17:47 The_E File Added: 2733.patch
2012-12-10 17:48 The_E Note Added: 0014410
2012-12-12 09:55 karajorma Note Added: 0014426
2012-12-12 12:46 karajorma Note Added: 0014427
2012-12-12 13:26 The_E Assigned To karajorma => The_E
2012-12-12 13:26 The_E Status feedback => assigned
2012-12-12 13:27 The_E Note Added: 0014428
2012-12-12 13:27 The_E Status assigned => resolved
2012-12-12 13:27 The_E Resolution reopened => fixed