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

Optionally Use Search Engine from JEI or REI #7311

Closed

Conversation

MarkusTieger
Copy link

@MarkusTieger MarkusTieger commented Aug 2, 2023

Allows to use the Search Engine from JEI or REI.

This can be useful because it is annoying when you try to search something with the modid and the name like it is possible in JEI. For example "@ae2 crystal". Without it, it tries to search a mod with the name "ae2 crystal".
It is configurable in the Terminal Settings. (opt-out to disable btw)

@shartte
Copy link
Member

shartte commented Aug 2, 2023

Thanks for this PR, this is an interesting feature. I'll need to take an in-depth look at this w.r.t. performance and behavior when JEI/REI are absent.

Comment on lines +48 to +49
if (!AEConfig.instance().isSyncWithExternalSearch())
filter.setFilterText(old);
Copy link
Member

Choose a reason for hiding this comment

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

Probably should not have this side effect even IF external search syncing is enabled hmm I will look at this

Copy link
Author

Choose a reason for hiding this comment

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

Else the current search in jei will be overriden with the ae2 search, even if "sync with external search" is off. Thats the "fix" for it.

var old = filter.getFilterText();
filter.setFilterText(searchString);

var filtered = new ArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

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

List and raw type. Needs to be a set.

Copy link
Author

Choose a reason for hiding this comment

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

I don't see why it's needed for type "Object".
Intellij doesn't either.
image

@shartte shartte changed the base branch from master to main November 11, 2023 18:57
@Mari023 Mari023 mentioned this pull request Nov 14, 2023
@shartte
Copy link
Member

shartte commented Nov 21, 2023

@MarkusTieger Sorry for not coming back to you earlier. I am in the process of moving master to Neoforge, so it'll take a while before I can get to this PR.
I also would like to actually refactor the functionality significantly, namely that "use JEI search" (the BIG radio buttons we currently have) should probably use this functionality instead of just hiding our own search box, but then use different filter logic.

All in all: it will take a while 😓

@MarkusTieger
Copy link
Author

@MarkusTieger Sorry for not coming back to you earlier. I am in the process of moving master to Neoforge, so it'll take a while before I can get to this PR. I also would like to actually refactor the functionality significantly, namely that "use JEI search" (the BIG radio buttons we currently have) should probably use this functionality instead of just hiding our own search box, but then use different filter logic.

All in all: it will take a while 😓

no problem

@Relvl
Copy link

Relvl commented May 15, 2024

Any progress? We all very need this change! :)

@MarkusTieger
Copy link
Author

Any progress? We all very need this change! :)

I almost forgot this issue, thanks for reminding me. This week I am busy, but I should be able to update this pr in the next week.

@Relvl
Copy link

Relvl commented May 23, 2024

Also maybe add JEI sorting function? #7827

@shartte
Copy link
Member

shartte commented May 23, 2024

We no longer support JEI in 1.20.6+ at the moment, so that is not going to work...

@MarkusTieger
Copy link
Author

MarkusTieger commented Jun 8, 2024

I guess this issue is no longer required and #7883 will resolve the issue with the bad search?

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.

None yet

3 participants