fix: adjust linked filter to be left-right and do not link to new forked instances #2913
Conversation
dordsor21
commented
Sep 15, 2024
dordsor21
commented
- Assume that child filters know about their own forks
- fixes Bad calculation of the number of blocks changed? #1874
|
Please take a moment and address the merge conflicts of your pull request. Thanks! |
…ked instances - Assume that child filters know about their own forks - fixes #1874
dordsor21
force-pushed
the
fix/linked-filter-adjustment
branch
from
8141051
to
bea5ef5
Compare
| return this.child; | ||
| @Override | ||
| public boolean appliesLayer(IChunk chunk, int layer) { | ||
| return getLeft().appliesLayer(chunk, layer) && getRight().appliesLayer(chunk, layer); |
There was a problem hiding this comment.
This should be ||, shouldn't it?
There was a problem hiding this comment.
I wasn't really sure what was best. Do we want to still filter in one if the other doesn't want to? That implies that edit shouldn't be happening in the layer
There was a problem hiding this comment.
My understanding was that the methods basically mean return whether a filter may do something with the chunk/layer. Now if we say "right only needs to do something if left did something", then AND should be fine, but otherwise not. There's also a theoretical scenario where right can only decide whether it needs to do something after left already did its work. We currently don't have any custom implementations for those methods (the default returns true, other impls just delegate), so maybe it would make sense to just remove them? And if we ever think that something like that might be useful, we can reintroduce it and think about it again, having an actual example at hand.
There was a problem hiding this comment.
Mm that might be the best solution for now, setting the return for applyChunk to nonnull I suppose
| this.child = child; | ||
| @Override | ||
| public <T extends IChunk> T applyChunk(T chunk, @Nullable Region region) { | ||
| chunk = getLeft().applyChunk(chunk, region); |
There was a problem hiding this comment.
chunk could be null after the call according to the docs, would it make sense to pass the original value to the right in that case?
There was a problem hiding this comment.
I suppose this is similar to above, if it's an OR then we use original of null. If the above is AND then we should probably just return null
| } | ||
|
|
||
| @Override | ||
| public boolean appliesChunk(int chunkX, int chunkZ) { |
dordsor21
force-pushed
the
fix/linked-filter-adjustment
branch
from
217d308
to
5b7c436
Compare
