Repository navigation
Keep ArmourShop skins when a mage weapon is rebuilt - #55
Merged
Merged
Conversation
A refresh or socket rewrite rebuilt the weapon from its MMOItems template and the part model scheme, which dropped any skin the player had applied and wasted the skin scroll. Players hit this after socketing a rune: the next hotbar switch refreshed the staff and reset its model. GearSkin puts the old weapon's skin back through TLibs' ItemSkinPreserver. A weapon whose amodel tag no longer matches its model already lost its skin, so it keeps the part model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
the
size:M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to The skin-preservation paths match the intended behavior; no identified issue prevents merging after normal checks.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AlokVocayne reported on Main that a skinned staff lost its skin after switching hotbar slots away and back, which cost them two skin scrolls. Their saved staff still carries the Heir Staff
amodel 66tag, but its model is back to 2, the steel scheme's staff. Something rebuilt the weapon after the skin went on.Two paths rebuild a mage weapon:
GearRefresher(hotbar switch, click, join, chest open,/magic refresh, station reclaim) andGearItemBuilder.rewriteSockets(charging). Both build from the MMOItems template and then apply the part model scheme, so neither kept a skin. The likely trigger is socketing a rune. MMOItems cancels that click, so Magic's click refresh skips it and the rune stays unstamped until the next hotbar switch, which then rebuilds the staff.GearSkin.keepnow runs after the scheme model on both paths and puts the old weapon's look back through TLibs'ItemSkinPreserver, the same helper AdvancedCrafting uses. A weapon only counts as skinned when itsamodeltag matches its current model, or when it has an ItemsAdder skin without one. Alok's staff, whose tag outlived its skin, keeps the part model and does not get a free Heir Staff skin back.The POM pins
local:item-nbt-api-pluginas provided, matching AdvancedCrafting, becauseItemSkinPreserverneeds NBTAPI types for Mockito to load it in tests. The ServerAssets catalogue already lists it.Main runs Magic 0.4.17 and this branch builds on the unreleased character-identity work (TLibs 2.3.0), so the fix ships with that set unless a 0.4.x hotfix is wanted.
Docs: TF-Minecraft/Docs#191.
mvn clean verifypasses locally: 503 tests, 100% line, branch and instruction coverage.🤖 Generated with Claude Code