[Bug]: Engineers ringing a mex that already has one or more mass storages will queue a T3 upgrade before building the other mass storages.

IndexLibrorumProhibitorum commented 1 month ago

Describe the Bug

Using the relevant game setting, it is possible to take a group of engineers and by assisting a mex, queue up the upgrade to T2. When that mex is assisted again, the engineers will queue 4 mass storages around around the mex. Once those mass storages are built, another assist will queue the T3 upgrade for that mex.

However, if a mex already has a mass storage, which may happen because the building of queued storages was aborted, the engineers will queue and start a T3 upgrade before they've finished ringing the storages.

This causes T3 mexes to be inadvertendly started way before they should. It is not uncommon to see a group of engineers going around trying to ring all mexes, but then accidentally upgrading one mex to T3 while several mexes still awaiting mass storage rings.

The logic should check if the mex is fully ringed before being allowed to queue a T3 upgrade.

Reproduce the bug

Build one mass extractor to T2 and add one mass storage. Assist the mex with engineers, and observe that they'll start the T3 upgrade rather than build the rings.

Screenshots

No response

Additional context

No response

Garanas commented 1 month ago

This is not a bug, this is intended. When one or more storages are connected to an extractor then it will queue the upgrade.

In the UI script we can not compute what structures are adjacent to what structures. The criteria we use now is whether the mass extractor has more income then what is defined in its blueprint. If that is the case, then we assume it is already capped and we'll issue the upgrade too.

The reason for this is simple: not all extractors can be fully ringed with storages. The terrain could be blocking it. Or another structure is adjacent that does not provide a resource bonus to the extractor. But the only information we have in the UI is the mass income that is mentioned before.

lL1l1 commented 4 weeks ago

You could sync what adjacencies a mex has, and also mark an adjacent slot as occupied if storage can't be built there when the max is placed. This would solve everything except two cases:

a building not giving adjacency but also blocking the storage because its placed 1 unit off
mexes that are larger than 2x2 units

You could check for blocked spaces with: https://github.com/FAForever/fa/blob/dea4a06761332438fb4e397161114345c520a20e/engine/Sim/CAiBrain.lua#L67-L71 https://github.com/FAForever/fa/blob/fd3205075499e36833f185ebb114b2419cf15957/lua/utilities.lua#L59

Syncing adjacency could also be useful for that UI idea of displaying how much of a discount is being given/bonus is received.

Garanas commented 3 weeks ago

Syncing adjacency could also be useful for that UI idea of displaying how much of a discount is being given/bonus is received.

We can already do this I think by comparing the blueprint value(s) with the actual produced/consumed values.

I'm personally not in favor of the increased complexity of syncing values between Sim/UI. There is the AdjacentoTo and NotAdjacentTo events if I am not mistaken, the checks and syncs would need to happen there. I'd also use GetStat and SetStat over syncing values via the Sync. We could do this for all structures.

FAForever / fa