Per-turret disable/enable/burst mode #10532

Coolthulhu · Dec 18, 2014

Similar to individual engine selection, except for turrets.

Each turret can be enabled, disabled or in burst mode. Burst mode isn't offered for turrets with no burst capability. When selecting turrets, screen will center on highlighted turret's location with an 'X' denoting its position.

Adds a new int to amount/direction/open union (turrets didn't use this union before, right?). Currently uses only values 0, 1 and 2 for disabled, enabled and burst.

By default all newly installed turrets are disabled.

jokermatt999 · Dec 18, 2014

Awesome, I'd had a half assed, half finished version of this sitting around that I meant to work more on, but this looks much better than what I'd planned. Good work, and I'm definitely going to love this in conjunction with Blaze's turrets.

Rivet-the-Zombie · Dec 18, 2014

How about an option to tell mounted charge rifles what charge level to fire at?

KA101 · Dec 18, 2014

KA101 reviewed Dec 18, 2014

View changes

KA101 · Dec 18, 2014

src/vehicle.cpp

+    // cycle individual turret modes
+    if( has_mult_turrets ) {
+        options_choice.push_back(cont_turrets);
+        options_message.push_back(uimenu_entry(_("Control individual turrets"), 'x'));


Nitpick: Should be "toggle" the turrets lest folks think they can manually control a gun. Apart from that, sure, looks pretty good. Folks'll love this.

Coolthulhu · Dec 19, 2014

Removed hard-coded values. Should play more nicely with mods now (Blazemod only atm?).

Allowed burst and charge limiting. I couldn't think of a good way to set a charge level without buffing the turret hard or introducing ugly randomness that could cause loss of 3 turns and then 3 big fireballs. If the buffing is allowed, the change is trivial.

Managed to get burst limiting to play nicely with FIRE_x tagged weapons (flamethrower at least).

Removed some hacks like constant fuel cost. It is now read from weapon tags and if burst ends early, the unused part is not consumed. Still hardcoded for hydrogen weapons and weapons with CHARGE tag.

Fixed a bug where a weapon using gas/hydrogen/battery would discharge every single container of this type rather than breaking out as soon as enough fuel is consumed.

KA101 · Dec 19, 2014

KA101 reviewed Dec 19, 2014

View changes

KA101 · Dec 19, 2014

src/vehicle.cpp

+        vehicle_part &tr = parts[turret_index];
+        if( precise ) {
+            tr.mode = 
+                helper::to_int( string_input_popup( _("Limit burst/charge to?") ) );


My only objection is that this isn't available when firing the weapon on foot, so folks are gonna clamor for that ability. It may be worth making non-Blaze turrets (which already have this sort of thing, or at least the high-end ones do) need additional electronics to justify computerized burst-limiting/charge-control.

(Moved from your repo to the diff, more accessible, sorry)

Well you can technically do so on foot, you just have to fire it as soon as it reaches the level of charge you want to use. I'm guessing the turrets would work the same way, simply firing the moment they reach the desired level?

The NX-17, yes, but it doesn't specify charge levels. Burst-limiting, not so much.

BevapDin · Dec 19, 2014

BevapDin reviewed Dec 19, 2014

View changes

BevapDin · Dec 19, 2014

src/vehicle.cpp

@@ -4773,54 +4869,66 @@ bool vehicle::fire_turret (int p, bool burst)
    if (!gun) {
        return false;
    }
+
+    if( parts[p].mode <= 0 ) {
+        return false;


This is very annoying: I installed a flamethrower and activated turrets, for specific turrets the default mode seems to be 0, so the new turret was still inactive (mode==0) and this function returned 0, which immediately shut down all turrets, see vehicle::gain_moves line 4330 with this message: "The %s's turrets run out of ammo and switch off."

Edit: and you can not activate (via the new menu) the single turret, the option for the menu is not shown when there is only one turret. And activating one turret, but not another will still deactivate the whole turrets, making this feature and the new menu completely pointless )-:

BevapDin · Dec 19, 2014

BevapDin reviewed Dec 19, 2014

View changes

BevapDin · Dec 19, 2014

src/vehicle.cpp

            // consume ammo
-            if( charges >= parts[p].items[0].charges ) {
+            long charges_consumed = charges - charges_left;
+            charges_consumed *= charge_mult;


Have you tested this with a gun that has the FIRE_100 flag (and is not a flamethrower which would use the other branch)? player::fire_gun already removes 100 charges from the item in that case, and that is what remains in charges_left. fire_turret_internal is called with way to few ammo (charges starts with 1 for non-burst or burst value). So even if the gun has FIRE_100, it only gets 1 charge to use.

"Luckily" fire_gun does not care, but it still reduces the items charges by 100, therefor the "remaining" are -99. As you make sure at the end of fire_turret_internal that the remaining charge are at least 0 (why actually? there should be no need for it, just forward the number), the code in fire_turret thinks all charges have been consumed.

This happens even when the burst is stopped early:

Try a M249 with FIRE_1000 and a boomer or bobcat (it will die from the first few rounds) one tile away and burst set to 10, the turret fires and consumes 1000 charges! Note that each bullet makes a sound which is shown in the message log. So a full 30 round burst should have 30 sound messages.

I suggest you let the firing code handle those flags (and ignore them here) and only use the charges of the gun item directly (without clipping it, let negative values through, only the difference between before and after firing matters).

The same problem exists probably in the other branch using fuel.

EDIT: and I also suggest that you simply pass all the available ammo to the gun (pass parts[p].items[0].charges through).
Also: somebody should change the boolean parameter of player::fire_gun to a long, indicating the number of burst, because currently your are limiting the burst only through the available ammo.

Coolthulhu · Dec 19, 2014

Disabled custom burst limiting for now, back to disabled/1-charge/full burst.
Will still try to limit number of charges manually, because passing all of them wouldn't work with CHARGE tag weapons (charges = 0 at the end of fire_gun).
Also removed the clip to 0 for charges.

Disabled the turret system auto-shutdown, enabled the menu for 1 turret vehicles. Turrets still start in disabled mode.

I'll probably later add charge/burst limiting as an onboard computer part installed on turrets.

Anything I missed?

KA101 · Dec 19, 2014

If you got the liquid fuel working, we'll need @Aenye back as that was the problem with her fire engine. SWAT tanks won't be too far off either.

Coolthulhu · Dec 19, 2014

At the moment all liquid ammo needs to be ammo-typed, have an ammo category and a fuel tank that contains it. No water cannons yet unless you feed them with military grade assault water).

Proper water cannons that use water could be my next PR, because I have a simple solution to this (item substitution).

KA101 · Dec 20, 2014

KA101 reviewed Dec 20, 2014

View changes

KA101 · Dec 20, 2014

src/vehicle.cpp

+        // Regen menu entries
+        for( int i = 0; i < (int)turrets.size(); i++ ) {
+            int p = turrets[i];
+            it_gun *g = dynamic_cast<it_gun*>( item::find_type( part_info( p ).item ) );


This didn't compile when I merged it and BevapDin's PRs together.

Full disclosure: your turret functions merge-conflicted and I may have not resolved them properly, but the functions that I attempted to resolve were both after this line.

KA101 · Dec 20, 2014

KA101 reviewed Dec 20, 2014

View changes

KA101 · Dec 20, 2014

src/vehicle.cpp

    // Check for available power for turrets that use it.
    const int power = fuel_left(fuel_type_battery);
    if( gun->ups_charges > 0 && gun->ups_charges < power ) {
        return false;
    }
-    long charges = burst? gun->burst : 1;
+    long charges = std::min( gun->burst, parts[p].mode );


First conflict was here.

KA101 · Dec 20, 2014

KA101 reviewed Dec 20, 2014

View changes

KA101 · Dec 20, 2014

src/vehicle.cpp

            return false;
        }
        it_ammo *ammo = dynamic_cast<it_ammo*>( item::find_type( amt ) );
        if (!ammo) {
            return false;
        }
-        if (fire_turret_internal (p, *gun, *ammo, charges, whoosh)) {
+        long charges_left = charges;
+        if( fire_turret_internal(p, *gun, *ammo, charges_left, whoosh) ) {


Thinking the other one was here.

Coolthulhu · Dec 20, 2014

I can't do much for now. I don't know git well enough to remove a conflict like this on my side.
We both have different edits on the same line, I don't know how to ensure git deals with it.

Merge one and the other will be trivial to fix.

KA101 · Dec 21, 2014

Folks are gonna love single-shot capability. I like this.

Nitpicks that can be fixed in a later PR:

messing around with the turret settings whilst the vehicle is in motion seems like it jammed the camera off-center. Example: My moving road-roller with dual M249 tailguns was going more or less straight north at 9 MPH, so should be slightly below the center of the screen, but was in the upper-right quadrant of the screen after I'd set for single-shot rather than burst.
Easy enough to fix with : (manual re-center) and the vehicle motion shift kicked right back in, so not a dealbreaker.
Manually enabling the turrets does not then switch them on, so my M249s were sitting there (rather than bursting) as the bio-operators closed in. This may be a good safety-default, but isn't necessarily intuitive. Should post in the forums and possibly add a line in the help about this.

Aenye · Dec 23, 2014

@KA101
Sorry for being gone so long. Will read up on this PR and try to incorporate this into my firetruck.

Coolthulhu · Dec 23, 2014

It's not yet ready for firetrucks, sorry.
I removed some hardcoded limitations, but didn't manage to implement turrets that use non-ammo-typed ammo.

I can fix it quickly (looks like a 5 minute fix, less than wording this post took) by making it substitute a generic "no ammo" ammo-typed item with 0 in all stats rather than bailing out when it encounters a non-ammo typed ammo. This item would then get all stats from the weapon that fires it. It would have some disadvantages (inability to set armor piercing, for example), but should work.

I'll try doing it as soon as my compilation finishes (30 minutes?).

Aenye · Dec 23, 2014

Your idea on bypassing this issue looks good ! In the case of a water cannon, lack of armor piercing shouldn't be much of a problem, as I wanted to make it primarily a heavy-knockback-medium-damage type of weapon.
Thank You for helping with the issue - I wouldn't be able to resolve it, as I'm still learning the code...

Coolthulhu · Dec 23, 2014

Finished:
#10615

Not sure if knockback is possible right now, but BEANBAG tag on the watergun should make hits stun.

Aenye · Dec 23, 2014

Thanks ! Will look through it in a moment. :D

KA101 · Dec 23, 2014

WB Aenye. Hoping your travels went well.

Aenye · Dec 24, 2014

Thanks ! Pretty well, actually, apart from the airlines shipping me off to Atlanta (as planned) and my luggage (including my personal laptop) to Bangkok (not quite as planned). The fun started when my stuff arrived at Atlanta the day after I left for Berlin... Back at home safely, though and collected my laptop this friday - will get cracking at stuff as soon as I get up to speed with the current state of the game (also: cement mixers <3 !).

Coolthulhu added some commits Dec 18, 2014

Per-turret disable/enable/burst

6d3a083

Cancel->Finish

Loading status checks…

b571f01

kevingranade added the unknown label Dec 18, 2014

Coolthulhu added some commits Dec 18, 2014

Major hack removal, allow burst/charge limiting

Loading status checks…

e1305aa

Control turrets -> configure turrets

Loading status checks…

123f32f

Use drain rather than manually inlining it

Loading status checks…

f49539b

Coolthulhu added some commits Dec 19, 2014

Enable turret menu with 1 turret, disable auto shutdown

5b24d34

Disable burst limiting for now

Loading status checks…

9fd02c9

Generic liquid fuel checking instead of hardcoded

Loading status checks…

b30b15d

KA101 self-assigned this Dec 19, 2014

KA101 removed their assignment Dec 20, 2014

KA101 self-assigned this Dec 20, 2014

KA101 merged commit b30b15d into CleverRaven:master Dec 21, 2014
1 check passed

1 check passed

default
Details

kevingranade removed the unknown label Dec 21, 2014

Coolthulhu deleted the cataclysmbnteam:turret-modes branch Dec 28, 2014

CleverRaven/Cataclysm-DDA

Join GitHub today

Per-turret disable/enable/burst mode #10532

Conversation

Coolthulhu commented Dec 18, 2014

Coolthulhu added some commits Dec 18, 2014

kevingranade added the unknown label Dec 18, 2014

This comment has been minimized.

jokermatt999 commented Dec 18, 2014

This comment has been minimized.

Rivet-the-Zombie commented Dec 18, 2014

KA101 reviewed Dec 18, 2014 View changes

This comment has been minimized.

Coolthulhu added some commits Dec 18, 2014

This comment has been minimized.

Coolthulhu commented Dec 19, 2014

KA101 reviewed Dec 19, 2014 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

BevapDin reviewed Dec 19, 2014 View changes

This comment has been minimized.

BevapDin reviewed Dec 19, 2014 View changes

This comment has been minimized.

Coolthulhu added some commits Dec 19, 2014

This comment has been minimized.

Coolthulhu commented Dec 19, 2014

This comment has been minimized.

KA101 commented Dec 19, 2014

KA101 self-assigned this Dec 19, 2014

This comment has been minimized.

Coolthulhu commented Dec 19, 2014

KA101 reviewed Dec 20, 2014 View changes

This comment has been minimized.

KA101 reviewed Dec 20, 2014 View changes

This comment has been minimized.

KA101 reviewed Dec 20, 2014 View changes

This comment has been minimized.

KA101 removed their assignment Dec 20, 2014

This comment has been minimized.

Coolthulhu commented Dec 20, 2014

KA101 self-assigned this Dec 20, 2014

This comment has been minimized.

KA101 commented Dec 21, 2014

Hide details View details KA101 merged commit b30b15d into CleverRaven:master Dec 21, 2014 1 check passed

1 check passed

kevingranade removed the unknown label Dec 21, 2014

This comment has been minimized.

Aenye commented Dec 23, 2014

This comment has been minimized.

Coolthulhu commented Dec 23, 2014

This comment has been minimized.

Aenye commented Dec 23, 2014

This comment has been minimized.

Coolthulhu commented Dec 23, 2014

This comment has been minimized.

Aenye commented Dec 23, 2014

This comment has been minimized.

KA101 commented Dec 23, 2014

This comment has been minimized.

Aenye commented Dec 24, 2014

Coolthulhu deleted the cataclysmbnteam:turret-modes branch Dec 28, 2014

KA101 reviewed Dec 18, 2014

View changes

KA101 reviewed Dec 19, 2014

View changes

BevapDin reviewed Dec 19, 2014

View changes

BevapDin reviewed Dec 19, 2014

View changes

KA101 reviewed Dec 20, 2014

View changes

KA101 reviewed Dec 20, 2014

View changes

KA101 reviewed Dec 20, 2014

View changes

KA101 merged commit `b30b15d` into CleverRaven:master Dec 21, 2014
1 check passed