-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding TTL to CWeapon #1893
base: master
Are you sure you want to change the base?
Adding TTL to CWeapon #1893
Conversation
rts/Sim/Weapons/Weapon.h
Outdated
@@ -151,6 +151,7 @@ class CWeapon : public CObject | |||
int nextSalvo; // when the next shot in the current salvo will fire | |||
int salvoLeft; // number of shots left in current salvo | |||
int salvoWindup; // delay before first shot (in frames) | |||
int ttl; // flight time for missile type weapons |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe comment should be "projectile flighttime" since it's not just for missile type weapons, also maybe should mention it refers to frames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed the comment to say projectile type weapons. It does set the ttl
field in the projectile params.
It seems Starburst weapons/projectiles are complicated and do their own thing. I will leave them alone in this PR. |
@@ -39,9 +39,10 @@ void CStarburstLauncher::FireImpl(const bool scriptCall) | |||
params.end = currentTargetPos; | |||
params.speed = speed; | |||
params.error = aimError; | |||
params.ttl = 200; //??? | |||
// Projectile TTL (params.ttl) is ignored by the Starburst Projectile and it only uses the weapondef. | |||
// I tried overriding the projectile TTL to 1 and it caused the starburst rocket to fly off into space |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it goes up until uptime runs out, and only then is ttl evaluated, so you need to set a ttl a bit higher than uptime in that case to see an effect. Could make the logic of how ttl and uptime interact a bit more intuitive maybe, but not in scope of this pr.
@@ -39,9 +39,10 @@ void CStarburstLauncher::FireImpl(const bool scriptCall) | |||
params.end = currentTargetPos; | |||
params.speed = speed; | |||
params.error = aimError; | |||
params.ttl = 200; //??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can make this pass ttl on and then make StarburstProjectile.cpp not overwrite it from weaponDef. That way ttl should also be controllable for starburst.
@@ -110,27 +110,27 @@ void CCannon::FireImpl(const bool scriptCall) | |||
launchDir += (gsRNG.NextVector() * SprayAngleExperience() + SalvoErrorExperience()); | |||
launchDir.SafeNormalize(); | |||
|
|||
int ttl = 0; | |||
int thisTtl = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't like Ttl form personally, but not the only place that writes it (Ttl) like that (vs TTL). Also maybe consider myTTL or myTtl instead of thisTtl/thisTTL, I think it's more in line with style around these files.
Anyways not really an issue since it's more of a personal preference thing, you can choose any you want from thisTtl/thisTTL/myTtl/myTTL
@@ -2206,6 +2206,10 @@ static bool SetSingleUnitWeaponState(lua_State* L, CWeapon* weapon, int index) | |||
weapon->collisionFlags = lua_toint(L, index + 1); | |||
} break; | |||
|
|||
case hashString("ttl"): { | |||
weapon->ttl = lua_toint(L, index + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use seconds (see e.g. line 2176)
weapon->ttl = lua_toint(L, index + 1); | |
weapon->ttl = (int) (lua_tofloat(L, index + 1) * GAME_SPEED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can make it a bit awkward if a specific number of frames is desired.
If changing it to seconds, then SetSingleUnitWeaponState key should maybe be called since ttl is in frames for projectiles Get/SetProjectileTimeToLive.flightTime
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can control frames perfectly by multiplying by frames per second. Calling it flightTime
sounds fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually... weapondef flightTime is also in frames, probably better to have all in frames for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it's not.
spring/rts/Sim/Weapons/WeaponDef.cpp
Line 123 in d9dcff2
WEAPONDUMMYTAG(float, flighttime).defaultValue(0).scaleValue(GAME_SPEED).description("Lifetime of the projectile, in seconds. Missile/Torpedo/Starburst projectiles 'lose fuel' and fall down; Cannons explode; others fade away"); // needs to be written as int and read as float |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, perfect then, lets call it flightTime
and make it in seconds too.
Going on vacation for a week, will address the comments when I get back. |
Added option to set
ttl
inSetUnitWeaponState
, and added attl
field intoCWeapon
. This overrides the originalweaponDef->flighttime
so games can set weapon ttl per weapon.Addresses #1683
Tested with the following gadget in Zero-K:
Verified: