Skip to content

Fix curves in extended negative world heights - #3608

Open
twme-ai wants to merge 4 commits into
IntellectualSites:mainfrom
twme-ai:fix/issue-3606-negative-y-curve
Open

Fix curves in extended negative world heights#3608
twme-ai wants to merge 4 commits into
IntellectualSites:mainfrom
twme-ai:fix/issue-3606-negative-y-curve

Conversation

@twme-ai

@twme-ai twme-ai commented Aug 1, 2026

Copy link
Copy Markdown

Overview

Fixes #3606

Description

LocalBlockVectorSet.wrapped() is designed to upgrade to BlockVectorSet when a position falls outside its local coordinate range. However, its add(BlockVector3) overload delegated directly to the underlying set and bypassed the upgrade logic in add(int, int, int). EditSession#drawSpline uses the vector overload, causing curves below the vanilla world height to throw an IndexOutOfBoundsException.

This delegates vector additions through the upgrade-aware overload and adds regression coverage for the coordinates reported in the issue, including preservation of entries added before an upgrade.

Testing

  • ./gradlew :worldedit-core:test --tests com.fastasyncworldedit.core.math.LocalBlockVectorSetTest --rerun-tasks --no-daemon --no-configure-on-demand
  • ./gradlew :worldedit-core:build --no-daemon --no-configure-on-demand

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. No public API was added.
  • I read and followed the contribution guidelines.

@twme-ai
twme-ai requested a review from a team as a code owner August 1, 2026 15:24

@dordsor21 dordsor21 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch

@dordsor21
dordsor21 requested a review from a team August 8, 2026 11:55
@NotMyFault
NotMyFault requested a lite review from Copilot August 14, 2026 18:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes //curve failures in extended negative world heights by ensuring LocalBlockVectorSet.wrapped() correctly upgrades from LocalBlockVectorSet to BlockVectorSet when positions fall outside the local coordinate bounds, and adds regression tests around the reported coordinates.

Changes:

  • Route BlockVector3SetHolder#add(BlockVector3) through the upgrade-aware add(int,int,int) overload.
  • Add LocalBlockVectorSetTest coverage for negative-Y coordinates and preservation of entries across an upgrade.
  • Add sparsebitset to the worldedit-core test classpath to support running the new tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
worldedit-core/src/main/java/com/fastasyncworldedit/core/math/LocalBlockVectorSet.java Fixes the vector-based add path in the upgrading wrapper so it can trigger upgrade logic.
worldedit-core/src/test/java/com/fastasyncworldedit/core/math/LocalBlockVectorSetTest.java Adds regression tests for negative-Y additions and retention across upgrades.
worldedit-core/build.gradle.kts Adds a test dependency required to run the new test against code using SparseBitSet.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +39 to +51
@Test
void retainsExistingVectorsWhenUpgraded() {
BlockVector3Set set = LocalBlockVectorSet.wrapped();
BlockVector3 originalPosition = BlockVector3.at(-38, 128, -20);
BlockVector3 positionOutsideLocalRange = BlockVector3.at(-38, -357, -20);

assertTrue(set.add(originalPosition));
assertTrue(set.add(positionOutsideLocalRange));
assertTrue(set.contains(originalPosition));
assertTrue(set.contains(positionOutsideLocalRange));
}

}

@twme-ai twme-ai Aug 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in ce9cfe9. The upgradesWhenBulkAddingVectorsOutsideLocalRange test exercises addAll with both an in-range vector and the reported extended-negative-Y vector, then verifies that both entries remain in the upgraded set.

Comment on lines 577 to 580
@Override
public boolean add(BlockVector3 blockVector3) {
return set.add(blockVector3);
return add(blockVector3.x(), blockVector3.y(), blockVector3.z());
}

@twme-ai twme-ai Aug 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in ce9cfe9. BlockVector3SetHolder.addAll now iterates the collection and calls the holder add(BlockVector3) method, so out-of-range entries trigger the upgrade logic while preserving the Set.addAll return value.

@NotMyFault
NotMyFault requested a lite review from Copilot August 15, 2026 09:13

@NotMyFault NotMyFault left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@NotMyFault

Copy link
Copy Markdown
Member

Unsure why the build is failing, locally it works fine.

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.

//curve fails in extended world heights (Negative Y)

4 participants