FAForever / fa

Lua code for FAF
221 stars 228 forks source link

Tree mass reclaim #6096

Closed BlackYps closed 2 months ago

BlackYps commented 2 months ago

Farms said that only dead trees having no mass is not enough to properly test it, as team game maps with relevant forests often feature green trees, so I extended the change to all trees for testing purposes.

Btw, I don't really understand what "Changes are annotated" in the checklist is supposed to be...

Checklist

lL1l1 commented 2 months ago

I don't think it's useful to experiment with just 0 mass trees because it throws the mass/energy balance way off compared to normal games.

Garanas commented 2 months ago

Btw, I don't really understand what "Changes are annotated" in the checklist is supposed to be...

It means that when you write functions such as:

function IsAlly(army1, army2)
  -- (...)
end

With annotations like this:

---@param army1 Army
---@param army2 Army
---@return boolean
function IsAlly(army1, army2)
end

Which applies to prop blueprints too, but that work is mostly completed. The annotations allow the intellisense-like features of the FA plugin to work.

Garanas commented 2 months ago

I don't think it's useful to experiment with just 0 mass trees because it throws the mass/energy balance way off compared to normal games.

I agree - removing it from individual trees is fine but this also removes it from tree groups which is not the intention in the end?

Garanas commented 2 months ago

These changes are equivalent to playing with the following mod:

image

Which does the following:

do
    local oldModBlueprints = ModBlueprints

    function ModBlueprints(all_bps)
        oldModBlueprints(all_bps)

        for k, prop in all_bps.Prop do
            if prop.ScriptModule == '/lua/proptree.lua' then
                prop.Economy.ReclaimMassMax = 0
            end
        end
    end
end

Farms can host a game or two with the mod to test out the feature.

BlackYps commented 2 months ago

removing it from individual trees is fine but this also removes it from tree groups which is not the intention in the end?

Yes, that is not the intention in the end. It's to temporarily test out the scenario of basically all tree groups have been broken. Farms described some potential gameplay problems that can arise if trees give only energy and you rely on the energy reclaim but you sometimes overflow as well. So this is intended to be an easy way to gather some insight how the game plays out in these scenarios. I opted to do this, as it is significantly less effort than doing the armature changes in all the tree groups. Especially because the blender importer is a bit bugged at the moment.

BlackYps commented 2 months ago

I pinged you in discord, but actually keep this closed. It was based on a misunderstanding and is not needed