[SparK]

[OLD] http://files.fringespace.org/spark/misc/reverse_afterburner.patch
[FINAL] http://files.fringespace.org/spark/misc/revAB.patch

I need opinions and reviews, some feedback and suggestions

the main idea is to be able to add
+Aburn Max Reverse Vel:      0.0
+Aburn Rev accel:         0.0

to the ships.tbl
these parameters are optional and if not present or zeroed the default behaviour will take place

I got help from a lot of people on IRC

[General Battuta]

This is awesome. I would like to see this feature very much.

[Dragon]

How would it work?

[General Battuta]

It would basically cause reverse thrust to behave like forward thrust.

[JGZinv]

Assuming Z is set to regular reverse...

Press Z + afterburners to go in reverse burn (aka backwards really fast).

[General Battuta]

I'll just repeat that this is fantastic and would do oh-so-much, especially if reverse afterburner speed can be set independently.

[Spoon]

I'll just repeat that this is fantastic and would do oh-so-much, especially if reverse afterburner speed can be set independently.

I for one wholeheartly agree with this.

[SparK]

Code: [Select]

+Aburn Max Reverse Vel:      0.0
+Aburn Rev accel:         0.0

any value you put there will be your reverse speed
you can set it to be sluggish by using 60.0 on max speed and 0.1 on accel
or you can make a super ship and use 300.0 and 5.0

[Mongoose]

Does standard backwards thrust already work in the same way as forward thrust?  I was footling around with ship tables a while back in an attempt to mimic Descent-style motion, and I seem to remember not being able to completely control how quickly the ship would transfer from full forward to full reverse velocity.

[General Battuta]

No. That's what the acceleration field is for.

[TrashMan]

This would be grand, I agree.

[Sushi]

Looks good overall. A couple of nitpicks/questions:

Code: [Select]

- if ( goal_vel.xyz.z < -pi->max_rear_vel )
+ if ( goal_vel.xyz.z < -pi->max_rear_vel && !(pi->flags & PF_AFTERBURNER_ON) )
  goal_vel.xyz.z = -pi->max_rear_vel;
Maybe it would make sense here to cap the rear velocity to afterburner_max_reverse_vel when appropriate instead? The fact that we are essentially removing a check when afterburner is on without explicitly handling the new case makes me a bit nervous.

Code: [Select]

+ goal_vel.xyz.z = ci->forward* pi->afterburner_max_vel.xyz.z;
+ if(ci->forward < 0.0f){
+ goal_vel.xyz.z = ci->forward* pi->afterburner_max_reverse_vel;
+ }
This works, but it's probably better to only assign once. In other words, format this as an if/else or as a ? : ternary.

Code: [Select]

ramp_time_const = pi->forward_decel_time_const;
- // hmm, maybe a reverse_accel_time_const would be a good idea to implement in the future...
+ if ( pi->flags & PF_AFTERBURNER_ON )
+ ramp_time_const = pi->afterburner_reverse_accel;
Same thing here.

Code: [Select]

+ /* SparK 2010-01-18: this guy is buggin me out, I need reverse burners to be an option.
  // AL 12-29-97: If afterburner key is down, player should have full forward thrust (even if afterburners run out)
  if ( check_control(AFTERBURNER) ) {
  ci->forward = 1.0f;
  }
+ */
Don't comment this out. If it really is unneeded logic that is already duplicated in physics.cpp, then just cut it out completely.

Code: [Select]

- dest_ci->forward_cruise_percent = 0.0f;
- dest_ci->fire_countermeasure_count = 0;
- dest_ci->fire_secondary_count = 0;
- dest_ci->fire_primary_count = 0;

- dest_ci->forward_cruise_percent = src_ci->forward_cruise_percent;
- dest_ci->fire_countermeasure_count = src_ci->fire_countermeasure_count;
- dest_ci->fire_secondary_count = src_ci->fire_countermeasure_count;
- dest_ci->fire_primary_count = src_ci->fire_countermeasure_count;
Why are you removing these lines?

[SparK]

ok, will do

but...   WTF the last lines! i didn't remove that... will fix, don't worry


edit:

fixed everything, except for that negative max speed checker because I'm setting goal_vel already, why would i check if its smaller than the value I just set it to?
(I don't think afterburner_max_reverse_vel can be smaller than afterburner_max_reverse_vel)

sorry for those erased lines, they were added in the last update i did but somehow i reverted my files to a previous release...

(patch on the same link)
http://files.fringespace.org/spark/misc/reverse_afterburner.patch

[Sushi]

I'd still change this:

Code: [Select]

+ if(ci->forward < 0.0f)
+                  goal_vel.xyz.z = ci->forward* pi->afterburner_max_reverse_vel;
+ else goal_vel.xyz.z = ci->forward* pi->afterburner_max_vel.xyz.z;

to this:

Code: [Select]

+ if(ci->forward < 0.0f)
+                  goal_vel.xyz.z = ci->forward* pi->afterburner_max_reverse_vel;
+ else
+                  goal_vel.xyz.z = ci->forward* pi->afterburner_max_vel.xyz.z;

in both places where it applies, but that's the only quibble I have left (and it's just a style-consistency nitpick ).

Otherwise... looks good to me. Anyone else have comments or suggestions on this patch?

[JGZinv]

All I can say is that it works... we needed it specifically for FringeSpace, and he completed it.
Does what it says it will do.

[chief1983]

Appears to, but that's the kind of thinking that's gotten us into messes before.  Like with changes to default AI behavior.

[SparK]

remember it does nothing without the + flags on ships.tbl

[Sushi]

remember it does nothing without the + flags on ships.tbl

Right. As long as the current behavior remains unchanged without the new flags (which, as far as I can tell, is the case) then we should be good.

[General Battuta]

Just wait until people start spotting the AI firing its afterburners at random non-tabled intervals! Then where will Sushi be!

[SparK]

this one works if you are on high forward speed too:
http://files.fringespace.org/spark/misc/revAB.patch

also Sushi, I fixed the else\n stuff
thnx Zacam for the feedback on irc