Skip to content
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

Support block placement #46

Closed
caelunshun opened this issue Aug 13, 2019 · 5 comments

Comments

@caelunshun
Copy link
Owner

commented Aug 13, 2019

No description provided.

@caelunshun caelunshun added this to the 0.4 milestone Aug 13, 2019

@caelunshun

This comment has been minimized.

Copy link
Owner Author

commented Aug 21, 2019

The most laborious part of this is that the item generator needs to generate mappings from items to blocks such that the block corresponding to an item can be found. Since, in the future, we all also have to add mappings the other way around (to handle survival mode digging), I envision adding another crate, named feather_block_item, which links together the block and item crates. This would avoid a cyclic dependency between feather_blocks and feather_items.

@caelunshun caelunshun self-assigned this Aug 23, 2019

@momothereal

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

I noticed that the handling of the Creative Item packet doesn't take into account the "raw slot" logic:

match packet.clicked_item.as_ref() {
Some(item) => {
inventory.set_item_at(packet.slot as usize, item.clone());
}
None => {
inventory.clear_item_at(packet.slot as usize);
}
}

Essentially, packet.slot won't match the "inventory slot" properly. For example, "Creative Slot #0" is not the hotbar, but the helmet slot. This mapping depends on the "top view" and "bottom view" of the inventory (think of the slots in a crafting bench, vs. the slots in a survival inventory with the equipment).

In Bukkit, there is this concept of "InventoryView" which has a convert-slot function to handle this.

This is an issue because there is a desync as to which item is currently held by the player: using the "hotbar index" for the inventory slot will give the wrong item.

@caelunshun

This comment has been minimized.

Copy link
Owner Author

commented Aug 23, 2019

Yes, I see what you mean. Inventory slot indices are currently numbered as per https://wiki.vg/Inventory#Player_Inventory, which is only valid for the normal inventory—not for when e.g. the player is interacting with a crafting table (although those aren't implemented, so I haven't observed the problem).

I'm not sure of the ideal way to fix this in a Rust-like way. Perhaps we could use an enum type with a variant for each possible slot and then map those variants to raw slot IDs depending on the inventory type.

I'm going to move this discussion into #79, since it's a separate issue from block placement.

@caelunshun

This comment has been minimized.

Copy link
Owner Author

commented Aug 26, 2019

Block placement is now implemented.

#79 and #84 remain as issues related to it, but both are unnoticeable with the current feature set (inventories other than the player inventory menu are unimplemented, and water/lava physics don't exist yet).

I will move discussion to those respective issues and close this.

@caelunshun caelunshun closed this Aug 26, 2019

@momothereal

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

Just to clarify, the reason I commented about indexing was because of a desync between the client and the server as to which item was being held by the player when placing the block. I didn't realize that the inventory.item_at(...) function does not have the same indexing as Bukkit, since you add SLOT_HOTBAR_OFFSET to get the slots for hotbar #0-8.

I'm curious if that constant only applies to the CreativeItemPacket / creative view though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.