Enable counterintel by default for AI units

[lL1l1]

Description of the proposed changes

Implements a discord suggestion.

• Replaces toggles like:

  --Turns Jamming off when unit is built
  self:SetScriptBit('RULEUTC_JammingToggle', true)

  with

  -- Don't turn off jamming for AI so that it uses it by default
  if self.Brain.BrainType == 'Human' then
      self:SetScriptBit('RULEUTC_JammingToggle', true)
  else
      self:SetMaintenanceConsumptionActive()
  end

• Formatting
• Annotations

Testing done on the proposed changes

All units work as expected when spawned for the player and the default civilian AI.

Spawn command:

CreateUnitAtMouse('ura0401', 0,    1.93,  115.47,  0.00000)
CreateUnitAtMouse('ura0304', 0,   14.43,  117.47,  0.00000)
CreateUnitAtMouse('uea0304', 1,  -11.18,  -89.86,  0.00000)
CreateUnitAtMouse('uea0304', 1,  -22.74,  -49.33,  0.00000)
CreateUnitAtMouse('uea0304', 1,  -14.93,  -80.66,  0.00000)
CreateUnitAtMouse('xab1401', 0,    1.29,   99.40,  0.00000)
CreateUnitAtMouse('dra0202', 1,   -6.43,  -78.16,  0.00000)
CreateUnitAtMouse('ura0304', 1,   15.07,  -80.66,  0.00000)
CreateUnitAtMouse('xra0305', 1,   28.57,  -81.16,  0.00000)
CreateUnitAtMouse('ura0401', 1,    2.65,  -83.16,  0.16435)
CreateUnitAtMouse('xra0305', 0,   27.93,  116.97,  0.00000)
CreateUnitAtMouse('url0101', 0,  -25.07,  115.97,  0.00000)
CreateUnitAtMouse('ura0303', 1,   23.07,  -81.66,  0.00000)
CreateUnitAtMouse('xsb3201', 1,    3.12,  -67.09, -0.00000)
CreateUnitAtMouse('uea0304', 1,   -7.37,  -78.44,  0.00000)
CreateUnitAtMouse('xab1401', 1,    2.29,  -98.60, -0.00000)
CreateUnitAtMouse('url0101', 1,  -24.43,  -82.16,  0.00000)
CreateUnitAtMouse('uea0304', 1,   -9.34,  -60.59,  0.00000)
CreateUnitAtMouse('ura0303', 0,   22.43,  116.47,  0.00000)
CreateUnitAtMouse('dra0202', 0,   -7.07,  119.97,  0.00000)
CreateUnitAtMouse('uea0304', 0,  -15.57,  117.47,  0.00000)
CreateUnitAtMouse('xsb3201', 0,    1.29,   92.40,  0.00000)

Checklist

• Changes are annotated, including comments where useful
• Changes are documented in the changelog for the next game version

[relent0r]

@lL1l1
Sorry if this is a very late comment since I'm asking for a possible change.

Rather than doing this at the unit level. Since its for AI only could we perhaps tackle at the aiBrain layer? Jip introduced callback triggers against the aiBrains for OnUnitStopBeingBuilt which I believe could tackle this.

A reason why this is a good approach is that we can make it only impact the AI's that we are interested in such as the default and campaign. 3rd party AI's handle their own counter intel units(which isn't really relevant since this just changes the on built state). There is a set of brain files that live under /lua/aibrains with one for each AI type. You can see the default one in the aibrain.lua file under /lua/aibrain.lua, make the change and paste into the other brains to maintain separation.

Just a thought.

[lL1l1]

So you want me to put the callback into campaign-ai.lua and base-ai.lua? I'm guessing this would work for enabling counterintel in all the coop missions, would it work for spawned units in skirmishes like survival maps?

[relent0r]

So you want me to put the callback into campaign-ai.lua and base-ai.lua? I'm guessing this would work for enabling counterintel in all the coop missions, would it work for spawned units in skirmishes like survival maps?

So you want me to put the callback into campaign-ai.lua and base-ai.lua? I'm guessing this would work for enabling counterintel in all the coop missions, would it work for spawned units in skirmishes like survival maps?

I was just having a look as I've forgotten. I couldn't find a reference to it using the base-ai.lua file

In the OnCreateArmyBrain function I could see this.

    if (not info.Human) then
        local reference
        local keys = import("/lua/aibrains/index.lua").keyToBrain
        if (not info.Civilian) and (info.AIPersonality != '') then
            -- likely a skirmish scenario
            reference = keys[info.AIPersonality]
        else
            -- likely a campaign scenario
            if ScenarioInfo.type != 'skirmish' then
                reference = keys['campaign']
            end
        end

        if reference then
            local instance
            if not table.empty(getmetatable(reference)) then
                instance = reference
            else
                instance = import(reference[1])[reference[2]]
            end

            if instance then
                setmetatable(brain, instance)
            end
        end
    end

I know the table here

`keyToBrain = {
-- default
default = { "/lua/aibrains/base-ai.lua", "AIBrain" },
campaign = { "/lua/aibrains/campaign-ai.lua", "AIBrain" },

-- base AI
tech = { "/lua/aibrains/tech-ai.lua", "AIBrain" },
turtle = { "/lua/aibrains/turtle-ai.lua", "AIBrain" },
rush = { "/lua/aibrains/rush-ai.lua", "AIBrain" },
easy = { "/lua/aibrains/medium-ai.lua", "AIBrain" },
medium = { "/lua/aibrains/medium-ai.lua", "AIBrain" },
adaptive = { "/lua/aibrains/adaptive-ai.lua", "AIBrain" },
random = { "/lua/aibrains/adaptive-ai.lua", "AIBrain" },

-- base AIX
techcheat = { "/lua/aibrains/tech-ai.lua", "AIBrain" },
turtlecheat = { "/lua/aibrains/turtle-ai.lua", "AIBrain" },
rushcheat = { "/lua/aibrains/rush-ai.lua", "AIBrain" },
adaptivecheat = { "/lua/aibrains/adaptive-ai.lua", "AIBrain" },
randomcheat = { "/lua/aibrains/adaptive-ai.lua", "AIBrain" },

}`

Provides all the key references. So I think the base-ai.lua is a base template rather than something that is used.

So in theory if we want all the default AI including campaign to use it we update all the brain files for the various AI.

In answer to your other question. Sadly no it won't apply to spawned units since they don't perform that callback. The callback is just for when a unit is built (I couldn't remember if it applies to just factories or if both factories and engineers).

If you want me to validate first I can run the changes locally here before you put in the effort.

disclaimer : The default skirmish AI can also handle enabling the counter intel stuff itself when it forms its platoons, it just hasn't been done yet.

[lL1l1]

@relent0r I moved the counterintel enabling to the campaign AI and the default AIs. Can you review it please?

[relent0r]

@relent0r I moved the counterintel enabling to the campaign AI and the default AIs. Can you review it please?

Looks good.

@Dhomie fyi

[Dhomie]

Yep, looks good indeed, although I do have a question.

When it comes to spawning units, doesn't Unit:OnCreate() get called?
I assume it's also called when the unit entity is created when it has just started building, but would it be possible to handle the intel/counter intel for spawned units in Unit:OnCreate() ?

[relent0r]

Yep, looks good indeed, although I do have a question.

When it comes to spawning units, doesn't Unit:OnCreate() get called? I assume it's also called when the unit entity is created when it has just started building, but would it be possible to handle the intel/counter intel for spawned units in Unit:OnCreate() ?

Yea OnCreate does get called. From an AI developers pov OnCreate would be better (its what m28/m27 use as the hook for assigning units to logic). I always assumed there was a valid reason why it was never used as a callback for ai. e.g why have a heavy platoon manager when you could just use oncreate callbacks.

[Dhomie]

I second the idea of having the whole thing handled by Unit:OnCreate(), as it would simply deal with both spawning, and building cases. If there's no AIBrain:OnUnitCreate(unit) callback yet, it can be introduced for this use case, and the idea of keeping this QoL update to specific brains is still applicable.

As for why GPG did things this way, I think they preferred the idea of manually enabling these whenever the mission, and/or AI developers have seen it fit, or simply didn't have the time nor resources to seperate AI and player cases.
These intel features aren't enabled for players either by default, and I'd leave the player matter up to the balance, and senior staff.
However for AIs really, it's just a chore having to always manually enable the intel features on units.

[relent0r]

I second the idea of having the whole thing handled by Unit:OnCreate(), as it would simply deal with both spawning, and building cases. If there's no AIBrain:OnUnitCreate(unit) callback yet, it can be introduced for this use case, and the idea of keeping this QoL update to specific brains is still applicable.

As for why GPG did things this way, I think they preferred the idea of manually enabling these whenever the mission, and/or AI developers have seen it fit, or simply didn't have the time nor resources to seperate AI and player cases. These intel features aren't enabled for players either by default, and I'd leave the player matter up to the balance, and senior staff. However for AIs really, it's just a chore having to always manually enable the intel features on units.

I think for the sake of getting this implemented we should run with what we have as hooking OnCreate is something that requires a bigger discussion given the potential for impact where as this change is a known quantity.

I know Jip spent a bunch of time attaching callbacks to the aiBrain class so that we could leverage added functionality but I never thought to ask why he didn't add OnCreate as part of that work. Perhaps we could create an issue and get a wider discussion on it?

[Dhomie]

I think for the sake of getting this implemented we should run with what we have as hooking OnCreate is something that requires a bigger discussion given the potential for impact where as this change is a known quantity.

I know Jip spent a bunch of time attaching callbacks to the aiBrain class so that we could leverage added functionality but I never thought to ask why he didn't add OnCreate as part of that work. Perhaps we could create an issue and get a wider discussion on it?

Agreed, this might be an issue that could have further implications.

[relent0r]

@lL1l1

I just attempted to test this but for some reason its not setting stealth.

In this screenshot there is a cybran strat bomber and ASF's that did not enable stealth.

I logged the callback and its firing, its detecting that the unit supports stealth and attempts to set the script bit but the unit itself never goes into stealth. I double checked the unit id and its passing the correct unit.

Thoughts?

Validation testing against branch failed.

[lL1l1]

@relent0r

I logged the callback and its firing, its detecting that the unit supports stealth and attempts to set the script bit but the unit itself never goes into stealth. I double checked the unit id and its passing the correct unit.

Thoughts?

In the unit scripts it runs the base function first and then the rest, so it's enabling the intel but re-enabling it in the unit script. Seems like the unit scripts need to be edited after all.

[relent0r]

@relent0r

I logged the callback and its firing, its detecting that the unit supports stealth and attempts to set the script bit but the unit itself never goes into stealth. I double checked the unit id and its passing the correct unit.
Thoughts?

In the unit scripts it runs the base function first and then the rest, so it's enabling the intel but re-enabling it in the unit script. Seems like the unit scripts need to be edited after all.

ooooh ok I see what you mean. I didn't spot that the first time around. In that case I apologize from making you do a bunch of extra work. I guess if you revert to your previous commit I'll retest on that.

[lL1l1]

ooooh ok I see what you mean. I didn't spot that the first time around. In that case I apologize from making you do a bunch of extra work. I guess if you revert to your previous commit I'll retest on that.

I think a combination of the two is the best: the unit script only disables it for human units and it does nothing for the AI, leaving the implementation up to the AI brain callback. Keeping it in the script for humans makes it easy for balance team to really fine tune the gameplay experience per-unit, and having it in the callback for AI keeps all behavior in one place for AI devs.

[relent0r]

Oh yea valid point. Sounds like a good middleground.

[lL1l1]

I looked into it but making it work that way would be changing the base intel component, which is a bit out of scope of this PR. In short, the issue is that I want to put the human/default behavior before OnStopBeingBuilt so that the AI callback behavior can override it, but the intel component forcefully enables all intel in OnStopBeingBuilt, so the AI callback would work but the human intel disable wouldn't work.

So I'll just revert to the unit script changes I had before, and without the AI callbacks because they don't do anything since intel is enabled by default.

[relent0r]

ok have retested it and its working as expected.