FAForever / fa

Lua code for FAF
221 stars 228 forks source link

Fix free intel not being toggleable #6207

Closed lL1l1 closed 1 month ago

lL1l1 commented 1 month ago

Description of the proposed changes

Adds 1 energy cost for spy plane jamming to make the jamming retoggling feature work with spy planes.

Testing done on the proposed changes

Spawned a radar for myself and a UEF spy plane on the enemy team. Toggled the full map vision cheat on then off to clear the spy plane's jammer blips. After some time, the blips return.

Additional context

I looked into the defaultcomponents.lua DisableUnitIntel function and how the intel status is created in blueprint-units.lua and it seems like there is no way to create a free but toggleable intel since the table AllIntel isn't filled if all intel is free, but DisableUnitIntel(disabler, intelType) requires the intelType to be in AllIntel.

Checklist

Garanas commented 1 month ago

How much does the Cybran Spy Plane consume for its maintenance? We could use that value instead of 1, as 1 looks odd

lL1l1 commented 1 month ago

The Cybran spy plane doesn't consume anything since stealth never needs to be toggled, but UEF frigates use 5 energy.

Garanas commented 1 month ago

Let's go with 5 energy here too then.

lL1l1 commented 1 month ago

What about allowing setting custom status from the bp? Currently it is completely overwritten by lua blueprint modification, but this would allow placing Jamming in AllIntel and omni/radar into AllIntelMaintenanceFree manually.

Garanas commented 1 month ago

I'm not in favor of adjusting the blueprint process related to intel. The process is complicated and difficult to verify given that it also has to deal with numerous units from mods. If we can get away with just giving a unit an energy maintenance of 5 then that would be my preferred solution here.

lL1l1 commented 1 month ago

Yeah that's a better fix. Currently it is impossible to disable free intel entirely, which you might want to do for intel types other than jamming if you're modding some custom intel resource.

lL1l1 commented 1 month ago

I upvalued and optimized the intel logic quite a bit, it should work the same way as before. Tests:

lL1l1 commented 1 month ago

By the way, @clyfordv you can link files and their line numbers as a preview inside a comment by going to the file page, clicking+shift clicking the line numbers to select a line/range of lines, and then getting a permalink from the options menu. I saw you having trouble with that in another comment too so I thought I'd mention it. image The preview is this: https://github.com/FAForever/fa/blob/87e6fb0ed883e404a407d727bc3baddf95341018/lua/defaultcomponents.lua#L198-L212

clyfordv commented 1 month ago

I ran all your tests, everything is good except for the cybran SACU:

  1. Spawn an un-upgraded SACU
  2. Upgrade to personal stealth
  3. It still turns off when e stalling, but toggling it manually has no effect (energy consumption and stealth effect stays).
  4. There are other weirder effects when you upgrade to another enhancement (like cloak), then downgrade--instead of never turning off, it just never turns on.

So looks like that enhancement section probably did something after all.

lL1l1 commented 1 month ago

All fixed. The enhancements section had nothing to do with that, the CCommandUnit script was just overriding the script bit behavior entirely instead of hooking, which broke the stealth toggle.

Garanas commented 1 month ago

I'll be honest - these are scary changes because it's a complicated system. If you both trust the changes then feel free to approve and merge them. But please keep checking intel continuously once this is merged in.

clyfordv commented 1 month ago
  1. I asked Balthazar about the enhancementIntel and he said he had never seen it in his life.
  2. There's a lot of green in the changed files, but I think most of it is formatting. Perhaps the formatting/performance improvements would be best separated from the bugfix.
  3. I think we're good to go.
lL1l1 commented 1 month ago

@clyfordv If it's ready to go can you approve it so it can be merged?