Skip to content

Missing hut advancements added #10906

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

Open
wants to merge 1 commit into
base: version/main
Choose a base branch
from

Conversation

kezmodius
Copy link
Contributor

Changes proposed in this pull request

  • Added Advancements for Graveyard, Hospital, University (minecolonies tab)
  • Added Alchemist, Apiary, Chickens, Concreter, Cows, Dyer, Glassblower, Kitchen, Netherworker, Plantation. Rabbits, School, Sheep, Swine (production tab)
  • Added Bees/Apiary to the Build All Herders advancement.
  • Changed parent to build_graveyard for citizen_bury, citizen_resurrect and undertaker_totem
  • Added translation keys to manual_en-us.json

Testing

  • Yes I tested this before submitting it.
  • I also did a multiplayer test.

Review please

…tab)

Added Alchemist, Apiary, Chickens, Concreter, Cows, Dyer, Glassblower, Kitchen, Netherworker, Plantation. Rabbits, School, Sheep, Swine (production tab)
Added Bees/Apiary to the Build All Herders advancement.
Changed parent to build_graveyard for citizen_bury, citizen_resurrect and undertaker_totem
Added translation keys to manual_en-us.json
@Thodor12
Copy link
Contributor

"datagen" are generated sources created by a dataprovider. If you add these by hand without a data provider, the next time one runs datagen it'll wipe all of your custom files.
You either need to add them to a data provider so they will become auto generated, OR add them to the resources directly.

@@ -1358,6 +1358,32 @@
"advancements.minecolonies.build.mysticalsite.description": "A Mystical Site may be pointless. It will only make your citizens proud. But you want one, so you should research and build one!",
"advancements.minecolonies.build.mysticalsite_5.title": "I can see my house from here!",
"advancements.minecolonies.build.mysticalsite_5.description": "Upgrade your Mystical Site to level 5. Become the envy of the whole world!",
"advancements.minecolonies.build.beekeeper.title": "Buzz Buzz Buzz",
"advancements.minecolonies.build.beekeeper.description": "Build a Beekeeper for their sweet honey and sticky wax. BYOBees!",
Copy link
Contributor

Choose a reason for hiding this comment

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

Some slight inconsistencies in the building names here. A beekeeper is an Apiary, on top of that you capitalized some buildings, but no others.
"concretemixer" is the building ID, not the visual name, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did I change the wrong files then? Can you point me to the non-datagen place to put them please?
I tried to use terms that were already in the files. I saw them in there and thought I better use them to be consistent. I didn't just make them up. I can change them if you tell me which ones are wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e.g. if you ctrl-f 'beekeeper' in the manual_en_us.json you'll see why I called it that.

Copy link
Contributor

Choose a reason for hiding this comment

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

The file where you're supposed to edit it is the DefaultAdvancementsProvider, then execute datagen and you should get the same files.

Copy link
Contributor

Choose a reason for hiding this comment

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

The beekeeper is the citizen, who works at the apiary
(E.g. the same as the carpenter who works at the sawmill)

Copy link
Contributor

Choose a reason for hiding this comment

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

e.g. if you ctrl-f 'beekeeper' in the manual_en_us.json you'll see why I called it that.

The fact is that the advancements are related to a building, not a job. Any other advancement that doesn't list the building name itself is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I wasn't too fussy about the translated part of the en_us file because I assumed you were going to want to rewrite it anyway. I looked at the DefaultAdvancementsProvider and if that's all I need to change then I think I can actually manage to do that. But I will do that after I've finished the reason I wanted to do this first. Should I close this pr?

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