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

Minecraft: Set "Two by Two" and "A Complete Catalogue" to "unreasonable" exclusion group #3977

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ArchooD2
Copy link

Please format your title with what portion of the project this pull request is
targeting and what it's changing.

ex. "MyGame4: implement new game" or "Docs: add new guide for customizing MyGame3"

What is this fixing or adding?

This changes the advancement groups of:
"Two by Two" and "A Complete Catalogue"
from "hard" to "unreasonable" exclusion group

How was this tested?

Running a 63 player all-minecraft with a yaml that enables 'hard' but disables 'unreasonable', and verified that they were all junk items.

If this makes graphical changes, please attach screenshots.

N/A

Set "Two by Two" and "A Complete Catalogue" to "unreasonable" exclusion group
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Sep 21, 2024
@ArchooD2
Copy link
Author

Reason: Played this in a competitive game, and Two by Two was BK-ing a peer on my team. Couldn't goal my game.

@NewSoupVi NewSoupVi added is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world. labels Sep 21, 2024
@NewSoupVi
Copy link
Member

@KonoTyran

Copy link
Contributor

@palex00 palex00 left a comment

Choose a reason for hiding this comment

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

This generates fine and the wished for changes are being considered. The locations are now part of the unreasonable subset.

I also think this change is a good one.

However, the YAML also needs to be changed as "include_unreasonable_advancements" specifies all included achievements.

I PR'd a fix: ArchooD2#1

palex00 and others added 2 commits September 22, 2024 06:26
Adds the newly classified unreasonable achievements to the YAML
Copy link
Contributor

@palex00 palex00 left a comment

Choose a reason for hiding this comment

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

LGTM now. Both code and docs wise.

@Seatori
Copy link
Contributor

Seatori commented Nov 11, 2024

Why exactly is 'A Complete Catalogue' being classified as "Unreasonable" here? There's no explanation given in the PR message or comments.
In any case, I personally disagree with that classification. It's not nearly as difficult as the other "Unreasonable" Advancements, and I would argue that it's much easier than other "Hard" Advancements too.

I also disagree with 'Two by Two' being classified as "Unreasonable" - I don't think it's as difficult as 'Monsters Hunted', for example - but at the moment its client integration has an issue that can make it impossible in some seeds, so this seems fine to me as a band-aid fix. I don't believe that it should remain that way once the client is fixed, though.

@KonoTyran
Copy link
Contributor

I'm also against these changes. This sounds like more of a use for the generic feature of excluded locations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. waiting-on: world-maintainer Issue/PR is waiting for feedback or approval by the maintainer of a world.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants