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

Add a better error message for the incompatibility between Scarce/Minimal and high heart win conditions #2182

Open
wants to merge 2 commits into
base: Dev
Choose a base branch
from

Conversation

fenhl
Copy link
Collaborator

@fenhl fenhl commented Feb 16, 2024

Fixes #1644. The fix chosen here is to add an explicit check for the incompatible settings to the world constructor and inform the user how to fix it. My opinion is that allowing the user to choose which fix to apply (changing item pool vs changing wincons) is better than silently changing the number of hearts in the item pool, which the user may not expect, especially if we end up separating the heart pool from the remaining item pool, which is a common suggestion and probably a good idea. Besides, even if we want to change the behavior later, this is an easy fix that should be included until then.

@fenhl fenhl added Type: Bug Something isn't working Status: Needs Review Someone should be looking at it Component: Setting specific to setting(s) Status: Needs Testing Probably should be tested labels Feb 16, 2024
@fenhl fenhl removed the Status: Needs Testing Probably should be tested label Mar 23, 2024
@cjohnson57
Copy link
Collaborator

I'm not sure I agree with this. If the user wants to use heart conditions on a scarcer item pool, they should be able to do so.

You could argue that it's fair to require them to use a plando for this, but the code doesn't seem to check the item pool itself (which plando would affect), just assuming how many hearts will be available based on the item pool setting.

@fenhl fenhl added the Status: Waiting for Author Changes or response requested label Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Setting specific to setting(s) Status: Needs Review Someone should be looking at it Status: Waiting for Author Changes or response requested Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Heart win conditions incompatible with Scarce and Minimal item pools
2 participants