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

#407 Add Achievement Tracking Option #425

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Ioney
Copy link
Collaborator

@Ioney Ioney commented Aug 3, 2024

No description provided.

@Ioney Ioney marked this pull request as ready for review August 26, 2024 19:59
@zarillion
Copy link
Owner

I took a look at this branch, and the behavior of some of the groups are indeed odd once this option is deselected. I see why you added the "Achievement Reward Tracking is off, therefore most Nodes will be hidden!" text to the top of the achievements section if those rewards are hidden. It causes most or all of the achievement nodes to just stop working entirely.

I think we should implement this as a rares-only toggle, since the display state of rares are never tied to achievement criteria. Instead of needing to display special text in the achievement menu section when this option is unchecked, we can just add extra text to the option saying it only affects rares. That covers the original ask here anyways.

I'll have to think more about where that option should live in the menus and what exactly it should say. The filtering will be more complex than the existing reward toggle logic since it needs to live in the Rare class instead of the base Node class.

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.

4 participants