bugfix(energy): Destroying disabled Power Plant lowers energy production twice

[Caball009]

• Fixes issue: Destroying Disabled Power Plants Upgraded with PowerPlantUpgrade removes excess Energy #1839

This change fixes a bug where destroying disabled power plants would trigger a second reduction in energy production. This affects reactors from USA and China if they're upgraded / overcharged.

[Stubbjax]

I'm unsure about leaving the assertion only in the #else block. I think it is still correct to indicate erroneous logic / behaviour even if it has been rectified via a different path.

[Caball009]

I'm unsure about leaving the assertion only in the #else block. I think it is still correct to indicate erroneous logic / behaviour even if it has been rectified via a different path.

I don't think I agree, but I also don't really care.

[xezon]

I think this fix is not robust. It will break again when external power conditions are changed.

Either unify the exclusion condition into a static function and call this everywhere applicable, or give the modules a state that tells whether they gave energy to the player.

[Caball009]

or give the modules a state that tells whether they gave energy to the player.

I'm not sure what you mean by this.

Considering how closely related the two issues are, is your intention to also overhaul the solution used in the following PR? #1302

[xezon]

Maybe refactors are better suited in a follow up change to not make this fix too complicated.

The reason these power bugs occur is that the conditions for adding and removing energy are scattered in code. Instead, there should be consistent conditions that the relevent energy consumptions go through. This way, when the conditions are changed, all code paths will recognize the new conditions.

or give the modules a state that tells whether they gave energy to the player.

This means make power modules stateful, whether they added energy to player. But perhaps that is too complicated because it would require Xfer.

[xezon]

Perhaps the getObject()->isDisabled() conditions simply need moving into both addPowerBonus and removePowerBonus. But yes can be follow up then.

[Caball009]

Nice, that keeps this PR simple for me :)