fs2open.github.com icon indicating copy to clipboard operation
fs2open.github.com copied to clipboard

ship-maneuver changes cause ludicrous speed

Open MjnMixael opened this issue 6 years ago • 24 comments

BtA's mission Rebels and Revolutionaries displays this problem.

Oct 24 build, the Seeker ships fly at a normal speed (about 20 m/s). This is done with a ship-maneuver sexp to tell the ship to fly forward at 100% velocity while rotating slightly.

After Oct 26 changes submitted by @Goober5000 the Seeker ships fly off into the sunset at +160 m/s.

MjnMixael avatar Nov 25 '19 16:11 MjnMixael

Screenshot... https://cdn.discordapp.com/attachments/223511363807346691/648545575381237781/screen0037.png

Goober5000 avatar Nov 25 '19 18:11 Goober5000

I tried the most recent build from source, and that worked perfectly fine. Next I tried the 20191112 build, and that worked too. Finally I tried the 20191026 build, and that also worked. In all three builds, both Seeker 1 and Seeker 2 went 20 m/s as expected. I confirmed that I was running the proper builds by checking the build ID string in the main hall.

Goober5000 avatar Nov 25 '19 21:11 Goober5000

Is this still a problem?

Goober5000 avatar Apr 13 '20 21:04 Goober5000

Unsure. It has been on my list to look in to again and I just haven't.

MjnMixael avatar Apr 13 '20 21:04 MjnMixael

Is the rotation on any one axis? Bank, perhaps?

z64555 avatar Dec 08 '20 02:12 z64555

We don't have reproduction and MJN has too much on his plate in RL to really look at this. I don't think we can easily fix this right now in the next few weeks.

But if anyone is looking into it, I would suggest looking at the fact that the object was docked to 30 other objects and not just looking at the sexp. I'm not sure if @Baezon's changes to the moment of inertia for docked objects may have inadvertently fixed this as well, because notice that the sexp was slightly turning the object.

But like I said, I just don't think we can fix this without being able to reproduce it, and it may be a while before MJN or someone else can give us a good handle on how to do that.

JohnAFernandez avatar Dec 08 '20 03:12 JohnAFernandez

I'm with Cyborg here, without a reliable means to produce it there's not much we can do. I don't feel confident enough to close it, but I don't think it's immediately urgent enough to warrant a high priority for the stable.

Baezon avatar Dec 08 '20 03:12 Baezon

Alright, bumping it down to Low for now

z64555 avatar Dec 08 '20 16:12 z64555

BTA team has not seem to encountered it recently. Closing until there is a new report.

JohnAFernandez avatar Jun 21 '21 01:06 JohnAFernandez

This showed back up again for a player. See here.

I took another look at the mission. I noticed two things. ship-maneuver has an optional 11th argument now that is a bitfield. Two things that can be turned on with that argument caught my eye. One allows the sexp to move the ship greater than tabled' values and the other keeps old ship-maneuver values.

The other thing I noticed was that my Events that trigger the ship-maneuver were repeat firing (had a -1 trigger count and a 3 second interval). This should not cause ship-maneuvers to build up.. but these are the only ship-maneuver sexps in the mission and the ships themselves have no waypoints. Their movement is entirely controlled by these Events.

So, with all that in mind, I would suggest looking in two places for any obvious errors. First, check that the optional argument defaults to all options being off. My Events to not specify the optional arguments because the mission was made before those existed and this issue started after they were added in PR #2118 according to my original note. I would also check that if those defaults aren't OFF if not specified, then make sure that the bit that allows maneuvers on top of maneuvers is working as intended.

MjnMixael avatar Dec 16 '21 22:12 MjnMixael

That's good detective work. If I have some time, I'll try to look at it this weekend, checking for exactly what you're talking about.

JohnAFernandez avatar Dec 16 '21 23:12 JohnAFernandez

I had some time tonight. The initial variable that takes the 11th argument seems to be properly zeroed, but maybe there's a problem if the 11th argument is not specified. I'm going to keep digging tomorrow night.

JohnAFernandez avatar Dec 17 '21 04:12 JohnAFernandez

To follow up on this, the optional 11th argument will default to the options being off if they are not specified.

However, the bit that adds maneuvers on top of maneuvers may indeed be the culprit. In the function sexp_set_ship_man, the CIF_DONT_OVERRIDE_OLD_MANEUVERS check will only reset the flags themselves. The rest of the values (rotation and translation) are not zeroed out.

This flag was originally added by @Baezon so I'll ask for his input.

Goober5000 avatar Jan 18 '22 05:01 Goober5000

I created a test mission based on the BtA mission in question, but I was unable to repro.

I don't think there's anything more we can do about this until we have a reliable repro (sorry Mjn).

To make testing easier, I created a "modpack" VP with all the non-retail assets needed to run the mission. To get GH to accept it as an attachment, I put a .7z inside a .zip. test_2157.7z..zip

jg18 avatar Feb 26 '22 23:02 jg18

Given my previous comment about being unable to repro this, closing it for now.

We can always re-open this issue if/when someone can repro it, for which the test mission/"modpack" in the last comment might help.

jg18 avatar Mar 11 '22 21:03 jg18

Re-opening per SCP standard procedures, although the ticket status is effectively unchanged.

jg18 avatar Mar 12 '22 16:03 jg18

So, I investigated some more, and I think there are some important facts that need to be mentioned.

  1. The freighter that is moving at excess speeds is docked with many pods that are spitting out fighters.
  2. None of the other ships in the arrangement have maneuver sexps or invulnerability of any kind.
  3. The freighter is not getting its forward thrust from the maneuver sexps but from it trying to complete its waypoints.
  4. Its speed is not a constant acceleration. At one point, it slows down and then speeds right back up. See the screenshots from the one time the bug was caught on camera. Screenshot 1, at 0 seconds its speed seems to be at least 160. Screenshot 2, at 2 seconds, it drops to below 100, and then, screenshot 3 less than a second later, it picks right back up. If the AI were behaving like normal, just with its max speed flag set, then it wouldn't have such wild swings in speed. It is very likely that the instantaneous acceleration flag is set as well. It is impossible to tell if the don't bank when turning flag is set.

Screen Shot 2022-06-11 at 2 53 43 AM Screen Shot 2022-06-11 at 2 53 40 AM Screen Shot 2022-06-11 at 2 58 32 AM

If the acceleration were caused by some weird collision bug, then it wouldn't be likely to start and stop, It also wouldn't get faster over time, but would reach some equilibrium speed, and the hud gauge would flash.

The real question is, what value is the flag argument set to if it is not specified. Because if it's set to -1, then because of signed ints, it would make all flags true.

JohnAFernandez avatar Jun 11 '22 17:06 JohnAFernandez

In sexp_set_ship_maneuver() maneuver flags is properly initialized to 0.

Baezon avatar Jun 11 '22 17:06 Baezon

I realize this, but it's getting its value from maneuver_flags = eval_num(n, is_nan, is_nan_forever); after. So the question remains, what is the value of the node when it is not specified in the mission file.

JohnAFernandez avatar Jun 11 '22 17:06 JohnAFernandez

If the node doesn't exist then the (n >= 0) check won't be true, and eval_num() will be skipped, leaving it as whatever value it was before.

Baezon avatar Jun 11 '22 17:06 Baezon

So we need to make sure the node doesn't exist, because there is literally no other explanation. As you said, maneuver_flags initialized to zero, and there's no other way in the codebase for that flag to be set, since they weren't using scripts.

JohnAFernandez avatar Jun 11 '22 17:06 JohnAFernandez

Being a bug, I assure you it's entirely possible there are several other explanations. By their very nature they are unpredictable, it may not even be related to the maneuver flags at all, that's just a theory. It could be any number of other things related to the AI or the physics, things which are also important elements of this situation. It could even be some sort of memory corruption, who knows. Auditing the maneuver code is good, but it's not the only possible culprit.

Baezon avatar Jun 11 '22 17:06 Baezon

In investigating A FCW related bug with ship-maneuver that may in fact be the same bug as this, my finding has been that... ship-maneuver sets some ai_override_flags ai_control_info_check() in aicode.cpp sets some physics info flags in response to the ai_override_flags if the ship is under AI control. I haven't hard confirmed the sequence of events because I don't know how this sexp works or how any of the related physics, ai or controls code works, but it appears to me that if the AI control ends before the ship-maneuver duration ends then some flags will be left in the physics info flags.

EatThePath avatar Jul 10 '22 03:07 EatThePath

Punting to 22.4 due to this issue's fickle nature.

z64555 avatar Jul 23 '22 13:07 z64555

It's been six months since the merge of #4483 and I have not seen or heard of this bug rearing up again in BtA. It seems likely that fixed whatever was causing the problem.

MjnMixael avatar Jan 05 '23 16:01 MjnMixael

We can always reopen if there is another report. Thanks for confirming!

JohnAFernandez avatar Jan 05 '23 17:01 JohnAFernandez

Ironically I may have encountered this bug just last night. I discussed it briefly with @Baezon and we will try poking at it a bit more.

The symptom I saw was that one ship-maneuver sexp was causing multiple speed changes. It did not accelerate to ludicrous speed, but it did increase past the speed limit it was given.

Goober5000 avatar Jan 05 '23 17:01 Goober5000

@Baezon discovered the cause of the acceleration and it was a different bug, likely not related to this. So I'll close this again.

Goober5000 avatar Jan 05 '23 21:01 Goober5000