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

Fix meBridge.listCraftableItems and meBridge.listCraftableFluids to not be quadratic. #570

Merged

Conversation

tomprince
Copy link
Contributor

Applied Energistics will return the set of all craftable things. Iterate over that to generate the list of craftable things, instead of something more complicated.

Previously, the code would

  1. Iterate over every item in the entire ME system.
  2. For each non-craftable item, iterate over every craftable item and a) create the corresponding lua object b) if the craftable item isn't present in ME system, check if the list of objects to return contains that object (which requires sequentially scanning the list of objects to return.

This meant that

  1. If there were no non-craftable items in the ME system, none of the craftable items that weren't present in the system would be returned.
  2. The code was quadratic or cubic in runtime.

PLEASE READ THE GUIDELINES BEFORE MAKING A CONTRIBUTION

  • Please check if the PR fulfills these requirements
  • The commit message are well described
  • Docs have been added / updated (for features or maybe bugs which were noted). If not, please update the needed documentation here. This is not mandatory
  • All changes have fully been tested
  • What kind of change does this PR introduce? (Bug fix, feature, ...)

This fixes a couple of bugs in listing craftable items/fluids in an ME system.

  • What is the current behavior? (You can also link to an open issue here)
  • Quadratic or cubic runtime for listing craftable items in an ME system.
  • Not listing craftable items that are not in the ME system, if there are no non-craftable items in the system.
  • Does this PR introduce a breaking change? (What changes might users need to make in their scripts due to this PR?)

This might change the order of the list of craftable items returned.

  • Other information:

This PR is against the 1.18 branch, since the modpack I am currently playing is a 1.18 modpack. I've not tested this change against newer versions.

…ot be quadratic.

Applied Energistics will return the set of all craftable things. Iterate
over that to generate the list of craftable things, instead of something
more complicated.

Previously, the code would

1. Iterate over *every* item in the entire ME system.
2. For *each* non-craftable item, iterate over every craftable item and
   a) create the corresponding lua object
   b) if the craftable item isn't present in ME system, check if the
      list of objects to return contains that object (which requires
      sequentially scanning the list of objects to return.

This meant that

1. If there were no non-craftable items in the ME system, none of the
   craftable items that weren't present in the system would be returned.
2. The code was quadratic or cubic in runtime.
@tomprince
Copy link
Contributor Author

This takes calling meBridge.listCraftableItems() for me in my current modpack from several plus seconds to basically instant. I have 777 items in my ME system, and 512 craftable items (of which 351 are not present in the ME system).

@SirEndii SirEndii self-requested a review March 28, 2024 13:18
@SirEndii SirEndii added the needs review Needs review from an Contributor label Mar 28, 2024
@SirEndii
Copy link
Member

Thank you for the PR!
I will test this a bit deeper and since it's for 1.18 port it to newer versions.

@SirEndii SirEndii changed the base branch from release/1.18 to dev/1.18.2 March 30, 2024 17:56
@SirEndii SirEndii merged commit c3058ec into IntelligenceModding:dev/1.18.2 Mar 30, 2024
@tomprince tomprince deleted the accidentally-quadratic branch March 30, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Needs review from an Contributor
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants