[The E]

Instead of using my usual approach (also known as "commit first, ask for forgiveness later"), I decided to post this here for some feedback.

Okay, as you may or may not know, while turrets do have an AI setting, it's basically unused. This patch changes that by introducing a new AI_profiles.tbl flag.
Now, I'm not really sure if ai_profiles is really the right place for it. It would make just as much sense as a missions flag, to be honest. What are your opinions on the subject?

Here's the code:

Code: [Select]

Index: code/ai/ai_profiles.cpp
===================================================================
--- code/ai/ai_profiles.cpp (revision 6314)
+++ code/ai/ai_profiles.cpp (working copy)
@@ -395,6 +395,8 @@
 
  set_flag(profile, "$big ships manage shields:", AIPF2_BIG_SHIELD_MANAGE, AIP_FLAG2);
 
+ set_flag(profile, "$turrets use separate ai class:", AIPF2_TURRETS_USE_SEPARATE_AI, AIP_FLAG2);
+
  // if we've been through once already and are at the same place, force a move
  if ( saved_Mp && (saved_Mp == Mp) )
  Mp++;
Index: code/ai/ai_profiles.h
===================================================================
--- code/ai/ai_profiles.h (revision 6314)
+++ code/ai/ai_profiles.h (working copy)
@@ -55,6 +55,7 @@
 #define AIPF2_NO_SPECIAL_PLAYER_AVOID (1 << 1)
 #define AIPF2_PERFORM_LESS_SCREAM_CHECKS (1 << 2)
 #define AIPF2_BIG_SHIELD_MANAGE (1 << 3)
+#define AIPF2_TURRETS_USE_SEPARATE_AI (1 << 4)
 
 #define MAX_AI_PROFILES 5
 
Index: code/ai/aiturret.cpp
===================================================================
--- code/ai/aiturret.cpp (revision 6314)
+++ code/ai/aiturret.cpp (working copy)
@@ -1414,6 +1414,7 @@
  matrix turret_orient;
  int weapon_objnum;
  ai_info *parent_aip;
+ ai_info *turret_aip;
  ship *parent_ship;
  float flak_range = 0.0f;
  weapon_info *wip;
@@ -1438,6 +1439,15 @@
  return false;
 
  parent_aip = &Ai_info[Ships[Objects[parent_objnum].instance].ai_index];
+ turret_aip = parent_aip;
+
+ if (The_mission.ai_profile->flags2 & AIPF2_TURRETS_USE_SEPARATE_AI) {
+ init_aip_from_class_and_profile(turret_aip, &Ai_classes[turret->weapons.ai_class], The_mission.ai_profile);
+ //Assertion((turret_aip != NULL), "No AI class set for turret %s", turret->system_info->name);
+ //if (turret_aip == NULL)
+ // turret_aip = parent_aip;
+ }
+
  parent_ship = &Ships[Objects[parent_objnum].instance];
  wip = get_turret_weapon_wip(&turret->weapons, weapon_num);
  int turret_weapon_class = WEAPON_INFO_INDEX(wip);
@@ -1447,8 +1457,10 @@
  turret->turret_last_fire_direction = *turret_fvec;
 
  // set next fire timestamp for the turret
- if (last_shot_in_salvo)
+ if (last_shot_in_salvo && !(The_mission.ai_profile->flags2 & AIPF2_TURRETS_USE_SEPARATE_AI))
  turret_set_next_fire_timestamp(weapon_num, wip, turret, parent_aip);
+ else
+ turret_set_next_fire_timestamp(weapon_num, wip, turret, turret_aip);
 
  // if this weapon is a beam weapon, handle it specially
  if (wip->wi_flags & WIF_BEAM) {
NOTE: No matter what the outcome of this is, this won't be in 3.6.12

[General Battuta]

It would be useful as both. A global force would be good for mods using this from the ground up, but a per-mission flag would allow us to use it only where needed and prevent the utter destruction of backwards compatibility if a mod added it without wanting to go through all its existing missions with a fine-toothed comb.

[The E]

Come to think of it, as an ai_profiles flag, it is already somewhat of a per-mission flag (since you have to specifically set the AI profile in the mission specs dialogue).

[FUBAR-BDHR]

Should not $fix AI class bug: YES already cover this?

[The E]

Whether it should or not, fact is that it doesn't.

In addition, I am a bit weary of rolling this flag into that one, as it might inadvertently alter mission behaviour.

[chief1983]

I like it, but it looks overly complicated near the end.  Based on the previous code, it looks like the last bit should just be:

Code: [Select]

// set next fire timestamp for the turret
if (last_shot_in_salvo)
- turret_set_next_fire_timestamp(weapon_num, wip, turret, parent_aip);
+ turret_set_next_fire_timestamp(weapon_num, wip, turret, turret_aip);

Since you've already set turret_aip based on the profile flag, default initializing it to parent_aip.

[The E]

True, that would work perfectly fine.

I didn't do much optimizing, apparently........ 

[FUBAR-BDHR]

Ideally an optional ai_class entry in the subsystem would be the way to go defaulting back to the parent ship if none provided.  That way you could have certain turrets set to higher accuracy by default.  Shadow main beams for instance as they never miss could be set to perfect while the ship still defaults to standard Shadow cap AI.. 

[chief1983]

It is defaulting to the ship's AI class via the initialization of turret_aip.  I'm guessing that init_aip_from_class_and_profile doesn't set it to NULL, if it was already initialized to the parent_aip?  If not that whole section might need to be reordered actually, to ensure turret_aip is always non-NULL.  And parent_aip shouldn't be NULL, and if it is there's nothing we can do about it at that point I don't think.

FUBAR, it sounds like what you're asking is what already happens, unless I'm really confused by what you're asking.

The_E, if my previous assumption about init_aip_from_class_and_profile was correct, I'd uncomment that assertion and delete the two commented out lines below it.

[FUBAR-BDHR]

Yep your confused

What I'm asking for is an additional optional field in the subsystem section that is used so you could have the ship default to captain but some turrets default to other ai classes.  So it would be like:

$subsystem turret03a,5,0
$subsystem_ai_class:  general

where $subsystem_ai_class is an optional field.  if not supplied then the ships ai_class is used.

[chief1983]

Ah, so, instead of having to define every turret's AI every mission, it would have a tabled default different from the ship's AI level.  Roger.  Not sure that's a great idea though, we don't even set a default ai level for ships in the ships.tbl.  But I suppose there's not a much better place to put it.  It does seem a bit like a lot of potentially extra code to just save setting turrets on a few capital ships, but I guess since ships can have 80 or more turrets that would save some time, if the turrets will always have the same setup.

[Sushi]

Should not $fix AI class bug: YES already cover this?

No, it shouldn't. The problems are different enough that they deserve to be handled separately.

Ah, so, instead of having to define every turret's AI every mission, it would have a tabled default different from the ship's AI level.  Roger.  Not sure that's a great idea though,

I am also dubious about the cost/benefit ratio of having subsystem-specific default AIs...

we don't even set a default ai level for ships in the ships.tbl. 

Actually, we do.

But I suppose there's not a much better place to put it.  It does seem a bit like a lot of potentially extra code to just save setting turrets on a few capital ships, but I guess since ships can have 80 or more turrets that would save some time, if the turrets will always have the same setup.

Either way, modders we be able to get their desired turret behavior. Adding defaults to ships.tbl makes it rougher on the coders, but I suppose it does make life (potentially) a bit easier on the modders.


Have you tested the patch to make sure it works? I could have sworn there was code elsewhere that looks at a turret's parent AI to decide stuff, and if so that would need to be adjusted too.

[The E]

It is defaulting to the ship's AI class via the initialization of turret_aip.  I'm guessing that init_aip_from_class_and_profile doesn't set it to NULL, if it was already initialized to the parent_aip?  If not that whole section might need to be reordered actually, to ensure turret_aip is always non-NULL.  And parent_aip shouldn't be NULL, and if it is there's nothing we can do about it at that point I don't think.

init_aip_from_class_and_profile() actually rewrites the whole entry based on the class and profile supplied, so there really shouldn't be a situation where it is not initialized correctly.

The_E, if my previous assumption about init_aip_from_class_and_profile was correct, I'd uncomment that assertion and delete the two commented out lines below it.

Code: [Select]

Index: ai/ai_profiles.cpp
===================================================================
--- ai/ai_profiles.cpp (revision 6316)
+++ ai/ai_profiles.cpp (working copy)
@@ -395,6 +395,8 @@
 
  set_flag(profile, "$big ships manage shields:", AIPF2_BIG_SHIELD_MANAGE, AIP_FLAG2);
 
+ set_flag(profile, "$turrets use separate ai class:", AIPF2_TURRETS_USE_SEPARATE_AI, AIP_FLAG2);
+
  // if we've been through once already and are at the same place, force a move
  if ( saved_Mp && (saved_Mp == Mp) )
  Mp++;
Index: ai/ai_profiles.h
===================================================================
--- ai/ai_profiles.h (revision 6316)
+++ ai/ai_profiles.h (working copy)
@@ -55,6 +55,7 @@
 #define AIPF2_NO_SPECIAL_PLAYER_AVOID (1 << 1)
 #define AIPF2_PERFORM_LESS_SCREAM_CHECKS (1 << 2)
 #define AIPF2_BIG_SHIELD_MANAGE (1 << 3)
+#define AIPF2_TURRETS_USE_SEPARATE_AI (1 << 4)
 
 #define MAX_AI_PROFILES 5
 
Index: ai/aiturret.cpp
===================================================================
--- ai/aiturret.cpp (revision 6316)
+++ ai/aiturret.cpp (working copy)
@@ -1413,7 +1413,7 @@
 {
  matrix turret_orient;
  int weapon_objnum;
- ai_info *parent_aip;
+ ai_info *turret_aip;
  ship *parent_ship;
  float flak_range = 0.0f;
  weapon_info *wip;
@@ -1437,7 +1437,13 @@
  if((!timestamp_elapsed(get_turret_weapon_next_fire_stamp(&turret->weapons, weapon_num))) && last_shot_in_salvo)
  return false;
 
- parent_aip = &Ai_info[Ships[Objects[parent_objnum].instance].ai_index];
+ turret_aip = &Ai_info[Ships[Objects[parent_objnum].instance].ai_index];
+
+ if (The_mission.ai_profile->flags2 & AIPF2_TURRETS_USE_SEPARATE_AI)
+ init_aip_from_class_and_profile(turret_aip, &Ai_classes[turret->weapons.ai_class], The_mission.ai_profile);
+
+ Assertion((turret_aip != NULL), "No AI class set for turret %s", turret->system_info->subobj_name);
+
  parent_ship = &Ships[Objects[parent_objnum].instance];
  wip = get_turret_weapon_wip(&turret->weapons, weapon_num);
  int turret_weapon_class = WEAPON_INFO_INDEX(wip);
@@ -1448,7 +1454,7 @@
 
  // set next fire timestamp for the turret
  if (last_shot_in_salvo)
- turret_set_next_fire_timestamp(weapon_num, wip, turret, parent_aip);
+ turret_set_next_fire_timestamp(weapon_num, wip, turret, turret_aip);
 
  // if this weapon is a beam weapon, handle it specially
  if (wip->wi_flags & WIF_BEAM) {
@@ -1551,7 +1557,7 @@
  }
  }
  //Not useful -WMC
- else if (!(parent_aip->ai_profile_flags & AIPF_DONT_INSERT_RANDOM_TURRET_FIRE_DELAY) && last_shot_in_salvo)
+ else if (!(turret_aip->ai_profile_flags & AIPF_DONT_INSERT_RANDOM_TURRET_FIRE_DELAY) && last_shot_in_salvo)
  {
  float wait = 1000.0f * frand_range(0.9f, 1.1f);
  turret->turret_next_fire_stamp = timestamp((int) wait);

Like that?

Ah, so, instead of having to define every turret's AI every mission, it would have a tabled default different from the ship's AI level.  Roger.  Not sure that's a great idea though, we don't even set a default ai level for ships in the ships.tbl.  But I suppose there's not a much better place to put it.  It does seem a bit like a lot of potentially extra code to just save setting turrets on a few capital ships, but I guess since ships can have 80 or more turrets that would save some time, if the turrets will always have the same setup.

We so do set default AI classes for ships. That was sort of the problem with the is-ai-class sexp (See also: Commit 6135).

I am also dubious about the cost/benefit ratio of having subsystem-specific default AIs...

FUBAR's point is right on the money. You might want to set default values like that to simulate something like "supercharged" weapons, without having to do a whole new weapon entry for Weapons.tbl.

Have you tested the patch to make sure it works? I could have sworn there was code elsewhere that looks at a turret's parent AI to decide stuff, and if so that would need to be adjusted too.

Couldn't find any on my search (But then, I was coding this on a train ride, so I might not have looked hard enough).
However, I did test it, and it does, in fact, work.

[General Battuta]

I just want to emphasize that none of this should work unless a specific AI profile is selected. It's very important that default turret AI behavior not change, and that means relying on the ship's pilot AI UNLESS an AI profile that enables this option is turned on.

I know The_E is aware of this but I just want to make sure it's vocalized.

[chief1983]

Ok, my bad, we do set default ai

Just did not remember ever setting that in ships.tbl.

[chief1983]

turret_aip gets initialized by default the same way it always has, it only does the extra init step if the ai_profile flag is enabled, so you shouldn't have to worry about that Battuta.

[AugustusVarius]

turret_aip gets initialized by default the same way it always has, it only does the extra init step if the ai_profile flag is enabled, so you shouldn't have to worry about that Battuta.

I think that's what he was saying.  He tends to be a bit of a stickler when it comes to changing default behavior.

Also, haven't default AI settings been in ships.tbl since retail?

[Fury]

Oh boy....

If this flag gets in and is enabled, every turret in every mission would have to be checked what AI class it has not to alter mission balance. I hope you're prepared for that. From the looks of it, you'd have to set AI class for each and every turret manually as opposed to just the ship pilot. Would be a lot of work.

[FUBAR-BDHR]

No you wouldn't.  If it you don't specify it would default to the ships just as it always did even if the flag is on.  The only way it could break anything is if someone turns it on it their mod and doesn't bother to check the missions in the mod to see if they set ai for turrets. 

On the other hand I'm actually looking forward to adding this to multiplayer as most of the missions were written with the assumption that setting turret ai actually did something.  So we will get to see those missions as they were intended.

[Sushi]

No you wouldn't.  If it you don't specify it would default to the ships just as it always did even if the flag is on.  The only way it could break anything is if someone turns it on it their mod and doesn't bother to check the missions in the mod to see if they set ai for turrets. 

On the other hand I'm actually looking forward to adding this to multiplayer as most of the missions were written with the assumption that setting turret ai actually did something.  So we will get to see those missions as they were intended.

...but not as they were tested or ultimately balanced. That's hardly a problem, though, because this is set up purely as an "opt-in" feature.