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

Remove the CT Ingredient Wrapper #1618

Merged
merged 6 commits into from
Mar 18, 2023
Merged

Remove the CT Ingredient Wrapper #1618

merged 6 commits into from
Mar 18, 2023

Conversation

TechLord22
Copy link
Member

What

This PR removes the CT Ingredient Wrapper for recipes, instead converting the provided CTRecipeBuilder input directly to GTRecipeInputs. This will prevent future bugs with CT-specific behavior we would otherwise be unaware of, and also defines the behavior of all input scenarios CT can provide. This will us to choose which configurations we support.

As of this PR, items used in recipes with NBT will match any items that contain all of the recipe's NBT. As long as the sample completely overlaps with the recipe, matching will succeed. More complex matching can be achieved with GroovyScript instead, if desired.

For inputs representing multiple items, typically produced using CT's | operator, the NBT for each stack ORed together will be matched for exclusively the stack it applies to. See the validation section for details.

Validation

Using the following script, simple validation of this behavior can be performed.

<recipemap:assembler>.recipeBuilder()
    .inputs(<minecraft:dirt>.withTag({RepairCost: 0, display: {Name: "MyName"}}))
    //.inputs(<minecraft:dirt>.withTag({display: {Name: "MyName"}}))
    //.inputs(<minecraft:dirt>.withTag({display: {Name: "MyName"}}) | <minecraft:stone>.withTag({RepairCost: 0}))
    .outputs(<metaitem:dustSilver>)
    .duration(20).EUt(30).buildAndRegister();

Renaming dirt in the anvil to MyName will allow testing this the easiest, leaving only one input line uncommented for each of the following scenarios:

  • With the first input line, the renamed dirt will match the tags exactly.
  • The second input line will match the renamed dirt as well, as it contains all of the required tags.
  • The third input line can be tested with /give @p minecraft:dirt 64 0 {display: {Name: "MyName"}} and /give @p minecraft:stone 64 0 {RepairCost: 0}. Both of the stacks for the third case match one of the valid input ingredients.

Outcome

Removes the CT Ingredient Wrapper to prevent undefined behavior. Supersedes #1604, and also fixes #1600.

Potential Compatibility Issues

No compatibility issues are expected other than those outlined above, which are now considered invalid input configurations.

@TechLord22 TechLord22 added the type: refactor Suggestion to refactor a section of code label Mar 17, 2023
Copy link
Contributor

@PrototypeTrousers PrototypeTrousers 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

@TechLord22 TechLord22 force-pushed the tc-remove-ct-wrapper branch from ffb4bcc to fd4240a Compare March 18, 2023 03:11
@TechLord22 TechLord22 added this to the 2.5.5 milestone Mar 18, 2023
Copy link
Contributor

@LAGIdiot LAGIdiot left a comment

Choose a reason for hiding this comment

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

Implementation looks reasonable. Ingame test was not performed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: refactor Suggestion to refactor a section of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failure to register CT made PBF recipes
3 participants