ShadowTheAge / yafc

Powerful Factorio calculator/analyser that works with mods
GNU General Public License v3.0
176 stars 56 forks source link

[BUG] YAFC thinks Wind Turbines require power #205

Open KiwiHawk opened 1 year ago

KiwiHawk commented 1 year ago

Reproduction

YAFC v0.5.8 Mod List: Issue can be seen in YAFC with just KS Power mod. Enable Sea Block Pack mod (and prerequisites) to see Analysis Warning.

https://mods.factorio.com/mod/KS_Power https://mods.factorio.com/mod/SeaBlockMetaPack

Issue Description

When using the full Sea Block mod pack, YAFC gives an Analysis Warning.

image

YAFC doesn't think that Electricity generating (Mechanics) is unlocked.

image

Wind Turbine (entity) should not depend on Electricity (power). It generates power! YAFC knows that it generates power: image

Wind Turbine Details

Type: electric-energy-interface Electric Energy Source Prototype: image image

image

KiwiHawk commented 1 year ago

I would have expected Wind Turbines to depend on Void (Power), like Solar Panels do. image

image

https://github.com/ShadowTheAge/yafc/blob/fd80865b3429e99d592426f2107a2dbe9a0eb553/YAFCparser/Data/FactorioDataDeserializer_Entity.cs#L356

Adding "electric-energy-interface" to the following line does fix the issue. I'm not sure that this is correct though. Electric Energy Interface can be set to have energy consumption. https://github.com/ShadowTheAge/yafc/blob/fd80865b3429e99d592426f2107a2dbe9a0eb553/YAFCparser/Data/FactorioDataDeserializer_Entity.cs#L411

DaleStan commented 1 year ago

Issue can be seen in YAFC with just KS Power mod.

I reproduce the Sea Block Pack analysis error, but I can't reproduce this particular claim. With only KS Power enabled, YAFC determines that electricity is accessible. Even with SeaBlock (but not SeaBlockMetaPack), YAFC decides that electricity is accessible, so it looks to me like something added by SeaBlockMetaPack is confusing YAFC.

Will you find the minimum mod set required to get YAFC to come to the wrong conclusion, please?

KiwiHawk commented 1 year ago

Issue

With just KS Power mod, YAFC thinks that Wind Turbines require Electricity. This is incorrect. In this configuration, this doesn't cause the user any problem. But it's still incorrect. Instead, it should say Void (Power).

image

Minimum Mod List

If you want to see the Analysis warning from mods on the mod portal, mod list:

Test Mod

Alternatively, I've made a tiny test mod that demonstrates the same issue. Mod list:

YAFC_Test_1.0.0.zip

DaleStan commented 1 year ago

Ah, OK. I see now.

YAFC knows that it generates power:

YAFC also knows that it consumes power: image

I agree that changing https://github.com/ShadowTheAge/yafc/blob/fd80865b3429e99d592426f2107a2dbe9a0eb553/YAFCparser/Data/FactorioDataDeserializer_Entity.cs#L411 is inelegant, but I don't really see a better fix. Technically, YAFC's current behavior is correct, in that (as far as I can tell) all EEIs both consume and produce power. But the changed behavior matches the actual usage of EEIs in mods, which is to generate free power.

Nullius gets the wrong answer for its wind turbines and Stirling engines, but the answer after the change isn't particularly wronger than the answer before the change, and I think that's something that will require special hooks for Nullius.

veger commented 1 year ago

I ran in the same issue with Pyanodon mod-suite for the Multiblade "fish" Turbine. Adding factorioType != "electric-energy-interface" indeed fixes the issue.

I have no idea how EEIs work, but I guess the mod keeps inserting energy so it produces/delivers power to the network? YAFC knows (somehow) the amount/maximum power the EEI can provide (I guess it is part of the proptype?). Maybe it makes sense to add a UI component to let the user confirm that the EEI indeed delivers this amount of power manually?

It prevents that YAFC is incorrectly assuming that the EEI is indeed meant as a power source, but still allows the modded EEI act as power sources... :thinking: