Add animations of block groups falling #9
Conversation
|
The animation still doesn't look quite as good in multiplayer, but there is a prediction system now so it's at least somewhat smooth. |
There was a problem hiding this comment.
IMO the whole FallWithAnimationSystem needs more documentation and breaking down into smaller methods. Especially onBlockGroupDetached is very long and I don't think I properly understand what's happening there as it's hard to read, but I feel like it's doing multiple things that could probably be refactored into methods. update and land are not that big but still could use at least some documentation on what's supposed to be happening there in case you don't want to split them up a bit.
| @Replicate | ||
| public Map<Vector3ic, Block> blocks; | ||
|
|
||
| // The components of the block entities, removed from the entities themselves do the EntityAwareWorldProvider doesn't mess with them |
There was a problem hiding this comment.
| // The components of the block entities, removed from the entities themselves do the EntityAwareWorldProvider doesn't mess with them | |
| // The components of the block entities, removed from the entities themselves so the EntityAwareWorldProvider doesn't mess with them |
Might want to link to EntityAwareWorldProvider here
| @Owns | ||
| public Map<Vector3ic, Set<Component>> components; | ||
|
|
||
| // The block region entities, with their BlockRegionComponents removed to deactivate them temporarily |
There was a problem hiding this comment.
Might want to link to BlockRegionComponent here
| // The lower edge of the group, the only part of it that can collide with blocks | ||
| public Set<Vector3ic> frontier; | ||
|
|
||
| // 0 if the block group is still falling, otherwise, the number of ticks since it landed. |
There was a problem hiding this comment.
what does "the number of ticks since it landed" to do with "dying"? 🤔
| */ | ||
| @ReceiveEvent | ||
| public void blockUpdate(OnChangedBlock event, EntityRef blockEntity) { | ||
| boolean oldSolid = TreeUtils.isSolid(event.getOldType()); |
There was a problem hiding this comment.
Why is this using something from TreeUtils 🤔 Should isSolid maybe be somewhere more central and less tree-specific?
| // TODO: Maybe make this a WorldChangeListener instead? Compare efficiency. Aggregate changes into batches (should improve efficiency and avoid confusing reentrant behaviour when falling blocks cause block updates). | ||
| /** | ||
| * Called every time a block is changed. | ||
| * This means that the type of the block has changed. |
There was a problem hiding this comment.
The "type" of the block as in "dirt" vs "stone"?
| totalMass += block.getMass(); | ||
| Optional<Prefab> blockPrefab = block.getPrefab(); | ||
| if (blockPrefab.isPresent()) { // Can't use ifPresent because the local variable totalLevitation is accessed. | ||
| logger.info("Has prefab."); |
There was a problem hiding this comment.
This seems like a printf debugging comment. Either remove it or at least reduce it to debug level.
| } | ||
|
|
||
| private void blockGroupDetached(Set<Vector3i> positions) { | ||
| logger.info("Block group falling."); |
There was a problem hiding this comment.
This seems like a printf debugging comment. Either remove it or at least reduce it to debug level.
| logger.info("Has prefab."); | ||
| LevitatingBlockComponent levitation = blockPrefab.get().getComponent(LevitatingBlockComponent.class); | ||
| if (levitation != null) { | ||
| logger.info("Has levitation."); |
| } | ||
| } | ||
| } | ||
| logger.info("Levitation {}, mass {}", totalLevitation, totalMass); |
| } catch (ArrayIndexOutOfBoundsException ignored) { | ||
| // This is actually the expected exit from the loop. | ||
| } |
There was a problem hiding this comment.
That seems really weird. Why do you need to rely on an ArrayIndexOutOfBoundsException to exit the loop 🤔
Rather than instantly moving blocks down when they fall, create an entity that stores all the data about the blocks and looks like them, then re-create the blocks when it lands. This depends on the engine PR MovingBlocks/Terasology#4614 for the rendering of the falling entities. If the block group contains some levitating blocks that aren't able to support its full weight, it will fall slower. I tried to make the falling and landing animation seamless, but unfortunately the entity can't contribute to (or block) block lighting, so there's still some discontinuity.
This PR also rearranges the package structure of the module, splitting it into org.terasology.fallingblocks.ecs, which includes all the classes that interact with the rest of the game, and org.terasology.fallingblocks.calculation, which includes all the classes involved in the internal connectivity calculations on the other thread.
Apart from adding the delay and the animation, a few other associated features are changed. Falling block groups no longer deal damage by crushing players beneath them (because that would be more difficult to implement with the appropriate delay, and it doesn't entirely fit when the falling block groups don't have normal collisions). Liquids are no longer displaced when solid blocks land in them, instead being destroyed, to account for the possibility that more blocks were placed at the original positions while the solid blocks were falling, which could leave the liquids with nowhere to go. Blocks that reach the bottom of the world (i.e. unloaded chunks) no longer land there, but continue falling until the entity is unloaded. Assuming entity saving works how I think it does, the blocks will resume their fall if those chunks they fell into are ever loaded. There is now an event for when blocks become detached and start falling, which may be cancelled to prevent the fall, or used to trigger other effects.
In multiplayer, the mesh for the falling block group entities is generated on the server and sent to the client, unlike all the other
ChunkMeshes which are generated by each client. This requires that the block tile texture atlas is identical on the client and on the server. This has been true in my testing (after a bit of debugging), with both a headless server and a non-headless one, but I suspect that it's a somewhat fragile assumption. Also, if the game is saved while some blocks are falling, then some changes are made to assets that change the layout of the tile atlas, the textures of the entity may be messed up when the game is resumed. This will probably not be too much of an issue, because the falling block entities are generally short-lived.Also in multiplayer, the falling animations are much less smooth, because the position updates as it falls are calculated on the server then sent to the client. This wouldn't be too hard to fix, with a client prediction system, but I thought I'd just push the basic version first.
There was a bug I observed a few times. It seems that in multiplayer, if the client destroys a block that just fell before the client sees it appear, the client will see the falling entity persist stuck in place past when it should have landed, and the whole block group that fell (the blocks, rather than the entity) disappears on the server. As I have no idea how FallingBlocks's code could have caused this, my current best guess for the reason is that there's some weird issue with the network code on the server when events overlap.