Author Topic: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]  (Read 10286 times)

0 Members and 1 Guest are viewing this topic.

Offline Parias

  • 27
Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Hi all!

I'm usually pretty dormant in the community and this is my first attempt at a major contribution, so bear with me.

For the last few weeks I've been intensely engaged in a project to try and dramatically improve the co-op experience in FSO-based games. This has mainly been oriented around two goals:

* Enable Command Briefings for multiplayer
* Enable Cutscenes for multiplayer

Yeah, there are other issues such as the quirky netcode and inconsistent network replication for newer code features, but that's a little outside my current scope of knowledge to address (yet ;) ) But something I do value greatly is the ability to have the same experience in co-op multiplayer that you can in singleplayer - that is, all cutscenes, and all the command briefings, propogated correctly to all multiplayer clients, all seamlessly integrated, with the same animations, text overlays, etc.

Having spent a couple of weeks giving myself a literal crash-course in the game's source code and scripting system (seriously, I still barely know LUA or C++ at all), I think I've managed to pull together a creative solution for this. Behold - this is a snippet from my primary goal of porting the entire Freespace 1 campaign (with working cutscenes and command briefings) to co-op, which will be coming out soon:


This isn't necessarily the most elegant solution, because the absolute greatest approach would be to just integrate all this into the native codebase. However, since that would require an extreme amount of work, I've come up with this workaround:

* Command Briefings are individual missions built into the co-op campaign structure. When started, multi-eval and show-subtitle SEXPs are used in combination with a bit of crafty scripting to present the CommandBriefing overlay, as well as play the associated animations (thanks for the assistance guys!). The briefings are a bit crude and hotkey (not mouse)-driven, but work consistently for both host and client players in presenting the briefing data. At the end, the command briefing "system" cleans up and players can move on to the next actual mission in the campaign.

* Cutscenes work by a script that hooks into GS_STATE_BRIEFING (i.e. right after a mission loads). It'll scroll through a list and see if the mission we're on has a corresponding cutscene, and if so, play it using a new scripting function I exposed (tentatively labelled playCutscene() ), which just gives me an easy ability to call the internal playmovie() function whenever I want. This script processing is local and so works seamlessly for both host and client at playing cutscenes.


What I need to finish this:

The problem is that to get all of this working, I've had to make a few discreet code changes - some a bit more questionable than others. I've submitted Mantis tickets regarding these changes already so they can be reviewed properly, as I'm not sure if what I've done is really for the best. I know the code maintainers are busy and all requests will be looked at in due time, but was hoping to put some additional context and attention around these particular requests to see if there was a chance of getting them wrapped up a bit sooner for my co-op mod.

I've already had some help in getting multi-eval added to trunk (http://scp.indiegames.us/mantis/view.php?id=3079) and fixing a nasty memory problem (http://scp.indiegames.us/mantis/view.php?id=3084), and have submitted additional Mantis tickets for what's left:

* show-subtitle-text SEXP causes MP clients (but not host) to crash if message over 32 characters is sent (Mantis 3077)
Sort of self-explanatory. My current implementation for the FS1 Co-Op project relies heavily on show-subtitle-text for Command Briefings, which is supposed to support up to 255 characters per-messages. When playing multiplayer however, clients will crash out if they process messages over 32 characters. I have a minor code change that fixes this, but Karajorma's feedback is this may cause other problems. An ultimate solution would be welcome, as the only alternative is that I port everything over to use something else instead (maybe some kind of crazy gr.drawString() implementation) which would be a fair amount of extra work at this point.

* mn.getMissionFilenname() doesn't return a value if in GS_STATE_BRIEFING (Mantis 3093)
For my multiplayer-friendly cutscene script to work, I hook into the GS_STATE_BRIEFING state, iterate through a list of lines, and compare the results to the currently-loaded mission file. If we get a match (i.e. we're on a mission where a cutscene is supposed to play), the cutscene triggers. The problem is that mn.getMissionFilename() doesn't seem to work when you're entering the briefing stage because of a check to confirm if your game mode is "GM_IN_MISSION" or not. My workaround was to simply comment out this check and my script works perfectly, but I'm guessing this if-check exists for a good reason. Is there a better solution? I tried appending an OR check for GS_STATE_BRIEFING using other examples in lua.cpp as a guide, but this still returned nil values.

* Request for new scripting function: ts.playCutscene() (.patch included) (Mantis 3094)
A second necessary component for my multiplayer-friendly cutscene script to work. This adds the ts.playCutscene() scripting function (experimental), as currently multiplayer clients won't process the standard movie play requests when the host starts a campaign mission that's supposed to have a cutscene. A secondary problem to this I've noticed is that while the cutscene plays, network processing is suspended and you might return to find you've been booted out of the game with a "timeout" error. I'm looking at addressing this seperately, but for now the workaround is just to use the -timeout argument on your game client  and set it to a value higher than the cutscene play time.

And that's about it. One goal with this was to minimize any source code changes as much as possible, but in these cases I couldn't see any other way to accomplish my objectives (maybe I'm just not smart enough, heh). Input into these would be much appreciated.


Otherwise, the FS1 portion of this project is nearing completion - I just have to wrap up the last leg of the campaign and do some more internal testing. Even if all if these changes had to be compiled into a separate test branch of the code, I'd love if I could link to that rather than distributing my own NOTAVIRUS.EXE with the mod. I'll make a big announcement on the FSPort board when this is all ready to roll, and will then be moving on to the FS2 campaign.

Thanks! And apologies again if I'm going about this the wrong way - I'm excited by the opportunities this will bring the community to make FSO's co-op play a bit more popular and hope this turns out to be really useful.

(Here's the part where you tell me all of this has already been implemented in some manner and all my work is for naught...)
« Last Edit: September 17, 2014, 10:03:58 am by Parias »

 

Offline karajorma

  • King Louie - Jungle VIP
  • Administrator
  • 214
    • Karajorma's Freespace FAQ
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Sounds good. I'll be overhauling the multi_sexp system around the start of next month to deal with the issue I brought up in the Mantis 3077 report. I can definitely fix the show-subtitle-text multiplayer functions then but it should be possible for any other coder to fix it the way you described before that.
Karajorma's Freespace FAQ. It's almost like asking me yourself.

[ Diaspora ] - [ Seeds Of Rebellion ] - [ Mind Games ]

 

Offline Parias

  • 27
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Thanks Karajorma. I should mention I might have actually just run into the symptoms you described in Mantis 3077 too - suddenly the multi_reduce_counts void in multi_sexp.cpp started bombing out when I hit certain stages of my command briefings (client-side only of course, not host-side).  Debugging showed this was because current_argument_count was coming up as -1 somehow; the trigger seemed to be me calling multi-eval in the middle of several show-subtitle SEXPs (all of them over 32 characters of course).  In debug this also of course threw the "multi_get_x function call has read an invalid amount of data" warning and ultimately led to an assert.

I fixed the problem by shuffling my SEXP order so that all of the show-subtitle calls were sequential, and my multi-eval call only appeared at the very end. I can't definitively confirm what was causing the issue on a code level (I was even using breakpoints to inspect the received data packets on a per-execution basis step by step - if malformed / invalid data was coming in due to the strings being too large, it wasn't obvious) and things are running stable again now that I've made the change, but it does seem like the multiplayer SEXP handling can become unhappy very easily if you're firing big strings over the network. I'll be looking forward to your overhaul!

Anyway, on a somewhat different topic: I just realized that the red-alert SEXP doesn't exactly work correctly in multiplayer. As per your guide (thanks for writing this) I've taken a crack at trying to convert the SEXP to a multiplayer-friendly format, but Red Alert is a very special condition and I think more work beyond the norm is needed for this. I actually can't find any voids for it in sexp.cpp, just the OP_CASE entries and such. Just for kicks I tried copying it (and it's call to red_alert_start_mission()) into multi_sexp_eval(), and the Red Alert start *will* trigger for both host and client players, but the current status of ships (i.e. type, loadout, and condition in the last mission) won't be carried over.

I figured the host would be authoritative for this stuff and processing for this would 'just work', but it sounds like making this function correctly in multiplayer might be a bit more ambitious? I was considering maybe just leveraging the ship save / load script Admiral MS had come up with instead, but that'll require a bit more work to integrate. It'd be cool to find a way of making red-alert work as seamlessly as possible.

Edit: Actually, maybe I'm wrong - reading through redalert.cpp shows several conditionals for GM_MULTIPLAYER in a few functions, so it sounds like maybe it should be working on some level? I'm not sure now.
« Last Edit: August 25, 2014, 04:45:06 pm by Parias »

 

Offline karajorma

  • King Louie - Jungle VIP
  • Administrator
  • 214
    • Karajorma's Freespace FAQ
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Yeah, that definitely sounds like the issue I mentioned. Reordering the SEXPs and having them suddenly start working seems to confirm it.


As for Red Alert, I'm rather surprised it worked at all to be honest. I don't think anyone has tried to check if that code works in multiplayer.
Karajorma's Freespace FAQ. It's almost like asking me yourself.

[ Diaspora ] - [ Seeds Of Rebellion ] - [ Mind Games ]

 

Offline Parias

  • 27
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Heh, turns out it really doesn't... after spending a few hours exploring this, I eventually decided to just give up and use a workaround instead. With a bit of creative mission scripting, Admiral MS' save/load script actually works almost flawlessly. I've adapted my mod to use that instead. There were a few issues I found while using it involving ship-vanish / destroy-instantly in that the host would see applicable AI ships get "removed" but the client wouldn't in my custom "red-alert" missions - I've put in a couple of throwaway code fixes that seem to address this and am investigating further before submitting the changes for review.

In any case, I've submitted one more Mantis ticket important for this mod: 3102, which is a fix for the "subtitles get shifted weirdly if the host is running a different resolution than the client" issue. Not critical but would be nice to have a validated fix for (I've of course included my own example code fix which seems to resolve the problem, as far as I can tell - hopefully it's as simple as that). This is necessary to ensure text lines up correctly for all clients as well as the host during my hacked-up Command Briefings, because of the crazy way I'm relying on show-subtitle-text for this.

I also completed 3-way co-op multiplayer testing over the last weekend with extremely promising results - FS1's campaign was a ridiculous amount of fun with 3 people. Like.. an INSANE amount of fun, as in this is my new most favorite way to play the game (now that it's not missing storyline content) :D All of the mission / campaign content has been converted, all of the cutscenes function, and all of the "red-alert" bits seem to function as they should. It was awesome, and I can't wait to get ST:R, FS2, and even potentially other campaigns and mods beyond those ported over in this manner (not to mention Diaspora - which my local LAN friends are begging for - but I don't want to step on your toes with that one)

We did still run into a few random odd client issues during the command briefings relating to the same SEXP handling bug we've been talking about (I thought I had licked it by shuffling the event order around a bit... alas), but I'm not sure if there's anything more I can do about them until you have a chance to overhaul the multiplayer SEXP system. I'll do a bit more of my own digging for the interim, if only because a lot of the multiplayer code baffles me and I'm still really unclear where the breakdown in multi_reduce_counts begins - I'd like to learn more.

Otherwise, I think I'm nearly ready to post a public beta release for the FSPort version of the mod. I don't want to put any extra pressure around you guys for getting my remaining changes (necessary for the mod) reviewed / integrated into trunk though; would you suggest waiting for all of the changes to make it into trunk before posting a beta, or would it be OK if I posted it with my own test build for the time being? I've been hesitant to do so, but at this point I think I'm at that stage of development.

 
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Parias this sound awesome! Me and my friends are excited to try it out. I remember a few years ago my brothers and I tried playing the coop missions in multi but the lack of cutscenes and command briefings really took the flavor out of the campaign. I can't wait to get my hands on the beta and try it out.

 

Offline Parias

  • 27
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Glad to hear it Halcyon! I've finally snuck the beta out - you can check it out here: http://www.hard-light.net/forums/index.php?topic=88385.0

I think I'm about done enumerating all of the code changes I've made to get this working into Mantis tickets / code submissions. I still do have one more I need to fire out for multiplayer behavior involving ship-vanish and destroy-instantly (I mentioned these earlier), but I still want to do some additional testing around them to confirm if any changes are actually needed here.  I'll update when this has been submitted.

 

Offline Cyborg17

  • 29
  • Life? Don't talk to me about life....
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Why do you need to use ship-vanish?  I know the mission where you capture the Ramses uses it for the Taranis if you disable it, but that's poor mission design.

 

Offline Parias

  • 27
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Nothing mission-specific - ship-vanish is used in my case by Admiral MS' ship save/load script to "remove" ships that don't have valid data (i.e. they were destroyed or departed before the end of the mission).  The concept is that you load the ship data right when the next mission in a red-alert sequence starts, and it immediately clears any ships that weren't supposed to be there by using ship-vanish.

This is used as a workaround because red-alert missions don't function correctly in multiplayer. Instead, I load the missions normally (no red-alert flags) and use the script to manually save / load data for individual ships at the right timeframes. This provides the same effect in a co-op campaign.

Anyways, I've seen weird behavior from ship-vanish in multiplayer involving AI-controlled ships; on the host system the ship will disappear, but on client systems the ship will remain. Things like that. I think I fixed the problem through some source code changes, but I kinda took a shotgun mentality to it in doing so and need to confirm exactly what I did that fixed it.

(I obviously don't use anything destructive on an actual player ship in this way as it makes multiplayer VERY unhappy; instead I do a check and just give the player a generic replacement ship if he was dead in the previous mission. Only previously-dead or departed AI ships are annexed.)

  

Offline Hellzed

  • 28
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Do you think a nice "respawn" mechanism would be that surviving players could call in dead players as reinforcements ?
(with some time/ticket limitations, or at fixed points in the mission, like respawn in Left4Dead 2)

 

Offline Parias

  • 27
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Do you think a nice "respawn" mechanism would be that surviving players could call in dead players as reinforcements ?
(with some time/ticket limitations, or at fixed points in the mission, like respawn in Left4Dead 2)

Might be neat! But... I'll take a pass on trying to code all that in, for now. ;)

Moving on - oy! Half a day wasted...

I haven't been keeping up on some of the latest code changes (got distracted by a little side project), but I'm getting back on track now and was taking a look at Mantis 3102 so I could finally get an updated .patch consistent with the latest trunk revision submitted.

I guess the build I was originally working with was actually pretty far out of date, so I just grabbed a totally fresh build and went about re-implementing my remaining changes from scratch. However, any time I booted my co-op mod and loaded a command briefing mission, FS2 kept bombing out with the following:

Code: [Select]
Argument count is illegal.

In sexpression: ( when
   ( has-time-elapsed 0 )
   ( multi-eval
      "readfile.showcboverlay(1)"
   )
   ( play-sound-from-file "Brief1.wav" 1 )
   ( set-camera-position 0 0 0 )
)
(Error appears to be: multi-eval)ERROR: Argument count is illegal.

This had never happened in my prior builds.. after several hours trying to figure out how the "Argument count is illegal" check even worked, I realized where the problem was.

Has anyone made further changes to line 685 in sexp.cpp (one of the multi-eval definitions)? It's this:

Code: [Select]
{ "multi-eval", OP_SCRIPT_EVAL_MULTI, 2, INT_MAX, SEXP_ACTION_OPERATOR, },

For my request to get multi-eval integrated (Mantis 3079), the third value (minimum number of arguments, I think) was at 1. However in the latest trunk build, it looks like this got switched to 2. This breaks my existing co-op missions because multi-eval is supposed to only require one argument (the script to run) with a second "optional" argument (which players to send it to, or all clients if no argument is given) - this change seems to make the SEXP require two arguments instead.  I'm a little confused on the changed requirements as none of my missions have this second argument, and I'm not familiar enough yet with the build system to check back and see the reason this change was made. I'm assuming there was a good reason, I'm just curious if I should make a Mantis request to get this switched back, or if I should go back and edit all of my briefing missions to conform to the new standard.

In any case, setting this back to "1" and recompiling got me back on track for now. :)

Edit: AHA! Maybe I found it after all: http://www.hard-light.net/forums/index.php?topic=88291.msg1762032#msg1762032

Code: [Select]
r11035 | karajorma | 2014-08-30 00:26:19 -0500 (Sat, 30 Aug 2014) | 1 line
Changed paths:
   M /trunk/fs2_open/code/parse/sexp.cpp

The previously committed version of multi-eval did not include the code to allow it to determine if client side scripts should also be executed on the server. This version does but will break any previously created missions.

I'm guessing the minimum number of 'required' arguments for this SEXP was a part of this change then. I'll edit my missions to take into account the new argument requirement.
« Last Edit: October 05, 2014, 05:13:47 pm by Parias »

 

Offline karajorma

  • King Louie - Jungle VIP
  • Administrator
  • 214
    • Karajorma's Freespace FAQ
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Yeah, I probably should have posted here when I committed that change. Sorry about that.

As stated in the message, the previous version caused all scripts to also execute on the server which might not be desired behaviour (Especially in the case of any scripts doing something to do with graphics on a standalone server).

Changing that value back to 1 probably won't help because the problem was that I had to insert the second option (Whether to execute on the server or not) in front of the optional list of clients to update. Which means that the game will expect to read a value there from the SEXP during the game. When it doesn't find anything, it will mark that as false and it won't execute the script on the server, only on all the client machines. It is a better idea to simply insert a true / false into the SEXPs using notepad (or FRED if it will let you) than to hack it in this way.

I probably could make that second argument optional fairly easily but the problem is that my second PC is broken at the moment so I have no way to test multiplayer behaviour properly. Since we're probably the only people using multi-eval, I'll leave it up to you whether you think it would be worth doing that as you're going to have to be the one to test if it broke something.
« Last Edit: October 06, 2014, 12:04:29 am by karajorma »
Karajorma's Freespace FAQ. It's almost like asking me yourself.

[ Diaspora ] - [ Seeds Of Rebellion ] - [ Mind Games ]

 

Offline Parias

  • 27
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
That's ok - as you probably noticed, I'm getting good at rooting these things up, heh. And at least it was a good excuse to learn a bit more about the code.

I was actually thinking about that too and wondering if the second parameter could still be made optional. Your comments make sense - I can understand wanting a bit of extra control for a standalone server case, but I'm actually a little unclear on the actual intended behavior. Bear with me:

If the optional parameter is set to false and clients join a standalone server, the script would run on all client systems - but the game instance the standalone server is running would not see script execution, correct?

What about if someone hosts a non-standalone server via the in-game system and plays that way - if the parameter is false, does this mean the host / server player (who's actually in the game) would still not see script execution, or is this a particular case where they still would? I do see this code in sexp_script_eval_multi:

Code: [Select]
// if this is me, flag that we should execute the script
if (p == Player) {
execute_on_server = 1;
}

Does this mean if "execute_on_server" is false, but the host / server player is hosting and playing from in-game anyways, that they would still see script execution after all because they've been assigned an actual player slot?

If that's the case, it almost sounds like this would be a consideration for standalone servers only - which is still probably important, but might be justification to make it optional and keep it "false" as the default.

Or I'm just totally misunderstanding things :) In any case, if you can whip up something that makes this optional (or advise where / how I can do it - is doing it properly more than just changing line 685 in sexp.cpp?), I have a multiplayer-ready testlab I can definitely validate this against.

 

Offline karajorma

  • King Louie - Jungle VIP
  • Administrator
  • 214
    • Karajorma's Freespace FAQ
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Basically the way the sexp works is this. If the execute on server option is set to true, then the script will always execute on the server regardless of what kind of server you have. You use this if the script is doing something that must always be done on the server (but which also needs to be done on one or more clients).

If the server option is set to false then the server does not execute the script UNLESS the player is on the list of clients who need to be updated. So if we have this

Code: [Select]
multi-eval
-SomeGraphicsScript(1)
-false

or

Code: [Select]
multi-eval
-SomeGraphicsScript(1)
-false
-Alpha 1
-Alpha 2
-Beta 1
-Beta 2

then a standalone server wouldn't run the script but a player server with Alpha 1 on it would. (In the first case all clients are being updated since a list of clients wasn't given).



Why would you need this? Suppose you're using scripting to make it so that ships can cloak and decloak themselves during a multiplayer mission. You'd want the players to get the update whether or not they are on the server or not, so you might think a default of

Code: [Select]
multi-eval
-SomeGraphicsScript(1)

would be enough. And sure enough it would seem to work. But when you release the mission you start getting reports of crashes. You quickly discover the cause is that they are running the game on a standalone server and as soon as your script starts messing around with the graphics the server is crashing because standalone servers don't bother to initialise the graphics engine at all. You didn't see the error when testing cause you played on the server.


The reverse situation is also true (and this is why I actually coded the feature). Suppose the standalone doesn't crash and you want to make Beta 2 Decloak to Beta wing but not to Alpha wing. So you do this with the old SEXP

Code: [Select]
multi-eval
-SomeGraphicsScript(0)
-Beta 1
-Beta 2

and test on a standalone where it works perfectly. You release the mission and then start hearing complaints from players that the server can see cloaked ships. The reason is that the non-standalone player on the server is also executing the decloak script. If you change to this

Code: [Select]
multi-eval
-SomeGraphicsScript(0)
-false
-Beta 1
-Beta 2

the script won't run on the server and you'd get the correct behaviour.


Basically I added this option because otherwise you had many situations where you got changes in behaviour depending on whether you are executing on the standalone or a player server.
« Last Edit: October 06, 2014, 02:16:30 am by karajorma »
Karajorma's Freespace FAQ. It's almost like asking me yourself.

[ Diaspora ] - [ Seeds Of Rebellion ] - [ Mind Games ]

 

Offline Parias

  • 27
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Wow, okay - I can see the conundrum then.

Thinking about this though, from what you've described, it sounds to me like it would be okay to make the true / false parameter optional and make it "false" by default (i.e. fire the event to all clients but not expressly to the server); mod / mission authors would then need to know to expressly set this to "true" if they're making something that's dependent on server execution (particularly in a standalone setting).

Let's try putting it together, and I'll put this through some rigorous testing in my MP lab in both standalone and regular server settings to see how it holds up (it occurs to me that I might want to tweak a few of my missions after all, as my "red-alert" hack may need to make sure the server executes script events in light of what you've described). You're probably right in that we're the only ones actually using multi-eval for now, but all this of course will also lay some important framework for other multiplayer mods down the road.

 
Re: Now announcing: The FSO Co-op Campaign Enhancement Project! [Code Review]
Has this project been worked on this year?