Skip to content

Add GlowAdvancementDisplay flags #1105

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

Merged
merged 6 commits into from
Feb 11, 2021
Merged

Add GlowAdvancementDisplay flags #1105

merged 6 commits into from
Feb 11, 2021

Conversation

x4e
Copy link
Contributor

@x4e x4e commented Jan 27, 2021

Just implementing a TODO, added flag definitions for the AdvancementDisplay packet writing using wiki.vg.

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@aramperes aramperes left a comment

Choose a reason for hiding this comment

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

The TODO here wasn't just to name the flags, but to have a way to pass them when creating the message.

I think this could be 3 new boolean fields: hasBackgroundTexture, showToast, hidden which then get encoded as byte flags.

@x4e
Copy link
Contributor Author

x4e commented Feb 7, 2021

@aramperes Thank You for clarifying, I have implemented this in 6ddaaad.

I think there are probably some issues with the achievement display writing (specifically the icon slot field doesn't seem to be written correctly), but I will leave these for now since you said the dev branch is not really active.

@x4e
Copy link
Contributor Author

x4e commented Feb 8, 2021

Checkstyle does not like my empty description for the hidden parameter - unfortunately I do not know the purpose of this parameter due to a lack of documentation on wiki.vg.

@aramperes
Copy link
Member

From Minecraft Wiki:

hidden: Can be true or false. Whether or not to hide this advancement and all its children from the advancement screen until this advancement have been completed. Has no effect on root advancements themselves, but still affects all their children. Defaults to false.

@x4e
Copy link
Contributor Author

x4e commented Feb 8, 2021

@aramperes Thanks! Implemented in 5ec1a5a

@aramperes aramperes merged commit 1624f60 into GlowstoneMC:dev Feb 11, 2021
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.

3 participants