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

Add optional built building count to recipe rows (v2) #16

Merged
merged 6 commits into from
Feb 6, 2024

Conversation

veger
Copy link
Collaborator

@veger veger commented Feb 5, 2024

This PR extends PR ShadowTheAge#118 by @Saklad5 (I rebased their commits for YAFC-CE)

Users can now specify the actual number of buildings that are built for a recipe. This has no impact on the model itself, but makes it easier to tell if existing infrastructure needs to be revisited following a change.

A certain amount of surplus production capacity is almost always necessary, whether to futureproof, maintain belt balance, maximize beacon utility, or because demand is met by a non-integral number of machines. With this feature, that surplus can be tracked even if the recipe is linked.

My own addition is to make the check recursive so it shows the error on the top recipe and all subgroups until the actual recipe that has not enough buildings.

image

You can see that Tree seedling requires 1.04 build instead of the 1 I have. The parent group (Wood) also has the exclamation mark and error message.
And the sheet itself shows a red error box with the message as well. (not visible in the screenshot).

This makes lack of buildings more clear, compared when they are hidden in multiple nested groups deep.

Saklad5 and others added 6 commits February 5, 2024 21:31
This variable, which defaults to null, can be used to track the number of buildings that have actually been built. This number has no impact on the model.
Dropdown menu items for setting or clearing the builtBuildings value have been added for each recipe row.

These items mirror the behavior and implementation of those for fixedBuildings, for the most part.
The initial value is buildingCount rounded up, or 0 if buildingCount is negative.
As 0 is a valid value, clearing builtBuildings returns it to null.

Shopping lists now use the builtBuildings value instead of buildingCount if the former is not null.
The text field is only visible if builtBuildings is not null, and is placed under the building count. It is colored grey to indicate that it is not playing a role in the model.
If the number of buildings needed for the model's solution is greater than the specified number of built buildings, the recipe will now be flagged accordingly.
This makes it substantially easier to determine which recipes need to be revisited if a change is made.
To make issues more apparent, the entire page now displays an error message (like with deadlocks) if a recipe needs more machines than it currenty has built.
@shpaass
Copy link
Owner

shpaass commented Feb 6, 2024

A brief sanity test showed no obvious errors.
@veger what do you think about YAFC doing the equivalent of clicking the button "Clear built building count" if a user were to leave nothing in the field? Would that be more convenient, or would that be counter-intuitive?

@veger
Copy link
Collaborator Author

veger commented Feb 6, 2024

A clear/empty value could be convenient to remove the count, I guess (only after losing the focus, otherwise it will be annoying to type).
I rarely unset the building count, as I use it a lot to make sure scaling up shows all building counts that are lacking. Unsetting this count, would hide potential issues.

BTW allowing 0 values, as @Saklad5 implemented, is quite nice as it helps me often remembering that I need to build some stuff. (Especially when I make big changes for new science pack for example)

@shpaass shpaass merged commit e83041c into shpaass:master Feb 6, 2024
@veger veger deleted the build-amount-v2-ce branch February 6, 2024 19:10
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

Successfully merging this pull request may close these issues.

3 participants