View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002733 | FSSCP | SEXPs | public | 2012-11-22 19:47 | 2012-12-12 13:27 |
Reporter | FUBAR-BDHR | Assigned To | The_E | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.6.15 | ||||
Summary | 0002733: Add-nav-waypoint 4th argument | ||||
Description | Well 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 Reproduce | Search 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) | ||||
Tags | No tags attached. | ||||
|
Hoping Kazan is available to fill in help text for this... |
|
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. |
|
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. |
|
https://svn.icculus.org/fs2open/trunk/fs2_open/code/parse/sexp.cpp?limit_changes=100&view=markup |
|
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 |
|
Fix committed to trunk@9418. |
|
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. |
|
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. |
|
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) |
|
Attached a patch that restores the intended behaviour (at least as far as I can tell what it should be). |
|
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. |
|
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. |
|
Fix committed to svn in revision 9422 |
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 |