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

[Forge] make processing pattern push items uncondensed and in order to the target inventory #6902

Conversation

gurmiguel
Copy link

Make Pattern Provider try to push items in order to the target inventory without condensing the ItemStacks first, only then, if there are remaining items from the recipe, it tries to jam items into valid slots.

@gurmiguel gurmiguel changed the base branch from forge/master to master January 18, 2023 20:48
@gurmiguel gurmiguel changed the base branch from master to forge/master January 18, 2023 20:49
@gurmiguel gurmiguel changed the title make processing pattern push items uncondensed and in order to the target inventory [Forge] make processing pattern push items uncondensed and in order to the target inventory Jan 18, 2023
remaining = handler.insertItem(i, remaining, simulate);
}
int emptySlotsOnlyTries = 1;
do {
Copy link
Member

@Technici4n Technici4n Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has unintended side effects: it will cause items to be fragmented in anything connected to a storage bus.

@Technici4n
Copy link
Member

Thanks for the contribution. The main thing I don't like with this is that separating the IInputs at the pattern level will also cause the crafting computation cost to increase a lot. (Each IInput needs to be resolved, this can easily combine at multiple levels and cause the size of the crafting tree to increase dramatically)

@Technici4n
Copy link
Member

This behavior would likely have to be opt-in, or done differently. However I feel like this would increase the configuration complexity of AE2 for a rather niche use case. So that's probably going to be a no from my side, sorry. I'm open to more feedback though.

@Technici4n Technici4n added the open-for-discussion Open for feedback from players or other devs label Feb 24, 2023
@gurmiguel
Copy link
Author

I see, as I said, it's my first try to contribute, so it's kinda hard to know of possible side effects. In the meanwhile, I'll try to figure out some possible solutions to the issues you mentioned, like making it opt-in for each process or something like that.
Thank you for the feedback.

@Starship-SpaceX
Copy link

The addition of this function can easily solve such problems, please be sure to add this function vectorwing/FarmersDelight#26

@Technici4n
Copy link
Member

This will not be implemented in that way, this is fundamentally a farmer's delight problem and not an AE2 problem.

I am fine with pushing the items in the original order, I think it's a good idea. However we will NOT prevent them from stacking in the target.

@Starship-SpaceX
Copy link

Well, the above problems are not unsolvable now, though. If I rename the item, it prevents the item from stacking in the target container, or I choose to issue a set of items for each empty slot, I just wish there was an easier option that would allow me to directly address this type of issue

@Technici4n
Copy link
Member

Superseded by #7007. As explained above, this will only work if the target inventory prevents the items from stacking in some way, but there's nothing better I can do.

@Technici4n Technici4n closed this Apr 1, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
open-for-discussion Open for feedback from players or other devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants