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

Tests: Test that swapper can work out a situation where TWO items need to be swapped "together" #2434

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

Conversation

NewSoupVi
Copy link
Member

@NewSoupVi NewSoupVi commented Nov 7, 2023

Circles are locations, and the completion condition is having all 5 items. Ok, maybe "useless" wasn't the best descriptor then, Also that "Needs Blue Keys" is supposed to be for both of the locations on the right. Lol the graphics are shite ok what can I say

The situation set up in this test is this:

image

There is a valid placement, which the test validates first:

image

Then, the test calls fill_restrictive, which will first set up this placement (because of placement order):

image

From here, fill needs to swap to work out the rest.
Swap will alternate between these two placements:

image
image

And then decide there is no valid placement. Meaning swap is "broken" atm.

The problem can be boiled down to the fact that the "useless" item is in an impossible position, but nothing can swap with it:
image
image

@NewSoupVi NewSoupVi added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. is: enhancement Issues requesting new features or pull requests implementing new features. is: maintenance Regular updates to requirements and utilities that do not fix bugs or change/add features. and removed is: enhancement Issues requesting new features or pull requests implementing new features. is: maintenance Regular updates to requirements and utilities that do not fix bugs or change/add features. labels Nov 7, 2023
@ThePhar ThePhar added the affects: core Issues/PRs that touch core and may need additional validation. label Nov 7, 2023
@black-sliver
Copy link
Member

Gotta love those mspaint skills.
Can we push to your branch if/when we find a solution?

@NewSoupVi
Copy link
Member Author

Can we push to your branch if/when we find a solution?

Absolutely - Do I need to do something for that?

@black-sliver
Copy link
Member

Hm, it droesn't tell me on gh mobile, but write permission from the target is on by default, so most likely no change required on your end for that. Just wanted to ask before changing your PR - I haven't found time to look for a fix yet though

Copy link
Collaborator

@agilbert1412 agilbert1412 left a comment

Choose a reason for hiding this comment

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

Always love more unit tests!

@agilbert1412 agilbert1412 added the meta: help wanted Additional review/assistance is requested for these issues or pull requests. label Feb 12, 2024
@github-actions github-actions bot removed the affects: core Issues/PRs that touch core and may need additional validation. label Mar 8, 2024
@NewSoupVi
Copy link
Member Author

NewSoupVi commented Mar 8, 2024

I was wrong about what my own test does. It does still test this case, but in a different way than I intended. Investigating right now.

The MSPaint drawing is wrong. The state I have it place itself into, that it can't get out of, is a little different than I described.

@NewSoupVi
Copy link
Member Author

Alright, I've updated the MSPaint drawings. I actually understand now why this specific case fails, which is nice.

@Exempt-Medic Exempt-Medic added the waiting-on: other Issue/PR is waiting for something else, like another PR. label Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. meta: help wanted Additional review/assistance is requested for these issues or pull requests. waiting-on: other Issue/PR is waiting for something else, like another PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants