mcl_explosions: Adjust explosion entity damage hitbox #62

ryvnf · 2021-05-09T14:46:39+02:00

ryvnf commented

2021-05-09 14:46:39 +02:00

In mcl_explosions the hitbox used for calculating the damage of an entity is its collisionbox multiplied by two. This commit removes the multiplication by two because that makes explosion damage behave weirdly in some circumstances. It was most likely implemented that way because of a misinterpretation of the Minecraft wiki.

This closes issue #48. This PR can be tested by following the steps described there.

In mcl_explosions the hitbox used for calculating the damage of an entity is its collisionbox multiplied by two. This commit removes the multiplication by two because that makes explosion damage behave weirdly in some circumstances. It was most likely implemented that way because of a misinterpretation of the Minecraft wiki. This closes issue #48. This PR can be tested by following the steps described there.

ryvnf added the

bug

label 2021-05-09 14:46:39 +02:00

ryvnf added 1 commit 2021-05-09 14:46:40 +02:00

d9bbf4879c Adjust explosion entity damage hitbox

In mcl_explosions the hitbox used for calculating the damage of an
entity is its collisionbox multiplied by two.  This commit removes the
multiplication by two because that makes explosion damage behave weirdly
in some circumstances.  It was most likely implemented that way because
of a misinterpretation of the Minecraft wiki.

ryvnf changed title from ~~Adjust explosion entity damage hitbox~~ to mcl_explosions: Adjust explosion entity damage hitbox

2021-05-16 11:17:21 +02:00

ryvnf referenced this pull request

2021-05-16 14:59:21 +02:00

First release discussion #69

ryvnf added this to the First Release milestone 2021-05-16 22:50:04 +02:00

cora approved these changes 2021-05-17 21:17:09 +02:00

cora left a comment

I have tested this before and after in both creative and survival exactly like the image suggests.

Before the change each tnt does 5 (2.5 hearts) damage

After the changes it's 1 (1/2 heart) per tnt explosion.

I have tested this before and after in both creative and survival exactly like the image suggests. Before the change each tnt does 5 (2.5 hearts) damage After the changes it's 1 (1/2 heart) per tnt explosion.

ryvnf referenced this issue from a commit

2021-05-18 18:17:01 +02:00

Merge pull request 'mcl_explosions: Adjust explosion entity damage hitbox' (#62) from mcl_explosions2 into master

erlehmann commented

2021-05-18 21:44:15 +02:00

@ryvnf how can this conflict while being merged? Shouldn't it be closed by now?

ryvnf commented

2021-05-19 10:35:42 +02:00

@ryvnf how can this conflict while being merged? Shouldn't it be closed by now?

This probably related to the rewind of the master branch. Initially it said the branch could not be merged because the fork had "incomplete information". I tried syncing the branch but it did not solve the issue. I tried using the listed command-line instructions to merge it locally. It merged fine and the diffs were identical to the PR.

I pushed the merge and and expected it to display "Manually Merged" like this PR from MineClone2 MineClone2/MineClone2#1285 but it appears Gitea were still confused. There are no conflicts as doing git merge mcl_explosions2 says Already up to date.

I will try and see if I get the PR to display as merged. Maybe syncing it with the master branch again will solve it.

> @ryvnf how can this conflict while being merged? Shouldn't it be closed by now? This probably related to the rewind of the master branch. Initially it said the branch could not be merged because the fork had "incomplete information". I tried syncing the branch but it did not solve the issue. I tried using the listed command-line instructions to merge it locally. It merged fine and the diffs were identical to the PR. I pushed the merge and and expected it to display "Manually Merged" like this PR from MineClone2 https://git.minetest.land/MineClone2/MineClone2/pulls/1285 but it appears Gitea were still confused. There are no conflicts as doing `git merge mcl_explosions2` says `Already up to date`. I will try and see if I get the PR to display as merged. Maybe syncing it with the master branch again will solve it.

ryvnf commented

2021-05-19 10:49:30 +02:00

After syncing the branch with the master branch again it says the branches are equal. This is worse because now one can no longer see the changes done in the PR.

ryvnf force-pushed mcl_explosions2 from 9d031db213 to 4c61fc80c6

2021-05-19 10:51:01 +02:00

Compare

ryvnf force-pushed mcl_explosions2 from a9f47709aa to 4c61fc80c6

2021-05-19 10:58:45 +02:00

Compare

ryvnf commented

2021-05-19 11:01:32 +02:00

Sorry. I just made things worse by attempting to fix it. Because now the changes @cora reviewed are no longer displayed in the Gitea. The PR was merged in f3b0285347. It had the following changes:

diff --git a/mods/CORE/mcl_explosions/init.lua b/mods/CORE/mcl_explosions/init.lua
index 79221d0e..82a3c07c 100644
--- a/mods/CORE/mcl_explosions/init.lua
+++ b/mods/CORE/mcl_explosions/init.lua
@@ -253,12 +253,12 @@ local function trace_explode(pos, strength, raydirs, radius, info, puncher)
 
                        if collisionbox then
                                -- Create rays from random points in the collision box
-                               local x1 = collisionbox[1] * 2
-                               local y1 = collisionbox[2] * 2
-                               local z1 = collisionbox[3] * 2
-                               local x2 = collisionbox[4] * 2
-                               local y2 = collisionbox[5] * 2
-                               local z2 = collisionbox[6] * 2
+                               local x1 = collisionbox[1]
+                               local y1 = collisionbox[2]
+                               local z1 = collisionbox[3]
+                               local x2 = collisionbox[4]
+                               local y2 = collisionbox[5]
+                               local z2 = collisionbox[6]
                                local x_len = math.abs(x2 - x1)
                                local y_len = math.abs(y2 - y1)
                                local z_len = math.abs(z2 - z1)

Sorry. I just made things worse by attempting to fix it. Because now the changes @cora reviewed are no longer displayed in the Gitea. The PR was merged in f3b0285347. It had the following changes: ```diff diff --git a/mods/CORE/mcl_explosions/init.lua b/mods/CORE/mcl_explosions/init.lua index 79221d0e..82a3c07c 100644 --- a/mods/CORE/mcl_explosions/init.lua +++ b/mods/CORE/mcl_explosions/init.lua @@ -253,12 +253,12 @@ local function trace_explode(pos, strength, raydirs, radius, info, puncher) if collisionbox then -- Create rays from random points in the collision box - local x1 = collisionbox[1] * 2 - local y1 = collisionbox[2] * 2 - local z1 = collisionbox[3] * 2 - local x2 = collisionbox[4] * 2 - local y2 = collisionbox[5] * 2 - local z2 = collisionbox[6] * 2 + local x1 = collisionbox[1] + local y1 = collisionbox[2] + local z1 = collisionbox[3] + local x2 = collisionbox[4] + local y2 = collisionbox[5] + local z2 = collisionbox[6] local x_len = math.abs(x2 - x1) local y_len = math.abs(y2 - y1) local z_len = math.abs(z2 - z1) ```

ryvnf added 1 commit 2021-05-19 11:21:51 +02:00

d9bbf4879c Adjust explosion entity damage hitbox

In mcl_explosions the hitbox used for calculating the damage of an
entity is its collisionbox multiplied by two.  This commit removes the
multiplication by two because that makes explosion damage behave weirdly
in some circumstances.  It was most likely implemented that way because
of a misinterpretation of the Minecraft wiki.

ryvnf force-pushed mcl_explosions2 from c3b516e5ef to d9bbf4879c

2021-05-19 11:34:55 +02:00

Compare

ryvnf changed target branch from master to fake_master

2021-05-19 11:35:14 +02:00

ryvnf merged commit 1320b27600 into fake_master

2021-05-19 11:35:35 +02:00

ryvnf referenced this issue from a commit

2021-05-19 11:35:36 +02:00

Merge pull request 'mcl_explosions: Adjust explosion entity damage hitbox' (#62) from mcl_explosions2 into fake_master

ryvnf commented

2021-05-19 11:41:10 +02:00

I managed to get Gitea to change the status of the PR to "Merged" by setting up a fake master branch and merging into that. I think this is better than leaving it closed because now you can actually see the changes which the PR had in the Gitea UI. Note that the message

The pull request has been merged as 1320b27600

is incorrect and should say

The pull request has been merged as f3b0285347

and that the target branch was master and not fake_master.

I managed to get Gitea to change the status of the PR to "Merged" by setting up a fake master branch and merging into that. I think this is better than leaving it closed because now you can actually see the changes which the PR had in the Gitea UI. Note that the message > The pull request has been merged as 1320b27600 is incorrect and should say > The pull request has been merged as f3b0285347 and that the target branch was `master` and not `fake_master`.

ryvnf referenced this pull request

2021-05-19 13:06:56 +02:00

Entity damage from explosions is ray-casted from incorrect position #48

This repo is archived. You cannot comment on pull requests.

No reviewers

No Milestone

No project

No Assignees

3 Participants

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Mineclonia/Mineclonia#62