-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Entity Display Block - Move Item Display's Spawn Egg Entity Display Feature To Its Own Block #58
base: 1.21
Are you sure you want to change the base?
Conversation
import dev.hephaestus.glowcase.block.entity.SoundPlayerBlockEntity; | ||
import dev.hephaestus.glowcase.block.entity.SpriteBlockEntity; | ||
import dev.hephaestus.glowcase.block.entity.TextBlockEntity; | ||
import dev.hephaestus.glowcase.block.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please configure your IDE to not do automatic star imports
blockEntity.displayEntity.tick(); | ||
++blockEntity.displayEntity.age; | ||
} | ||
//does nothing right now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method should be removed
public ItemDisplayBlockEntity(BlockPos pos, BlockState state) { | ||
super(Glowcase.ITEM_DISPLAY_BLOCK_ENTITY.get(), pos, state); | ||
this(Glowcase.ITEM_DISPLAY_BLOCK_ENTITY.get(), pos, state); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason behind this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. This was part of an attempt to abstractify the class for the entity display to reduce copy-pasting before I decided against that. Forgot to change this back though
Edit: turns out I actually did use it. The EntityDisplayBlockEntity changes the BlockEntityType
because it extends the ItemDisplayBlockEntity and needs its own entity type.
@@ -61,5 +63,8 @@ | |||
"block.glowcase.item_acceptor_block.tooltip.1": " Pushes the item stack to an inventory", | |||
"block.glowcase.item_acceptor_block.tooltip.2": " Emits a redstone pulse", | |||
"block.glowcase.item_acceptor_block.tooltip.3": "Interact in Creative mode to edit", | |||
"block.glowcase.entity_display_block.tooltip.0": "Displays an entity from a spawn egg", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should just say "Displays an entity"
@@ -8,6 +8,8 @@ | |||
"glowcase:sprite_block", | |||
"glowcase:outline_block", | |||
"glowcase:particle_display", | |||
"glowcase:item_acceptor_block", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate entry
import net.minecraft.util.math.Vec2f; | ||
import net.minecraft.world.World; | ||
|
||
public class EntityDisplayBlockEntity extends ItemDisplayBlockEntity { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this extending the item display block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wanting to reduce the amount of copy-pasting from the Item Display as both blocks(item and entity displays) acted mostly the same. I can change to not extend it and copy-paste everything needed from the item display for the entity display if wanted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should derive common code from the same abstract class, like an AbstractDisplayBlockEntity
import net.minecraft.util.math.BlockPos; | ||
import net.minecraft.util.math.RotationPropertyHelper; | ||
|
||
public record C2SEditEntityDisplayBlock(BlockPos pos, ItemDisplayBlockEntity.RotationType rotationType, ItemDisplayBlockEntity.GivesItem givesItem, ItemDisplayBlockEntity.Offset offset, EntityDisplayBlockValues values) implements C2SEditBlockEntity { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not have a GivesItem
value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Toggling giving the item (yes/once/no) is part of the entity display block, just like the item display block(which is what this packet was based on - the item display's packet).
Have I misunderstood something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think chai was thinking this was only for display, but this actually hands out a spawn egg, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entity display blocks should not be interactable to adventure mode players, much less give out their spawn egg—use the item display block for that (the current item display block will be renamed to better reflect its behavior as an item provider)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was intending for it to give out the spawn eggs, but I will change it to not give out anything.
import java.util.List; | ||
|
||
public class EntityDisplayBlock extends GlowcaseBlock implements BlockEntityProvider { | ||
private static final VoxelShape OUTLINE = VoxelShapes.cuboid(0.25, 0.25, 0.25, 0.75, 0.75, 0.75); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a full block shape
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though the item display is that size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All other glowcase blocks currently are full block shape, it would be best for consistency
Move the item display's entity display feature to its own block. This is just a port of that feature, expanded slightly for now.
This is technically my first modded block (entity), but I mostly copy-pasted the code from the Item Display, and extended its classes where possible(not a lot from what I could tell). That means please look it over and cleanup where you see fit.
I've added the ability to scale the display entity, and also changed the code for determining the entity's pitch to adjust for scale(this was figured out through trial and error, no real methodology, fyi). The ability to tick the entity has been added, but the code for that has been commented out(
EntityDisplayBlockEntity$tick()
andEntityDisplayEditScreen$init()
with theaddDrawableChild(this.tickEntityButton)
method).Tested with NeoForge(technically on the as of writing current ModFest 1.21 pack). I have no clue why the horizontal rotation does not rotate with an empty hand for both the item display and entity display in the dev env, but both do work outside of the dev env so there's that.
Weird things I noticed that were part of the original Item Display feature:
I can change anything as needed.