atlarge-research / opencraft

Other
5 stars 2 forks source link

Fix TNT behaviour - [merged] #162

Closed jdonkervliet closed 4 years ago

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:22

Merges feature/fix-tnt-particles -> development

Explosions now do a distance check before applying entity damage and also don't cause any block damage when in water. Furthermore are TNT particles themself also fixed to look more like the original Minecraft game.

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 10:23

Commented on src/main/java/net/glowstone/Explosion.java line 51

Could this be made an integer as well? (The other radii are)

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:24

added 1 commit

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:26

Commented on src/main/java/net/glowstone/Explosion.java line 51

changed this line in version 3 of the diff

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:26

added 1 commit

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:26

Commented on src/main/java/net/glowstone/Explosion.java line 51

done

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:26

resolved all threads

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 10:31

Commented on src/main/java/net/glowstone/entity/GlowPlayer.java line 1298

Can this be removed completely?

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 10:33

Commented on src/main/java/net/glowstone/entity/GlowTntPrimed.java line 72

Can this comment be removed?

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:33

Commented on src/main/java/net/glowstone/entity/GlowPlayer.java line 1298

Yeah this logic doubles the player's velocity for no apparent reason

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:34

Commented on src/main/java/net/glowstone/entity/GlowTntPrimed.java line 72

Done!

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 10:36

resolved all threads

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 10:38

Commented on src/main/java/net/glowstone/entity/GlowPlayer.java line 1298

Can we remove the override as a whole? Or does the code above have be rewritten at some point?

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 10:39

Commented on src/main/java/net/glowstone/entity/GlowTntPrimed.java line 28

Is rand used anywhere else than the constructor? If so, could you make it private? If not, could you make it local?

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 10:45

Commented on src/main/java/net/glowstone/util/Vectors.java line 64

Could you remove the whitelines in this method? Or, add one above double length. The one inbetween double length and the if-statements could be removed.

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 10:46

Commented on src/test/java/net/glowstone/util/VectorsTest.java line 74

Missing whitelines at the start of each test method.

jdonkervliet commented 4 years ago

In GitLab by @wubero on May 28, 2020, 10:54

Commented on src/main/java/net/glowstone/Explosion.java line 161

This if statement can be replaced by the isLiquid method

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:57

Commented on src/main/java/net/glowstone/entity/GlowPlayer.java line 1298

I think it has to be rewritten at some point to support proper velocity checking voor players

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:57

Commented on src/main/java/net/glowstone/util/Vectors.java line 64

Will do!

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 10:58

Commented on src/main/java/net/glowstone/Explosion.java line 161

Ah great. I didn't know it was implemented already

jdonkervliet commented 4 years ago

In GitLab by @larsdetombe on May 28, 2020, 11:02

Commented on src/main/java/net/glowstone/Explosion.java line 303

Maybe add a TODO, so we know this still has to be fixed.

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 11:10

Commented on src/main/java/net/glowstone/entity/GlowPlayer.java line 1298

Okay!

jdonkervliet commented 4 years ago

In GitLab by @JimVliet on May 28, 2020, 11:12

Can you update your branch with development, it is very far behind.

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 11:45

added 177 commits

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 11:50

Commented on src/main/java/net/glowstone/entity/GlowTntPrimed.java line 28

It is privated and used among the codebase

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 11:50

Commented on src/main/java/net/glowstone/Explosion.java line 161

changed this line in version 5 of the diff

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 11:50

added 1 commit

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 11:53

added 1 commit

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 11:53

resolved all threads

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 12:21

Commented on src/main/java/net/glowstone/Explosion.java line 400

You are missing a space after the comma, and I would prefer it if this line were split into two separate lines (the same holds for the else-statement).

Location center = location.clone().toCenterLocation();
world.spawnParticle(Particle.EXPLOSION_HUGE, center, 1);
jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 12:23

Commented on src/main/java/net/glowstone/entity/GlowTntPrimed.java line 28

Can it be made final though?

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 12:24

Commented on src/main/java/net/glowstone/entity/GlowTntPrimed.java line 28

Also, is it guaranteed that the use cases of the rand variable occur within the same thread? If not, it should always be reacquired (since it is a different value for each thread).

jdonkervliet commented 4 years ago

In GitLab by @swabbur on May 28, 2020, 12:24

Commented on src/test/java/net/glowstone/util/VectorsTest.java line 74

Could you do it for the existing methods in this class as well, please?

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:36

Commented on src/main/java/net/glowstone/entity/GlowTntPrimed.java line 28

changed this line in version 7 of the diff

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:36

added 2 commits

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @JimVliet on May 28, 2020, 12:39

Commented on src/main/java/net/glowstone/entity/passive/GlowDonkey.java line 12

Maybe revert the health change for the donkey :O

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:39

Commented on src/main/java/net/glowstone/Explosion.java line 400

changed this line in version 8 of the diff

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:39

Commented on src/main/java/net/glowstone/entity/passive/GlowDonkey.java line 12

changed this line in version 8 of the diff

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:39

added 2 commits

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:39

resolved all threads

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:46

added 1 commit

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:51

enabled an automatic merge when the pipeline for 62c0f5ced64cc15adf0775d64367aefb08ef7be3 succeeds

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 12:51

canceled the automatic merge

jdonkervliet commented 4 years ago

In GitLab by @larsdetombe on May 28, 2020, 13:06

It seems that when TNT explodes in my world, the actual blocks that are exploded, don't drop items and are only visibly removed. When I try to place a block in an area exploded by the TNT, the original block is placed back as if the actual block was only visibly removed but not physically.

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 13:07

Commented on src/main/java/net/glowstone/Explosion.java line 303

changed this line in version 10 of the diff

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 13:07

added 1 commit

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 13:07

Fixed it. A negation was missing at the liquid check

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 13:07

resolved all threads

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 13:08

added 8 commits

Compare with previous version

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 13:18

enabled an automatic merge when the pipeline for 228f52578b5b772ff019b3c1af9fe0f2ab75bf97 succeeds

jdonkervliet commented 4 years ago

In GitLab by @julian9499 on May 28, 2020, 13:51

merged