Skip to content

Conversation

@SirYwell
Copy link
Member

Overview

Description

For some region implementations, precise contains(x, z) implementations would require heavy computation, far more heavy than just also passing the Y coordinate. FuzzyRegion is a good example for that.
For such regions, //walls was broken before. This simple change addresses this problem.

Submitter Checklist

  • Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
  • Ensure that the pull request title represents the desired changelog entry.
  • New public fields and methods are annotated with @since TODO.
  • I read and followed the contribution guidelines.

@SirYwell SirYwell requested a review from a team as a code owner October 24, 2025 08:33
@github-actions github-actions bot added the Bugfix This PR fixes a bug label Oct 24, 2025
@dordsor21
Copy link
Member

Should the fix rather be to overload the contains(x,y,z) or to add them to the relevant regions?

@SirYwell
Copy link
Member Author

Overriding contains(x, z) should probably be done too, but especially for FuzzyRegion there is no fast way to check whether any y coordinate is included, you'd have to go from minY to maxY potentially. For some other regions it's similarly complex. Generally I think the contains(x, z) method does not provide much benefit, maybe we should just deprecate it.

@dordsor21 dordsor21 requested a review from a team October 25, 2025 08:57
@NotMyFault NotMyFault merged commit e2af5b4 into main Oct 26, 2025
9 checks passed
@NotMyFault NotMyFault deleted the fix/walls-non-cuboid branch October 26, 2025 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugfix This PR fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants