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

Rethink basic pattern items and expose a pattern item builder to Addons #7770

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

shartte
Copy link
Member

@shartte shartte commented Mar 16, 2024

No description provided.

@shartte shartte requested a review from XFactHD March 16, 2024 01:59
Copy link
Collaborator

@XFactHD XFactHD left a comment

Choose a reason for hiding this comment

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

This solution is a lot saner than my bruteforce idea of just moving them item to API and it would be perfectly suitable for my needs. Is there any particular reason to use a separate interface for the pattern decoder instead of the existing IPatternDetailsDecoder which has to be registered to PatternDetailsHelper anyway?

The only additional thought I have (which my PR wouldn't have solved either) is the ability to control how the InvalidPatternHelper decodes the inputs and outputs from broken patterns. In my case, I only keep around the registry name of the primary input and the output. The secondary inputs are stored separately. The current implementation wouldn't be able to decode them at all and if I adapt my input and output to be a list tag, then it would still miss the secondary inputs. The additional methods needed for that could be added to IPatternDetailsDecoder if that class were re-used instead of a new interface.

@shartte
Copy link
Member Author

shartte commented Mar 16, 2024

Is there any particular reason to use a separate interface for the pattern decoder instead of the existing IPatternDetailsDecoder which has to be registered to PatternDetailsHelper anyway?

Yeah that is really only needed if you want to use a fully custom item. If you use the new builder you do not need to register an additional decoder, since AE2 already registers a decoder that can handle all Items created by the builder. At the end of the day the IPatternDetailsDecoder ends up only being useful if a mod has an existing mechanism to encode patterns and wants to make those compatible with AE2.

I will look into enhancing this new builder to also replace the mechanism to describe invalid patterns

@shartte
Copy link
Member Author

shartte commented Mar 18, 2024

@XFactHD Alright, added the ability to customize how the "invalid pattern tooltip" is created
Including a tooltip builder to make it easier for us to maintain a common style..
Can you check if that works for you?

@shartte shartte force-pushed the rework-pattern-items branch from 1fced97 to bac309d Compare March 18, 2024 13:49
Copy link
Collaborator

@XFactHD XFactHD left a comment

Choose a reason for hiding this comment

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

Looks good to me, this will work very well for my needs :shipit:

@shartte shartte merged commit 9618e5f into main Mar 18, 2024
6 checks passed
@shartte shartte deleted the rework-pattern-items branch March 18, 2024 22:20
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants