Skip to content
This repository has been archived by the owner on May 27, 2024. It is now read-only.

SmithingTransformRecipe inproper itemmeta handling #53

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

Boy0000
Copy link
Contributor

@Boy0000 Boy0000 commented Jan 14, 2024

No description provided.

@Boy0000 Boy0000 requested a review from 0ffz as a code owner January 14, 2024 16:15
Copy link
Member

@0ffz 0ffz left a comment

Choose a reason for hiding this comment

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

Left some questions

@@ -10,13 +10,15 @@ interface ItemRecipes {
val query: ItemRecipeQuery

companion object : GearyAddonWithDefault<ItemRecipes> {
val recipes by lazy { default().query.registerRecipes() }
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of moving here vs in install?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mostly because before we would need to re-register to get all custom recipes
mostly forgot to remove it from some testing tbh

val customRecipeResult = smithingTransformRecipes.filter { it.template.test(template) && it.addition.test(mineral) && it.base.itemStack.itemMeta?.persistentDataContainer?.decodePrefabs()?.firstOrNull() == inputGearyEntity }.firstOrNull()?.result
var recipeResultItem = (customRecipeResult ?: ItemStack.empty()).let { result?.toSerializable()?.toItemStack(it, EnumSet.of(SerializableItemStack.Properties.DISPLAY_NAME)) }

recipeResultItem = recipeResultItem?.editItemMeta {
Copy link
Member

Choose a reason for hiding this comment

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

Is this part necessary? the toItemStack should override as needed and keep the displayName if the ignored property is set. Is this maybe covering some case I'm not considering?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

newResult is applied ontop of oldResult
this is because otherwise the finalResult will have the prefabs from oldResult aka Diamond Sickle
this means however that the finalResult will have the displayname of oldResult aka Diamond Sickle, or if renamed the renamed

so it does indeed require to fix that afterwards from my testing to get correct behaviour

@Boy0000 Boy0000 merged commit 002fe78 into master Jan 18, 2024
1 check passed
@Boy0000 Boy0000 deleted the smithing-transform-fix branch January 18, 2024 09:56
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