FAForever / fa

Lua code for FAF
221 stars 228 forks source link

Pull threads out of functions #6143

Closed Hdt80bro closed 1 month ago

Hdt80bro commented 2 months ago

Several functions define anonymous thread functions like ForkThread(function() ... end) which are low-hanging fruit to adding customization points by naming them. These threads are something that, were a mod were to want to change, would require replacing the entire function in addition to the thread function. This is unnecessary on our part, and increases such a mod's stiffness - which I have seen cause at least some parts of one mod to cease to function. While this won't help with anything already written, it is possible to prevent future bit rot.

Garanas commented 1 month ago

After running a few AI games it appears to be fine 👍

clyfordv commented 1 month ago

Think we got a problem. I can't identify the reason but the changes to DefaultProjectileWeapon.lua throw this error when cheat spawning a unit that A. requires energy to fire and B. does not have EnergyChargeForFirstShot set to false (so Salvation, but not Mavor):

image

Edit: Looks like it's because, when a unit is cheat spawned, the economy event only exists for a tick, and isn't around the next tick when the forked thread gets around to waiting for it.

Garanas commented 1 month ago

Ah yes, it passes the wrong reference:

    -- Create an economy event for those weapons which require Energy to fire
    ---@param self DefaultProjectileWeapon
    StartEconomyDrain = function(self)
        if self.FirstShot then return end
        if self.unit:GetFractionComplete() ~= 1 then return end

        if not self.EconDrain and self.EnergyRequired and self.EnergyDrainPerSecond then
            local nrgReq = self:GetWeaponEnergyRequired()
            local nrgDrain = self:GetWeaponEnergyDrain()
            if nrgReq > 0 and nrgDrain > 0 then
                local time = nrgReq / nrgDrain
                if time < 0.1 then
                    time = 0.1
                end
                self.EconDrain = CreateEconomyEvent(self.unit, nrgReq, 0, time)
                self.FirstShot = true
                self.unit:ForkThread(self.EconomyDrainThread) -- <-- passes the wrong reference
            end
        end
    end,

    ---@param self DefaultProjectileWeapon
    EconomyDrainThread = function(self)  -- <-- should be the weapon, not the unit
        WaitFor(self.EconDrain)
        RemoveEconomyEvent(self.unit, self.EconDrain)
        self.EconDrain = nil
    end,