Skip to content

Apify filters #8132

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

Open
wants to merge 2 commits into
base: mc1.20.1/dev
Choose a base branch
from

Conversation

ChiefArug
Copy link

Fixes #7706
There was A LOT of hardcoding there, wow.

Splits FilterItem and its mixed usage of the FilterType enum and /AllItems\.[A-Z]_?FILTER\.isIn/ method into four classes. FilterItem itself retains most of the logic, and it has three subclasses for ListFilterItem, AttributeFilterItem and PackgeFilterItem. This makes filters so much more extensible for addons, ie they can actually extend it and should be able to implement custom filtering behavior without requiring mixins.

I have tested this and it still works, including basic filter behavior and blueprints (overlay rendering and jei-autofilling).

WAIFU Analysis:

I have only looked at classes that have changes to existing methods, not just new methods.

  • Two mods mixin to FilterItem:
    • CreateBigCannons - The target method did not get modified, so this mixin will not break.
    • Jade Addons - The target method was made public, so this mixin will no longer be needed. In the meanwhile, it won't break because the signature did not change.
  • No mixins to BlueprintItem
  • One mixin to BlueprintOverlayRenderer:
    • Jade Addons - The field this makes a getter for was not modified in any way.
  • No mixins to AbstractFilterScreen
  • Two mixins to FilterItemStack:
    • Destroy - The method and field this uses was not touched.
    • CogsOfCarmite - The target method and instruction still exist, and their filter item does not extend FilterItem (that was actually impossible, the constructor was private) so it will not enter the if statement.
  • No mixins to FilterMenu

Therefore while this changes a lot it should not break addons at all, meaning it can be merged safely.

I would also like to get @jodlodi and @sashafiesta's opinions on this as you are the ones going to use it.

@ChiefArug
Copy link
Author

Build error does not seem to be related to my changes.

@VoidLeech VoidLeech added area: api Issue or PR is related to API pr type: feature PR adds a new feature or changes an existing feature labels Mar 27, 2025
@jodlodi
Copy link
Contributor

jodlodi commented Mar 27, 2025

This will be a godsend for me, making a custom filter was a bit of a nightmare

@ChiefArug
Copy link
Author

One thing I did not do is move anything to the API package. This is an option, but would break addons badly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api Issue or PR is related to API pr type: feature PR adds a new feature or changes an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Abstracting FilterItem class
3 participants