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

Return air instead of __reserved__ when reading non-cuboid clipboard #3071

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SirYwell
Copy link
Member

@SirYwell SirYwell commented Jan 9, 2025

Overview

Fixes #3065

Description

Returning the __reserved__ block state results in clipboards containing that block state, means they can't be loaded by other tools correctly and FAWE won't place air correctly.

Submitter Checklist

Preview Give feedback

@SirYwell SirYwell requested a review from a team as a code owner January 9, 2025 15:20
@github-actions github-actions bot added the Bugfix This PR fixes a bug label Jan 9, 2025
@dordsor21
Copy link
Member

Can we potentially add a flag somewhere for allowing the reserved? If we set a clipboard within FAWE it might be wanted not to set outside the original copy region, but still set air within. Presumably this is also an issue for the other FAWE implementations as well?

@SirYwell
Copy link
Member Author

Can we potentially add a flag somewhere for allowing the reserved? If we set a clipboard within FAWE it might be wanted not to set outside the original copy region, but still set air within.

This just mirrors the behavior of WE. Manually checking if the point is in the region when needed sounds more appropriate.

Presumably this is also an issue for the other FAWE implementations as well?

It seems like WorldCopyClipboard will just return the actual blocks outside of its region, so that's wrong in a different way. The memory clipboard seems to work correctly from a quick test, not sure why though.

@dordsor21
Copy link
Member

Can we potentially add a flag somewhere for allowing the reserved? If we set a clipboard within FAWE it might be wanted not to set outside the original copy region, but still set air within.

This just mirrors the behavior of WE. Manually checking if the point is in the region when needed sounds more appropriate.

We won't necessarily have the selection when pasting - it could have changed between the copy and paste, or have been made before a restart

Presumably this is also an issue for the other FAWE implementations as well?

It seems like WorldCopyClipboard will just return the actual blocks outside of its region, so that's wrong in a different way. The memory clipboard seems to work correctly from a quick test, not sure why though.

memory optimized doesn't "store" air so I believe would assume reserved is just normal air. CPU optimized is potentially problematic as it'd read a char of 0 from the array

@SirYwell
Copy link
Member Author

We won't necessarily have the selection when pasting - it could have changed between the copy and paste, or have been made before a restart

Hm, that's an interesting scenario. I guess SimpleRegion always returning a CuboidRegion is wrong then in first place, and we need to somehow improve on that?

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.

The construction diagram highlighted using //sel cyl is inserted without air blocks
2 participants