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 structure clear removing instead of restoring blocks #1107

Merged
merged 2 commits into from
Nov 28, 2022
Merged

Conversation

Pablete1234
Copy link
Member

@Pablete1234 Pablete1234 commented Nov 28, 2022

In current 1.8 PGM, structures were recently re-implemented, but have a significant behavior change in respect to legacy PGM.

In 1.11 PGM, clearing a dynamic structure did not mean filling it with air, instead, it meant restoring the original blocks before the structure was placed by the dynamic. See https://github.com/OvercastNetwork/Plugins/blob/master/PGM/src/main/java/tc/oc/pgm/structure/DynamicDefinition.java#L118, it clears by placing a clearImage, the clear image was made when the Dynamic was initialized.

This poses a subtle required change in paradigm, as at the moment we only store one world snapshot, intended to store all the very-original blocks. However, because structures can clear their own blocks so they're air by default, a dynamic's removal has to place blocks from a different world snapshot (one after structures have cleared). In legacy PGM this was a non-issue as it worked with immutable BlockImages, in here, we work with ChunkSnapshots.

The change makes it so some of the logic previously in SnapshotMatchModule is moved over to a new WorldSnapshot abstraction, which holds a view of the world at a certain time, the SnapshotMatchModule works with one "default" world snapshot used for most operations, and then, DynamicStructures will use a different one, created after clears have occurred.

As an additional optimization, if no structures clear themselves, then it will use the same world snapshot and won't create an extra one.

Lastly, the Structures now create a FiniteBlockRegion if they avoid air, which is a region that includes all non-air-blocks, that way places-and-removes no longer need to check for original block, if it's in the region then it should replace. This was needed because a dynamic's removal depends on if the original structure includes the block, not on if it's air in its current world snapshot (the one after structures may have potentially cleared).

This was brought up by @CrazyisCreeps on discord, and i have not yet ran tests on this PGM build, so i'll leave it as draft until we have some confirmation about if this fixes the issue or not.

Edit: it has now been tested and validated.

Signed-off-by: Pablete1234 <pabloherrerapalacio@gmail.com>
@Pablete1234 Pablete1234 added bug Something isn't working reintroduce Reintroduces a feature that was removed labels Nov 28, 2022
Signed-off-by: Pablete1234 <pabloherrerapalacio@gmail.com>
@Pablete1234 Pablete1234 marked this pull request as ready for review November 28, 2022 08:54
@Electroid Electroid merged commit 5103d4a into dev Nov 28, 2022
@Electroid Electroid deleted the structure-fix branch November 28, 2022 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working reintroduce Reintroduces a feature that was removed
Development

Successfully merging this pull request may close these issues.

2 participants