Hard Light Productions Forums
Modding, Mission Design, and Coding => FS2 Open Coding - The Source Code Project (SCP) => Topic started by: SparK on January 18, 2010, 01:35:24 pm
-
[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
-
This is awesome. I would like to see this feature very much.
-
How would it work?
-
It would basically cause reverse thrust to behave like forward thrust.
-
Assuming Z is set to regular reverse...
Press Z + afterburners to go in reverse burn (aka backwards really fast).
-
I'll just repeat that this is fantastic and would do oh-so-much, especially if reverse afterburner speed can be set independently.
-
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.
-
+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
-
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.
-
No. That's what the acceleration field is for.
-
This would be grand, I agree.
-
Looks good overall. A couple of nitpicks/questions:
- 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.
+ 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.
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.
+ /* 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.
- 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?
-
ok, will do
but... :eek2: 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? :P
(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
-
I'd still change this:
+ 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:
+ 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 :p).
Otherwise... looks good to me. Anyone else have comments or suggestions on this patch?
-
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.
-
Appears to, but that's the kind of thinking that's gotten us into messes before. Like with changes to default AI behavior.
-
remember it does nothing without the + flags on ships.tbl
-
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.
-
Just wait until people start spotting the AI firing its afterburners at random non-tabled intervals! Then where will Sushi be!
-
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
-
Here is an updated to Trunk version: 5845-RevAB.patch (http://zacam.ueuo.com/scp/Patches/5845-RevAB.patch)
This resolves some spacing changes in ship.cpp and ship.h.
Works as advertised, no new warnings in MSVC 2008.
EDIT:
Although, being able to use the existing "+Aburn Max Vel: 0.0, 0.0, 150.0" and adding in the speed to replace the 0.0, 0.0 (like we do for Glide n' Slide) and making it +Aburn For accel: and +Aburn Slide accel: would probably be handier for folks to use, but may or may not be more complicated.
Sushi and/or Backslash is probably the expert in this area though.
-
Although, being able to use the existing "+Aburn Max Vel: 0.0, 0.0, 150.0" and adding in the speed to replace the 0.0, 0.0 (like we do for Glide n' Slide) and making it +Aburn For accel: and +Aburn Slide accel: would probably be handier for folks to use, but may or may not be more complicated.
I'm afraid I don't understand what you are trying to get at. Care to explain further?
-
he wants lateral and vertical afterburners too
-
Yup. Sorry, I was verging on some serious need of sleep at that point last night.
Basically, Siled/Glide is able to use the X and Y in $Max Velocity. I'm suggesting that, rather than adding in "+Aburn Max Reverse Vel" that we use the X and Y values in "+Aburn Max Vel:" instead. The +Aburn accel functions could probably be entirely optional for Rev and Side.
(or better, added to "afterburner_forward_accel" to have it handle X Y and Z instead, but we might run into compatibility problems (potentially) as retail never stored X or Y there.)
-
the way i would do it is id give every degree of freedom its own max speed, acceleration, and deceleration values. technically speaking reverse acceleration would be the same thing as forward deceleration, since the means of slowing down is the same as the means for going in reverse. for slide (a term ive always considered a misnomer, but will use for the sake of sanity) the acceleration would be positive rate and your deceleration would be your negative rate, so for lateral slide, the acceleration would be how fast you accelerate to the left and deceleration being your acceleration to right. this is what my physics mod does, except uses forces instead of accelerations.
of course it seems like this would cause some compatability issues with retail tables, so just stick a slide accel in the afterburner section. +Aburn For accel: is a float and not a vector, and changing it to a vector will piss off every modder on hlp, unless of course if you detect this at parse time and if only one number is found, stick it into the z component, assume x and y are 0 and use the data as a vector, otherwise pars as a vector. the x and y max velocities should go where they go now. of course this is inconsistent with the way normal slide acceleration is handled, so then we go back to just adding +Aburn Slide Accel:
-
Well, although I agree that it might make sense to someday extend this to allow sidethrust afterburners as well, that is a bit more complicated and not needed by anyone at the moment. The reverse afterburners, on the other hand, are needed.
The good news is that the current additions to the table structure don't preclude the eventual addition of sidethrust afterburners. I do think that it's worth committing what we have now before worrying about extending it further, though.
-
...srsly?
If it'll be a nice addition and a worthy feature, why not make the commit complete and get it all in now, rather than gettin half in now, potentially forgetting about the rest until sometime later and then maybe getting it done then?
I'll take a look then at adding in the expanded functionality.
-
Sometimes it's not worth it to do 90% of the work to get 10% more feature. People have priorities.
-
And one of those priorities should be making features fully complete, rather than just doing the minimum necessary to implement the idea.
Now, seeing as how the feature works exactly as desired, I'm not laying this on the maker of the .patch file, but I do think a little for-thought and work on polishing it now and then putting it in would be a far better direction than just deciding "good enough" and saving it as is until later.
That is usually the root cause for why the majority of our current code looks the way it does, IMO.
-
Our current problems are due to badly coded features, not features that weren't taking every possibility into account. If it's not more work than it was to get this far, that's one thing, but if it's an order of magnitude harder to implement side thrust it doesn't make much sense right now.
-
My point was that the patch, as it is, should definitely go in. If you want to "complete" the feature further by making it support sidethrust as well, go right ahead, but there's no need to delay what we have so far in order to accomplish that.
-
Patch (as is) committed on revision 5858.
-
Great!
SparK, please make sure you update the freespace wiki to reflect the new table entries.
(http://hard-light.net/wiki/index.php/Ships.tbl)
-
Actually, I went ahead and did that too.
Feedback on the formatting would be nice, since $Afterburners did not sub-tree it's section very well, I altered that.
-
The formatting on the wiki looks good to me :yes:
Looking forward trying backwards afterburner (will give it a try the moment the nightly 5858 for windows is out)
-
Okay i've just tested it out. There is one issue with it however, if you are not flying at full forward speed the normal afterburner will not work properly. Instead of accelerating you to max normal speed and then exceeding that amount, it now only adds a small percentage ontop the speed you are currently flying.
So for example if the ship you are flying has a top speed of 140 and you set your speed to 30 and then trigger afterburners you'll only accelerate to say, 43. While consuming fuel etc.
The backwards afterburner works fine as long as you keep Z+tab both pressed.
-
Well ****. Confirmed.
And if you are at absolute zero, you don't go anywhere at all.
-
Seriously guys? Abort Retry Fail :P
-
It only affects ships where a TBM is used to specify a reverse AB speed.
-
5863 Fixes this issue. Now even with a TBM defining a ship to have Reverse AB, standard forward AB behaviour is retained.
(Zero or non-full Forward Velocity now accelerates to Max Afterburner, unless and until Reverse Thrust is also used)
-
For the record when I tested it, I added it to ships.tbl not in a seperate .tbm
-
Should still apply regardless of whether it was in a TBL or merged to a TBL by a TBM because in the end, it all goes to the same place.
But good to know, and just to make sure I'm not wrong, I'll test via TBL as well.
And yup, works when used in a TBL as well.
-
oh you guys found that too? XD
simple fix is add
if(ci->forward < 0.0f)
ci->forward = -1.0f;
else
ci->forward = 1.0f;
right after the check on physics.cpp, next to that comment with my name
-
Hmm. Not the approach I took, but I'll try it out, seeing as how there is a reported case where in forward AB (vs non-tbm'd for Reverse AB) the falloff back to normal speed is taking _much_ longer.
-
Okay, no changes needed to be made. The reported case wasn't a cause of ReverseAB, so I call it good until it's decided to expand it's functionality later.