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

Preserve banner names across place and pick up #5565

Merged
merged 2 commits into from
Jun 30, 2024

Conversation

mjagdis
Copy link
Contributor

@mjagdis mjagdis commented Jun 19, 2024

As with named enchanting tables, copy banner names back and forth between items and block entities. NBT format verified against actual Java Edition chunk saves.

Fixes #5564

@@ -25,16 +25,20 @@ class cBannerEntity :

public:

cBannerEntity(BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, Vector3i a_Pos, cWorld * a_World);
cBannerEntity(BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, Vector3i a_Pos, cWorld * a_World, unsigned char a_BaseColor);
cBannerEntity(BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta, Vector3i a_Pos, cWorld * a_World, unsigned char a_BaseColor = 15, AString a_CustomName = "");
Copy link
Member

Choose a reason for hiding this comment

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

Previously the default BaseColor was 1, now it's 15?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit pedantic, I guess :-). Banner colours are inverted 0-15 so 15 is white (0 is black and 1 is just somewhat random). That said crafting a banner always sets damage (i.e. colour) on the crafted item, loading by WSSAnvil always sets the colour to whatever was saved and placing a banner always does a SetBaseColor using the source item's damage. Unless there's a way to magic a banner into existence from Lua (I haven't checked) those two defaulted args have no real purpose and could be vacuumed up?

@bearbin bearbin enabled auto-merge (squash) June 30, 2024 17:49
@bearbin bearbin merged commit b7de59d into cuberite:master Jun 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Banners lose their names after being placed and picked up
2 participants