Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discussion] Prevent loading certain mods #286

Open
DaleStan opened this issue Sep 13, 2024 · 5 comments
Open

[Discussion] Prevent loading certain mods #286

DaleStan opened this issue Sep 13, 2024 · 5 comments

Comments

@DaleStan
Copy link
Collaborator

Some mods, such as fluid_permutations or WideChests, should probably not be loaded by Yafc. fluid_permutations adds accessible recipes that help in Factorio but not in YAFC, while WideChests adds lots of inaccessible entities that may cause spurious "More than 50% of all in-game objects appear to be inaccessible" warnings.

I'd like to prevent loading such mods, but there are some questions first:

  1. What should the default list be?
    Anything other than "empty" requires maintenance on our side, but "empty" is not particularly helpful to a user.
  2. Where should the list be stored?
    a. In a per-user (the Yafc.Preferences class) or per-mod-folder (a new mod-list.yafc.json file) locations?
    These make an empty default least painful and the user selections most persistent, but they also mean a bug report needs 3 files (project, save, exclusion list) instead of 2.
    b. In the project file?
    This means a bug report still only needs 2 files, but requires loading mod-list.json, part of the project file, the mods, and then the rest of the project file.
    c. In the source code?
    This also means a bug report still only needs 2 files, but makes an empty default completely useless, and forces more maintenance work on the developers.
  3. Do we want this at all?
    It might encourage future lua issues, akin to the flib bug, crash: mods: Deadlock-SE-bridge + deadlock-beltboxes-loaders + space-exploration #105, or Error While loading mod Kux-OrbitalIonCannon (SE + K2) #145, to be worked around by just not loading those mods, instead of creating a bug report.
@shpaass
Copy link
Owner

shpaass commented Sep 14, 2024

What should the default list be?

You mean, the default list of mods that are not loaded? WideChests is the first mod that I'd add to that list. After that, we can ask the pY community to list more of them.

Added my thoughts below.

@shpaass
Copy link
Owner

shpaass commented Sep 14, 2024

I'd like to prevent loading such mods

I faced a similar case recently. A person was filing a bug report, and they were disabling mods, for Yafc to use as few mods as possible when keeping the issue reproducible.

The solution I came up with is to copy the mods folder and disable the mods in a copied mod-list.json, so Yafc doesn't load some of the mods.

I'd like to know if this solution satisfies your goal of disabling some mods.

@veger
Copy link
Collaborator

veger commented Sep 14, 2024

  1. Do we want this at all?

I wonder besides the point you already made:

What would happen if a user disabled the 'wrong/random' mod and the whole project does not make sense (like disabling a core pY mod)? YAFC would behave weird/unexpected and we get reports about this. Just adding burden/wasting our time. It feels like a 'heavy/serious' setting to me, that should not be of a users toolset.

But then if we keep such a list: disabled mods could change, so we'd need to keep track of them to re-enable or disable again. Also there are loads and loads of mods, we cannot track them all...

And how would we decide to which mod we should 'protect' the users from. In a big set, there are always outliers that do not agree with (such) moderation. So converging to a perfect solution is quite impossible.


fluid_permutations adds accessible recipes that help in Factorio but not in YAFC

So just load, there are more recipes, but it doesn't matter?

WideChests adds lots of inaccessible entities that may cause spurious "More than 50% of all in-game objects appear to be inaccessible" warnings

This is a worse UX, but nothing breaking as well? Especially if we would improve the warning point the user how they can check if this is an issue or expected (eg by opening NEIE to see what is disabled).


For narrowing down issues (as @shpaass describes) it might be a convenient solution though...

I notice that am quite on the fence about this, one side it is impactful and might bother us and others, on the other hand it is kind of nice... 😄

@DaleStan
Copy link
Collaborator Author

The solution I came up with is to copy the mods folder and disable the mods in a copied mod-list.json

Using separate mod folders for Factorio and Yafc is a lot like storing the exclusion settings in the mod folder, except that Factorio doesn't know about Yafc's folder, and can't update the mods or startup settings in that folder.

fluid_permutations adds accessible recipes that help in Factorio but not in YAFC

So just load, there are more recipes, but it doesn't matter?

It's been long enough I forgot the extra recipes are inaccessible, so it's basically the same problem as for wide chests. It's slightly worse if you want to add an unbarrelling recipe as an oil source. Vanilla only has 5 recipes for producing petroleum gas, and displays all 5, but vanilla+permutations has 28, and shows the 4 normal recipes plus two inaccessible recipes in the dropdown. (So maybe inaccessible should sort after special?)

@shpaass
Copy link
Owner

shpaass commented Sep 15, 2024

Using separate mod folders for Factorio and Yafc is a lot like storing the exclusion settings in the mod folder, except that Factorio doesn't know about Yafc's folder, and can't update the mods or startup settings in that folder.

Do we want this [functionality] at all?

I'd say I wouldn't bother with it, but if you feel like implementing this idea, then I'm fine with it being in Yafc.

Regarding the location, I'd choose to store the list in a per-mod-folder fashion if Factorio doesn't wipe stray files in this folder. If all is well, that would allow Yafc to swap between projects without keeping a separate map between blocklists and projects.

shpaass added a commit that referenced this issue Sep 16, 2024
While organizing my thoughts about #286, I convinced myself this was a
good thing to do regardless of what else (if anything) happens on that
issue.

If there's an error loading a mod (e.g. Kux-BlueprintExtensions) the
user will have the opportunity to disable that mod and load the
remaining mods again. Based on not wanting wanting to add a third file
to bug reports and shpaass
[saying](#286 (comment))
"I'd choose to store the list in a per-mod-folder fashion", I decided to
remember the disabled mods only until the user changes the mod folder or
exits Yafc. If the error isn't obviously related to a mod, the button
will not appear.


![image](https://github.com/user-attachments/assets/818e148a-a6c6-48ed-b841-68a9cb0814ea)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants