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

(Investigation) Why do recipes sometimes show up for disabled recipes and sometimes do not? #122

Closed
Dorus opened this issue May 10, 2024 · 4 comments · Fixed by #130
Closed
Labels
help wanted Extra attention is needed

Comments

@Dorus
Copy link
Collaborator

Dorus commented May 10, 2024

From #121 (comment)

One thing i noticed while investigating this:

Before when you disable a recipe, the item row is emptied. Old projects still have this state stored:

afbeelding

If i enable them now and disable again, i get the recipes visible: afbeelding

I'm not sure if this was intended, i tried to go to older versions along the master branch, but couldn't find any yet without this problem until i ran into the ones with the libf problem.

@Dorus Dorus added bug Something isn't working and removed bug Something isn't working labels May 10, 2024
@shihan42 shihan42 added the help wanted Extra attention is needed label May 10, 2024
@DaleStan
Copy link
Collaborator

DaleStan commented May 13, 2024

For what it's worth, this behavior shows up in every version I tested back to v0.5.5.

@DaleStan
Copy link
Collaborator

I've figured out what's going on here. My inclination is almost to make disabled recipes look like this
image
but it’s not obvious how to make that module/beacon display work on a freshly-loaded file.

Either of these are easy. I can also display boxes for the building and fuel, or I could limit the display to just a single box even if there are multiple inputs or products, like the module display.
image

In all three cases, the link information is not displayed. For one, I figure that if you cared about whether you had production for small parts, you shouldn't have disabled that recipe, and for another, that information doesn't get updated for disabled recipes.

What thoughts do you have?

@Dorus
Copy link
Collaborator Author

Dorus commented May 14, 2024

I would keep buildings and fuel since that's an important decision of what the recipe actually is. As long as it's clear these are not enabled. Hiding the ingredients takes away a lot of visual clutter so that's preferable i think (?)

@Dorus
Copy link
Collaborator Author

Dorus commented May 14, 2024

In all three cases, the link information is not displayed.

It's still displayed on the other half of the link (if any). So that's fine i think.

DaleStan added a commit to DaleStan/yafc that referenced this issue May 14, 2024
Specifically, always show empty boxes for both ingredients and products.
@DaleStan DaleStan linked a pull request May 14, 2024 that will close this issue
shpaass pushed a commit to DaleStan/yafc that referenced this issue May 14, 2024
Specifically, always show empty boxes for both ingredients and products.
shpaass pushed a commit to DaleStan/yafc that referenced this issue May 14, 2024
Specifically, always show empty boxes for both ingredients and products.
shpaass added a commit that referenced this issue May 15, 2024
Always show empty boxes for both the ingredients and products of disabled recipes. 
That is, they'll always look like this:

![image](https://github.com/have-fun-was-taken/yafc-ce/assets/21223975/d60eeefa-f56d-4d9c-afc7-7dcd8759f784)

I also discovered the code that cleans up old links removes links if the
item/fluid does not appear in any _enabled_ recipes. I wanted the links
to stay put when disabling and enabling recipes, and I couldn't
concentrate on #122 with that behavior, so I changed the code to
consider all recipes. If that change (f28d35f) should be discussed more,
I can remove it from this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants