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

feat: add thaumcraft integration to search for essentia #24

Merged
merged 3 commits into from
Oct 21, 2024

Conversation

leumasme
Copy link

@leumasme leumasme commented Oct 8, 2024

Closes GTNewHorizons/GT-New-Horizons-Modpack#17591

temp.mp4

Searching for an Aspect Item from Thaumcraft NEI Integration will find Items (Phials, Jars) and Blocks (Nodes, Jars, ...) that contain essentia of that aspect.

This currently only happens when doing the Find Item on the Aspect Item specifically. It could easily be added that this also happens when pressing it on a filled Phial or Jar directly, if that's desirable (would mirror the behavior for fluid ex. water bucket).

@Dream-Master Dream-Master requested a review from a team October 8, 2024 20:09
@leumasme
Copy link
Author

leumasme commented Oct 8, 2024

The way that the Filter system since #22 is structured is a bit inverted for this (and in general?).
A filter is created for the TileEntity / ItemStack that is currently being inspected to check if it's a matching one, not the one that we're searching for.
This way, each filter is only being used once (Can't search for multiple Items at once + Filter isn't cached (not that it should)).
This means that (if necessary) extracting information from the ItemStack we're searching for has to be done again for every TileEntity/ItemStack that is getting checked. Here, that extra work is 'just' getting the Aspect of the Aspect Item, but in the future if there is a case that needs more work done on the item being searched, this design may be very inefficient,
so it seems like an odd choice.

@slprime
Copy link
Member

slprime commented Oct 17, 2024

The way that the Filter system since #22 is structured is a bit inverted for this (and in general?). A filter is created for the TileEntity / ItemStack that is currently being inspected to check if it's a matching one, not the one that we're searching for.
This way, each filter is only being used once (Can't search for multiple Items at once + Filter isn't cached (not that it should)). This means that (if necessary) extracting information from the ItemStack we're searching for has to be done again for every TileEntity/ItemStack that is getting checked. Here, that extra work is 'just' getting the Aspect of the Aspect Item, but in the future if there is a case that needs more work done on the item being searched, this design may be very inefficient, so it seems like an odd choice.

Correct. this is necessary because each block can have its own algorithm for reading its inventory.
It is impossible to build a filter from the side of the ItemStack you are looking for, since the filter is unique for each TileEntity/ItemStack in which you need to search (If you have an idea how to fix this, give an example)

@slprime
Copy link
Member

slprime commented Oct 17, 2024

This currently only happens when doing the Find Item on the Aspect Item specifically. It could easily be added that this also happens when pressing it on a filled Phial or Jar directly, if that's desirable (would mirror the behavior for fluid ex. water bucket).

I don't have an answer to that. I don't play with magic and I don't know how it would be convenient.

@YannickMG
Copy link

This currently only happens when doing the Find Item on the Aspect Item specifically. It could easily be added that this also happens when pressing it on a filled Phial or Jar directly, if that's desirable (would mirror the behavior for fluid ex. water bucket).

I don't have an answer to that. I don't play with magic and I don't know how it would be convenient.

Well aspects aren't actual items, so you would have to go out of your way to search them in NEI before you could ever make use of this feature.

Expanding this feature to include phials and jars sounds like a sensible way to allow people to discover it. Most likely you would want to interpret the search as an "item search" if the phial/jar is empty and as an "aspect search" otherwise.

@leumasme
Copy link
Author

leumasme commented Oct 17, 2024

It is impossible to build a filter from the side of the ItemStack you are looking for, since the filter is unique for each TileEntity/ItemStack in which you need to search (If you have an idea how to fix this, give an example)

It's possible - just do the logic of extracting the needed data from the TE in the match method of the filter instead of the constructor. The Filter would then be specific to the item we're searching for instead of the TE/Stack in which we're searching, and the logic to process the inspected TE/Stack would be moved from the constructor of the filter to the match method.
In pseudocode, currently it's

class SomeFilter {
constructor(inspectedThing) {
  // Extract needed data from inspected thing, ex. for backpack/chest, get the contents of it
}
boolean match(searchedItem/searchRequest) {
  // Extract needed data from the Item we're searching for, ex. for water bucket, get the water fluid
  // Check if the extracted data of the inspectedThing matches the extractedData of the searchedItem
}
}

if we just invert it, then it's

class SomeFilter {
constructor(searchedItem/searchRequest) {
  // Extract needed data from the Item we're searching for, ex. for water bucket, get the water fluid
}
boolean match(inspectedThing) {
  // Extract needed data from inspected thing, ex. for backpack/chest, get the contents of it
  // Check if the extracted data of the inspectedThing matches the extractedData of the searchedItem
}
}

(Whether the extracting from inspectedThing is actually done in the constructor or in the getFilter which calls the constructor is irrelevant here, it's the same)
Since now the filter is created for the searched item instead of the inspected TE/Item, it can be re-used so we don't have to call the constructor so amny times and we don't have to repeat extracting the data from the searchedItem every time.

This would mean the sigature of IStackFilter would change from

public interface IStackFilter {
    boolean matches(FindItemRequest request);
}

to

public interface IStackFilter {
    boolean matches(ItemStack stack);
    boolean matches(TileEntity tileEntity);
}

and similarly, getFilter of providers would get passed the FindItemRequest or the searched ItemStack instead of the inspected ItemStack/TileEntity.
Now, for example for a Backpack Provider, the IStackFilter would be constructed with the item we're searching for, and the matches(ItemStack stack) method would check if stack is a backpack, extract the inventory from the backpack, and check if the inventory contains the stack that it was constructed with.
This does mean that most Providers won't return null for getFilter in most cases, but it would still drastically reduce the amount of Filters constructed, since we're now scaling with the amount of Providers instead of the amount of inspected Stacks/TEs

@leumasme
Copy link
Author

temp.mp4

Now with searching by essentia container

@leumasme leumasme requested a review from slprime October 17, 2024 15:13
@Dream-Master Dream-Master merged commit 1dd6693 into GTNewHorizons:master Oct 21, 2024
1 check passed
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.

Let FindIt find essentia within Jars
4 participants