-
Notifications
You must be signed in to change notification settings - Fork 0
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
Bird cage #2
base: master
Are you sure you want to change the base?
Conversation
… might need a few changes.
- Parrots which are sitting down can be right-clicked with a cage to be placed in it - If the cage is named in an anvil, the parrot will be given the name of the cage - While caged, parrots will drop parrot feathers which currently have no use
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 made some general comments. Looks good though!
ItemScatterer.spawn(world, this.getBlockPos().getX(), this.getBlockPos().getY(), this.getBlockPos().getZ(), | ||
new ItemStack(ModIdItems.INSTANCE.getPARROT_FEATHER())); | ||
} | ||
featherDrop = 4000; |
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.
Do we want this to be constant? or should we add/subtract a random integer to it?
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.
Yeah I think it would be better if we added a random int every time. In fact, I think that's what I had originally, but I changed it for testing - I didn't want to wait for random lengths of time.
@@ -35,6 +37,12 @@ object ModIdBlocks { | |||
return block | |||
} | |||
|
|||
private fun <B: Block> addSpecialItemBlock(name: String, block: B): B { |
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.
The generic is actually not needed because of inheritance, but I forgot. The current version of the master branch removed the generic and just has block: Block.
I suggest we do the same here.
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.
Yeah let's remove it - I was just working from an older version and forgot to fetch the changes that you made.
@Inject(at = @At("HEAD"), method = "interactMob", cancellable = true) | ||
public void interactMob(PlayerEntity player, Hand hand, CallbackInfoReturnable<ActionResult> cir) { | ||
BlockPos cagePos = this.getBlockPos(); | ||
if (player.getStackInHand(hand).isOf(ModIdBlocks.INSTANCE.getBIRD_CAGE().asItem())) { |
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.
ModIdBlocks.INSTANCE.getBIRD_CAGE()
ugh this is some Kotlin -> java stuff... I bet there is a better way to get the bird cage from this object... gotta read the docs again
BlockPos cagePos = this.getBlockPos(); | ||
if (player.getStackInHand(hand).isOf(ModIdBlocks.INSTANCE.getBIRD_CAGE().asItem())) { | ||
if (this.isSitting() || this.isInSittingPose()) { | ||
if (player.getStackInHand(hand).hasCustomName()) this.setCustomName(player.getStackInHand(hand).getName()); |
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.
Does this override a parrots name? Because you might have renamed the parrot before putting it in the cage... I think we should check that?
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.
Well if you've purposely named the bird cage, then you probably want to change the parrot's name. I don't see why you would name it otherwise.
import net.minecraft.world.BlockView | ||
import net.minecraft.world.World | ||
|
||
open class CageBlock(settings: Settings?) : TallPlantBlock(settings) { |
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 forgot this, why is it an open class?
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.
So that it isn't final if we ever want to add other cage-type blocks with the same collison box.
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.
ah okay, wouldn't it make sense to "close" it for now and open it once we need it?
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.
Yeah that would probably be better.
(cherry picked from commit 4f36af3)
- Pillows change colour based on what block they are above - Pillow negate fall damage - Could possibly allow for a sleeping mechanic for pets in the future
Made a draft PR to better track this feature.
Some Things we've talked about so far
We discussed a change in parrots when they are in the cage:
This would add more use to parrots alongside decorative features.