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

Fix node placement #8277

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Fix node placement #8277

merged 1 commit into from
Nov 14, 2023

Conversation

farmaazon
Copy link
Contributor

@farmaazon farmaazon commented Nov 10, 2023

Pull Request Description

Fixes: #8230

Fixes some rare cases caught by property tests, involving nodes with 0 width. MultiRange does not handle those. The thing is fixed by not using MultiRange, but by sorting nodes by the axis along which we're searching for free place.

All three reported seeds seem to be fixed.

Important Notes

MultiRange is not used now, but I decided to spare it, violating YAGNI rule, as it is tested, and I feel it's a structure which may be useful.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • [ ] Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • [ ] If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@farmaazon farmaazon added CI: No changelog needed Do not require a changelog entry for this PR. -gui labels Nov 10, 2023
@farmaazon farmaazon self-assigned this Nov 10, 2023
@farmaazon farmaazon requested a review from Frizi as a code owner November 10, 2023 11:04
@farmaazon farmaazon added CI: Ready to merge This PR is eligible for automatic merge CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Nov 13, 2023
@mergify mergify bot merged commit 5baec46 into develop Nov 14, 2023
25 of 26 checks passed
@mergify mergify bot deleted the wip/farmaazon/fix-placement branch November 14, 2023 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing random tests
2 participants