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

multi Level Cache #5117

Open
wants to merge 4 commits into
base: 1.18.2
Choose a base branch
from

Conversation

ferriarnus
Copy link

Adds a second level cache to the melting inventory.
The constructor of a melting module now takes the parent inventory instead of the block entity. If the Block entity is needed, it can be gotten from the inventory.

@ferriarnus
Copy link
Author

So testing these changes with a debugger:

1ste slot iron -> Recipe manager look up
1ste slot iron -> Slot cache
2nd slot iron -> Inventory cache
3rd slot gold -> Recipe manger look up
1ste slot gold -> Inventory cache
2nd slot iron -> Slot cache
3rd slot iron -> Recipe look up / 4rd slot gold -> Inventory cache

Looking at these last 2 interactions, for the 2d slot, the slot cache can be used. However it does not set the inventory cache when used, making it so a look up is used later. Should this be changed so using the slot cache also updates the inventory cache? Or am I missing a case? The advantage of this setup is that if instead of the last iron, gold was used in slot 4, the inventory cache would resolve it.

@ferriarnus ferriarnus marked this pull request as ready for review March 21, 2023 09:32
@KnightMiner
Copy link
Member

KnightMiner commented Mar 21, 2023

I am not sure the slot cache should override the inventory cache on a cache hit, if they differ, odds are the next item in the smeltery will also be different. Probably best to stick with what we have, as that is simplier.

The important thing is that when finding a new recipe, it is cached at both the slot and inventory level. Additionally, when a slot has a cache miss, it should cache the inventory recipe even if that was a cache hit. Both of these appear to be done already

@KnightMiner KnightMiner added the 1.18 Issue affects 1.18 label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.18 Issue affects 1.18
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants