beyond-all-reason / spring

A powerful free cross-platform RTS game engine
https://beyond-all-reason.github.io/spring/
Other
224 stars 102 forks source link

weapons with LaserCannon type get less range than what the weapondef says #729

Open springraaar opened 1 year ago

springraaar commented 1 year ago

this is an old issue, was reported for engine 104 https://springrts.com/mantis/view.php?id=6384

The culprit code is this : https://github.com/beyond-all-reason/spring/blob/BAR105/rts/Sim/Weapons/LaserCannon.cpp#L32

    // round range *DOWN* to integer multiple of projectile speed
    //
    // (val / speed) is the total number of frames the projectile
    // is allowed to do damage to objects, ttl decreases from N-1
    // to 0 and collisions are checked at 0 inclusive

--------------- how to reproduce Can be reproduced on engine BAR105-1544, metal factions v2.07. Enable cheat mode, type

/give aven_skimmer (400 range cannon after weapondefs_post) /give claw_wolverine (400 range laserCannon after weapondefs_post)

to spawn them side by side. Select them and press A to view the range circles. The one for wolverine is noticeably smaller.

Type /give upgrade_red_1_range to trigger an upgrade of +3.5% range on all units, do so repeatedly and check how to range circles grow : skimmer's grows continuously, while wolverine's grows in steps of 67.73 every few upgrades


They should be allowed to have any range.

I think it may be easier to just use either the cannon or missileLauncher's behavior code and just render the projectile differently.

Cannon projectile's behavior would allow it to scale in range normally with heightBoostFactor and heightMod and also allow cylinderTargetting. MissileLauncher allows tracking, the trail, but doesn't get heightBoostFactor.

springraaar commented 1 year ago

Note that in MF v2.08+ claw_wolverine got a rework and the weapon uses a cannon instead, so the issue is no longer reproducible there.

MF v2.07 can be found here https://github.com/springraaar/metal_factions/releases/tag/v2.07 which corresponds to this commit : https://github.com/springraaar/metal_factions/commit/ca97be849550f31471b0082b4e9e369bcd822071

sprunk commented 1 year ago

ZK """workaround""" is to just yell at people to adjust projectile speed.

https://github.com/ZeroK-RTS/Zero-K/blob/master/gamedata/unitdefs_checks.lua#L40-L62

GoogleFrog commented 1 year ago

LaserCannon cannot simply have "any range". Ie there isn't just a line in the code to modify. It would need some fundamental change or new feature, as a result of it's current behaviour:

So what is to be done? I like having a LaserCannon that has its death timed to avoid overshoot, and I'm happy to tweak projectile speeds up or down a bit to hit whatever range I want. So you would have to add some sort of mode that disables the range sanitation step and gives LaserCannon one frame of overshoot, or perhaps something more complicated like giving it a frame of overshoot, but only doing the collision check on a truncated segment of its last frame of life.

springraaar commented 1 year ago

Even if it were replaced by cannon code, the default behavior should match the current one, that is, expiring at end of range (after accounting for heightMod and heightBoostFactor) in the direction it fired.

Range is a very important attribute, I'd rather have them match the range they're set and get automatically adjusted projectile velocity than the other way around.

sprunk commented 1 year ago

Perhaps a modrule with a handful of trivial adjustments? laserCannonAdjustmentMode = x, with the options being

Then you could set it to 2, ZK could set it to 3 and get rid of the gameside check that already achieves it, and (say) BAR can stay on 1 or something.

GoogleFrog commented 1 year ago

I feel like a modrule would just weigh down the engine with cases for little benefit. Each of your options can already be achieved with unitdets_posts. How about this:

The thing is, with raaar saying that he's fine with speed being adjusted rather than range, this ticket isn't for anyone involved in the ticket. Once you're aware of the requirement that LaserCannon range be a multiple of its per-frame speed, you can set any range you like with minimal tweaking of speed. There is almost no limitation here for those in the know. All we'd get with some last-frame interpolation feature is the ability to tweak projectile speeds by some small percent. So I think the ticket has become one about defaults, about friendly behaviour, rather than a feature any experienced dev needs.

If you really want to teach people about the relationship between range and speed, crash on load. If you don't want to be that extreme but want to increase their chances, warn in infolog.

springraaar commented 1 year ago

Seems like it should be a weapondef, not modrule.

Note that although it's not a direct request on this issue, using cannon would get it compatible with heightMod and heightBoostFactor, which the speed adjustment wouldn't fix.

sprunk commented 1 year ago

Each of your options can already be achieved with unitdets_posts. (...) There is almost no limitation here for those in the know

Yes, but the existence of an explicit modrule (also listed in the docs) draws attention to the fact that this is something a gamedev might want to be aware of and offers a few easy solutions. It draws people into the know.

Seems like it should be a weapondef, not modrule.

If you're tweaking the individual weapondefs you might as well tweak speed/range directly.

using cannon would get it compatible with heightMod and heightBoostFactor, which the speed adjustment wouldn't fix

At that point why not just use Cannon? Put myGravity = 0 and see if anything else is missing.

springraaar commented 1 year ago

because of the rendering type, cannon draws an array of blobby sprites on the projectile, the laser cannon draws a long side-texture and a front-texture.

Krogoth100 commented 1 year ago

To me too, it looks like you should just do the work in a def parser. Current behavior is reasonable, but parameters can be interpreted differently. That's exactly what a def parser is for: to match things like engine-side range with whatever range means for you, and so on.