Skip to content

fix: correct lazycopy dimensions and world usage - #3520

Open
dordsor21 wants to merge 4 commits into
mainfrom
fix/lazy-copy
Open

fix: correct lazycopy dimensions and world usage#3520
dordsor21 wants to merge 4 commits into
mainfrom
fix/lazy-copy

Conversation

@dordsor21

Copy link
Copy Markdown
Member

@dordsor21
dordsor21 requested a review from a team as a code owner May 10, 2026 15:59

@Override
public long getMaxSize() {
return -1;

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.

Why not just use Long.MAX_VALUE? The specification of the method in the superclass should also cover which values are valid.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I wanted to convey actually unlimited rather than functionally unlimited. It should go into the javadoc though yeah

*
* @return maximum size in blocks of this clipboard implementation.
*/
public long getMaxSize() {

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.

An instance method is somehow the wrong approach, mainly because it requires an escaping this in the constructor, and it also doesn't have any different use really. Maybe SimpleClipboard is generally a rather problematic abstraction?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is, but altering the hierarchy is rather a v3 change and it would be nice to have this working a little sooner than that... We could always deprecate for removal upon implementation and put the change into V3 immediately.

Alternatively I suppose we could just reimplement in a different class entirely? Certainly the clipboard hierarchy could do with some cleanup at some point regardless

@dordsor21
dordsor21 requested a review from SirYwell May 15, 2026 15:52
@dordsor21
dordsor21 requested a review from a team July 26, 2026 11:12
@dordsor21
dordsor21 requested a review from a team July 26, 2026 17:15
@NotMyFault
NotMyFault requested a lite review from Copilot August 15, 2026 09:19

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 addresses two //lazycopy regressions by allowing WorldCopyClipboard to represent extremely large selections without hitting the SimpleClipboard size guard, and by reducing the chance that the clipboard’s backing extent is reused/reinitialized in a way that changes the source world.

Changes:

  • Add configurable “max volume” constructors to SimpleClipboard/ReadOnlyClipboard and use an “infinite” limit for WorldCopyClipboard.
  • Update WorldCopyClipboard to construct its base clipboard with an unlimited max size and uncache the queue when a SingleThreadQueueExtent is detected.
  • Refactor deprecated WorldCopyClipboard constructors to delegate through the new constructor path.

Reviewed changes

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

File Description
worldedit-core/src/main/java/com/fastasyncworldedit/core/extent/clipboard/WorldCopyClipboard.java Uses unlimited clipboard sizing for lazy copy and uncaches queue when a reusable single-thread queue extent is present.
worldedit-core/src/main/java/com/fastasyncworldedit/core/extent/clipboard/SimpleClipboard.java Introduces max-size-aware constructors and changes the size guard to use that max.
worldedit-core/src/main/java/com/fastasyncworldedit/core/extent/clipboard/ReadOnlyClipboard.java Exposes max-size-aware construction for read-only clipboards and defaults to the prior size limit behavior.

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

Comment on lines +17 to +18
* New {@link SimpleClipboard} instance for the given region with max volume of {@link Integer#MAX_VALUE}. Initial offset
* is the clipboard minimum point.
Comment on lines 62 to 64
this.area = getWidth() * getLength();
this.volume = (int) longVolume;
this.origin = BlockVector3.ZERO;
Comment on lines +27 to +28
* New {@link ReadOnlyClipboard} instance for the given region with max volume of {@link Integer#MAX_VALUE}. Initial offset
* is the clipboard minimum point.
@NotMyFault NotMyFault added the Bugfix This PR fixes a bug label Aug 15, 2026
@NotMyFault
NotMyFault requested a lite review from Copilot August 15, 2026 11:22

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.

Suppressed comments (1)

worldedit-core/src/main/java/com/fastasyncworldedit/core/extent/clipboard/SimpleClipboard.java:63

  • maxSize == -1 currently allows volumes larger than Integer.MAX_VALUE, but area and volume are still stored as int (this.area = getWidth() * getLength(); and this.volume = (int) longVolume;). For very large regions this will overflow and can turn negative; additionally the long multiplication can overflow for large-but-valid Minecraft coordinate ranges (e.g. ~60M x 4096 x ~60M). Consider using Math.multiplyExact to detect overflow and Ints.saturatedCast (or equivalent) so getArea()/getVolume() never return nonsensical negative values when large lazy clipboards are permitted.
        if (maxSize != -1 && longVolume > maxSize) {
            throw new IllegalArgumentException("Dimensions are too large for this clipboard format.");
        }
        this.area = getWidth() * getLength();
        this.volume = (int) longVolume;

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.

LAZYCOPY will not copy clipboard source world //lazycopy dimensiona too large

5 participants