Discussion on explosion behavior change (1/2): Refactor to be ray-based instead of block bounds-based#925
Open
EZForever wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is not a plain bug fix; it's rather meant as a discussion starter on changing explosion behavior. I know I probably should discuss this with you guys on Discord first, but I don't have access to Discord, so... I hope this will do.
Minecraft's own explosion logic, according to Minecraft Wiki and the corresponding source code (
net.minecraft.world.level.Explosion#explode()), is based on "rays". When an explosion happens, multiple rays are created with varying "intensity", and travel outward from the center of the explosion, going 0.3 blocks on each step. For each step, the block at the ray's current position is calculated to be destroyed or not, and the ray's intensity is correspondingly reduced.If I understand correctly, current explosion handling logic in Sable is, for lack of a better word, "block bounds-based". For contraptions, the "the block at the ray's current position" part of the logic above is replaced by "the grid-aligned 1x1x1 area in global coords at the ray's current position mapped into sublevel coords", and all the blocks in this area are taken into consideration, as if they are a single block in global coords at the ray's position. This of course works, and I would assume this is intentional to some extent (for handling blast forces of multiple blocks, I guess?), but due to the way this logic is implemented (and bugs in it), it very commonly creates cases where the 1x1x1 area in global coords mentioned above gets mapped into a larger (2x2x2, or in an extreme case, 3x3x3) area in sublevel-local coords. Since all the blocks in this area are considered in one go, it is as if the ray is hitting all these blocks at once, reducing the ray's intensity too much. The final consequence of all of this is that a block in a contraption would be much tougher to explosions than a normal one, if it is surrounded by multiple other blocks.
In-game testing proves this; here we have two 16x16x16 glass cubes, one is just normal blocks, and the other (with red outline) is assembled as a contraption. A TNT is detonated in the center of both cubes. In the normal cube, this explosion creates a crater about 8 blocks in diameter, while in the contraption cube, the crater is only 4 blocks wide. The contraption here is not moved after being assembled (i.e. still global grid aligned); I believe with some tinkering on movement and rotation, the crater could be even smaller.
You can use the following commands to recreate the test. Stand still during the whole test.
/fill ~1 ~ ~1 ~16 ~ ~16 minecraft:obsidian/fill ~1 ~1 ~1 ~16 ~16 ~16 minecraft:glass/sable assemble area ~1 ~1 ~1 ~16 ~16 ~16/summon minecraft:tnt ~8 ~8 ~8This PR resolves the problem above by adapting Minecraft's original "ray-based" handling logic: the ray's position itself, not it's block position, gets mapped into sublevel-local coords, and only the block at that mapped position gets considered. It eliminates the need to do any block bounds calculation, so no rounding or off-by-one errors, and preliminary testing shows this approach is seemingly not breaking any stuff. I cannot say it's foolproof, though; it's a behavior change, and my testing absloutely didn't cover anything beyond the expected change. Maybe some contraptions made by some other people that rely on this explosion behavior will break; I'm not sure.
Here the same test is done on main branch (left) and this PR (right); with this PR's patch, TNT returns to it's former glory with the crater exactly like the one not in a contraption.
If this PR is deemed not acceptable, then maybe take a look at #926; it's a simple off-by-one bug fix for the original logic that partially resolves the problem.