Improve nether portals - [merged] #168

Closed jdonkervliet closed 4 years ago

In GitLab by @larsdetombe on Jun 3, 2020, 17:10

Merges feature/improve-nether -> development

Portals of various sizes can be created. When breaking a portal or portal block, all connected portal blocks are also deleted.

In GitLab by @JimVliet on Jun 3, 2020, 17:24

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 23

Is this the maximum portal size in minecraft?

In GitLab by @swabbur on Jun 3, 2020, 17:28

Commented on src/main/java/net/glowstone/block/blocktype/BlockPortal.java line 24

Missing whiteline at the start of the method.

In GitLab by @swabbur on Jun 3, 2020, 17:28

Commented on src/main/java/net/glowstone/block/blocktype/BlockPortal.java line 20

Whether may read better/nicer than If in this case.

In GitLab by @larsdetombe on Jun 3, 2020, 17:32

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 23

The size refers to the inner portal size (without the obsidian blocks). This is not the portal size in Minecraft, the size in Minecraft is 21, I will change this. I think INNER_PORTAL_SIZE is also a better name, so I will change the name as well.

In GitLab by @swabbur on Jun 3, 2020, 17:33

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 63

You could use an || operator achieve that you're doing here. If the first one succeeds, the second won't be called.

createValidPortal(target, BlockFace.NORTH, player) 
|| createValidPortal(target, BlockFace.EAST, player);

I'm not sure whether this notation is preferred, but it's an option.

In GitLab by @swabbur on Jun 3, 2020, 17:35

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 171

It might be neater to store the return value of this call in a separate variable. Or, you could store the return value of callevent.

Event event = new EntityCreatePortalEvent(player, blocks, PortalType.NETHER);
??? = EventFactory.getInstance().callEvent(event);
if (!???.isCancelled()) {
    // ...
}

In GitLab by @swabbur on Jun 3, 2020, 17:37

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 187

Missing whiteline at the beginning of the method.

In GitLab by @swabbur on Jun 3, 2020, 17:38

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 212

Missing whiteline at the beginning of the method.

In GitLab by @swabbur on Jun 3, 2020, 17:44

Creating and moving through portals worked perfectly. However, when I entered the Nether and starting swinging my arm, I spawned into some blocks, the following exception was thrown:

17:40:46 [SEVERE] Error while handling PlayerSwingArmMessage(hand=0) (handler: PlayerSwingArmHandler)
java.lang.IllegalStateException: Start block missed in BlockIterator
    at org.bukkit.util.BlockIterator.<init>(BlockIterator.java:170)
    at org.bukkit.util.BlockIterator.<init>(BlockIterator.java:233)
    at org.bukkit.util.BlockIterator.<init>(BlockIterator.java:268)
    at net.glowstone.entity.GlowLivingEntity.getLineOfSight(GlowLivingEntity.java:683)
    at net.glowstone.entity.GlowLivingEntity.getTargetBlock(GlowLivingEntity.java:722)
    at net.glowstone.net.handler.play.player.PlayerSwingArmHandler.handle(PlayerSwingArmHandler.java:23)
    at net.glowstone.net.handler.play.player.PlayerSwingArmHandler.handle(PlayerSwingArmHandler.java:16)
    at com.flowpowered.network.session.BasicSession.handleMessage(BasicSession.java:80)
    at com.flowpowered.network.session.BasicSession.messageReceived(BasicSession.java:136)
    at net.glowstone.net.GlowSession.pulse(GlowSession.java:420)
    at java.util.concurrent.ConcurrentHashMap$KeySetView.forEach(ConcurrentHashMap.java:4649)
    at net.glowstone.net.SessionRegistry.pulse(SessionRegistry.java:23)
    at net.glowstone.scheduler.GlowScheduler.pulse(GlowScheduler.java:227)
    at net.glowstone.scheduler.GlowScheduler.lambda$start$0(GlowScheduler.java:136)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)

In GitLab by @larsdetombe on Jun 3, 2020, 20:43

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 63

That is indeed a solution, however I think the way it is currently done is a bit better to understand. This way the intent is better understood I think.

In GitLab by @larsdetombe on Jun 3, 2020, 21:01

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 23

changed this line in version 2 of the diff

In GitLab by @larsdetombe on Jun 3, 2020, 21:01

Commented on src/main/java/net/glowstone/block/itemtype/ItemFlintAndSteel.java line 171

changed this line in version 2 of the diff

In GitLab by @larsdetombe on Jun 3, 2020, 21:01

added 1 commit