ClanGenOfficial / clangen

Warrior Cats fan game
https://clangen.io
Other
269 stars 483 forks source link

Assigning status via __init__ has unexpected effects vs assigning later #2455

Open j-gynn opened 5 months ago

j-gynn commented 5 months ago

Type: (only select one)

Describe the bug Good luck to thee that takes this on. I have first noticed it in freshkill_test, so it may be a freshkill quirk, but it may also be a Cat problem. Basically, sort order becomes arbitrary when an otherwise-blank Cat is initialised with a status, and I have no idea why.

Grade: (only select one)

Reproduce Steps to reproduce the behavior:

  1. Go to the freshkill tests
  2. move status="warrior" into Cat()`
  3. Watch tests fail 1/3 of the time
  4. (optional) cry

Commit # or Game Version Number: Development

Additional context Add any other context about the problem here.

Screenshots image image

larkgz commented 5 months ago

/assign-me

github-actions[bot] commented 5 months ago

πŸ‘‹ Hey @larkgz, thanks for your interest in this issue! πŸŽ‰

⚠ Note that this issue will become unassigned if it isn't closed within 7 days.

πŸ”§ A maintainer can also add the πŸ“Œ Pinned label to prevent it from being unassigned automatically.

larkgz commented 5 months ago

The default status for cats in __init__ is newborn. Therefore the cats have the age of a newborn in the original test. Newborns are always fed first according to feeding order. If you set the moons the same (specifying just status='warrior' gives you a random age between young adult and senior adult which are fed in a different order because they're auto-sorted by moons), then the tests pass again.

j-gynn commented 5 months ago

Ah, so it's the assignation of moons that causes the problem... I should probably have clocked that. Still, it's not an expected outcome for status and moons to be linked together in init to the end user. I've specified status, not age - and definitely not moons. It should probably be changed imo, but I'm no senior dev haha.

larkgz commented 5 months ago

Unassigning myself as it seems to require a larger change and I don’t think it’s a bug. Could maybe resolve with #2436 in the future.