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

add pattern blame #398

Merged
merged 4 commits into from
Oct 7, 2023
Merged

add pattern blame #398

merged 4 commits into from
Oct 7, 2023

Conversation

Glease
Copy link

@Glease Glease commented Oct 5, 2023

so you know who was that idiot adding broken pattern
Closes GTNewHorizons/GT-New-Horizons-Modpack#14297

image

@github-actions
Copy link

github-actions bot commented Oct 5, 2023

Warning: 2 uncommitted changes
#399

@Rika-TH
Copy link

Rika-TH commented Oct 5, 2023

Perfection, keep whoever makes the bad ones in charge of AE to get updoots 👍

@JodieRuth
Copy link

Kick this idiot out of the server
image

firenoo
firenoo previously approved these changes Oct 5, 2023
Copy link

@firenoo firenoo left a comment

Choose a reason for hiding this comment

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

Resolve regardless of action taken before merging

@firenoo firenoo added the merge by assignee only assignee should merge label Oct 5, 2023
@Glease
Copy link
Author

Glease commented Oct 5, 2023

we have discovered a subtle problem with the implementation. if there were two different people encoded the same recipe (or a recipe with no encoder information), they will be treated as two different pattern, and AE won't load balance between the two. I have come up with a solution of preprocessing the item used for equality check in PatternHelper, but I'm not sure if this is worth it (as copying an NBT can be expensive)

@firenoo
Copy link

firenoo commented Oct 5, 2023

Why not change PatternHelper to check only input/output stack nbt matches instead of full copy? Preferrably cache equivalent patterns

@Glease
Copy link
Author

Glease commented Oct 5, 2023

well AE2 has a shared NBT cache, so it'd actually be faster to compare full stack (which is just a == check between the two NBT tag) than to compare by indiviual inputs (which will need to iterate over every inputs and go into every subtag the inputs/outputs have)

@firenoo
Copy link

firenoo commented Oct 5, 2023

PatternHelper usages needs to be cleaned up anyways as its instantiated way too often so I think its fine the way you implemented it (with those patches)

@Dream-Master Dream-Master requested review from firenoo and a team October 5, 2023 21:01
@Glease Glease merged commit 4ca326e into master Oct 7, 2023
1 check passed
@Glease Glease deleted the feature/blame branch October 7, 2023 02:59
@Glease
Copy link
Author

Glease commented Oct 7, 2023

we will see how much of a performance impact this has

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge by assignee only assignee should merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request Signatured AE2 encoded. patterns
4 participants