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

Correct Interactive Block Behavior #388

Merged
merged 9 commits into from
Dec 27, 2024
2 changes: 1 addition & 1 deletion pumpkin-core/src/math/position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::math::vector2::Vector2;
use num_traits::Euclid;
use serde::{Deserialize, Serialize};

#[derive(Clone, Copy)]
#[derive(Clone, Copy, PartialEq, Eq)]
/// Aka Block Position
pub struct WorldPosition(pub Vector3<i32>);

Expand Down
7 changes: 7 additions & 0 deletions pumpkin-inventory/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ pub trait Container: Sync + Send {

fn all_slots_ref(&self) -> Vec<Option<&ItemStack>>;

fn clear_all_slots(&mut self) {
let all_slots = self.all_slots();
for stack in all_slots {
*stack = None;
}
}

fn all_combinable_slots(&self) -> Vec<Option<&ItemStack>> {
self.all_slots_ref()
}
Expand Down
14 changes: 14 additions & 0 deletions pumpkin-inventory/src/open_container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ use pumpkin_world::block::block_registry::Block;
use pumpkin_world::item::ItemStack;
use std::sync::Arc;
use tokio::sync::Mutex;

pub struct OpenContainer {
// TODO: unique id should be here
// TODO: should this be uuid?
players: Vec<i32>,
container: Arc<Mutex<Box<dyn Container>>>,
location: Option<WorldPosition>,
Expand Down Expand Up @@ -54,6 +56,18 @@ impl OpenContainer {
}
}

pub fn is_location(&self, try_position: WorldPosition) -> bool {
if let Some(location) = self.location {
location == try_position
} else {
false
}
}

pub async fn clear_all_slots(&self) {
self.container.lock().await.clear_all_slots();
}

pub fn all_player_ids(&self) -> Vec<i32> {
self.players.clone()
}
Expand Down
18 changes: 13 additions & 5 deletions pumpkin/src/block/block_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::block::pumpkin_block::{BlockMetadata, PumpkinBlock};
use crate::entity::player::Player;
use crate::server::Server;
use pumpkin_core::math::position::WorldPosition;
use pumpkin_inventory::OpenContainer;
use pumpkin_world::block::block_registry::Block;
use pumpkin_world::item::item_registry::Item;
use std::collections::HashMap;
Expand Down Expand Up @@ -34,7 +35,7 @@ impl BlockManager {
) {
let pumpkin_block = self.get_pumpkin_block(block);
if let Some(pumpkin_block) = pumpkin_block {
pumpkin_block.on_use(player, location, server).await;
pumpkin_block.on_use(block, player, location, server).await;
}
}

Expand All @@ -49,7 +50,7 @@ impl BlockManager {
let pumpkin_block = self.get_pumpkin_block(block);
if let Some(pumpkin_block) = pumpkin_block {
return pumpkin_block
.on_use_with_item(player, location, item, server)
.on_use_with_item(block, player, location, item, server)
.await;
}
BlockActionResult::Continue
Expand All @@ -64,7 +65,9 @@ impl BlockManager {
) {
let pumpkin_block = self.get_pumpkin_block(block);
if let Some(pumpkin_block) = pumpkin_block {
pumpkin_block.on_placed(player, location, server).await;
pumpkin_block
.on_placed(block, player, location, server)
.await;
}
}

Expand All @@ -77,7 +80,9 @@ impl BlockManager {
) {
let pumpkin_block = self.get_pumpkin_block(block);
if let Some(pumpkin_block) = pumpkin_block {
pumpkin_block.on_broken(player, location, server).await;
pumpkin_block
.on_broken(block, player, location, server)
.await;
}
}

Expand All @@ -87,10 +92,13 @@ impl BlockManager {
player: &Player,
location: WorldPosition,
server: &Server,
container: &mut OpenContainer,
) {
let pumpkin_block = self.get_pumpkin_block(block);
if let Some(pumpkin_block) = pumpkin_block {
pumpkin_block.on_close(player, location, server).await;
pumpkin_block
.on_close(block, player, location, server, container)
.await;
}
}

Expand Down
65 changes: 43 additions & 22 deletions pumpkin/src/block/blocks/chest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use async_trait::async_trait;
use pumpkin_core::math::position::WorldPosition;
use pumpkin_inventory::{Chest, OpenContainer, WindowType};
use pumpkin_macros::{pumpkin_block, sound};
use pumpkin_world::{block::block_registry::get_block, item::item_registry::Item};
use pumpkin_world::{block::block_registry::Block, item::item_registry::Item};

use crate::{
block::{block_manager::BlockActionResult, pumpkin_block::PumpkinBlock},
Expand All @@ -15,8 +15,15 @@ pub struct ChestBlock;

#[async_trait]
impl PumpkinBlock for ChestBlock {
async fn on_use<'a>(&self, player: &Player, _location: WorldPosition, server: &Server) {
self.open_chest_block(player, _location, server).await;
async fn on_use<'a>(
&self,
block: &Block,
player: &Player,
_location: WorldPosition,
server: &Server,
) {
self.open_chest_block(block, player, _location, server)
.await;
player
.world()
.play_block_sound(sound!("block.chest.open"), _location)
Expand All @@ -25,46 +32,60 @@ impl PumpkinBlock for ChestBlock {

async fn on_use_with_item<'a>(
&self,
block: &Block,
player: &Player,
_location: WorldPosition,
_item: &Item,
server: &Server,
) -> BlockActionResult {
self.open_chest_block(player, _location, server).await;
self.open_chest_block(block, player, _location, server)
.await;
BlockActionResult::Consume
}

async fn on_close<'a>(&self, player: &Player, _location: WorldPosition, _server: &Server) {
async fn on_broken<'a>(
&self,
block: &Block,
player: &Player,
location: WorldPosition,
server: &Server,
) {
super::standard_on_destroy_with_container(block, player, location, server).await;
}

async fn on_close<'a>(
&self,
_block: &Block,
player: &Player,
_location: WorldPosition,
_server: &Server,
_container: &OpenContainer,
) {
player
.world()
.play_block_sound(sound!("block.chest.close"), _location)
.await;
// TODO: send entity updates close
}
}

impl ChestBlock {
pub async fn open_chest_block(
&self,
block: &Block,
player: &Player,
location: WorldPosition,
server: &Server,
) {
let entity_id = player.entity_id();
// TODO: This should be a unique identifier for the each block container
player.open_container.store(Some(2));
{
let mut open_containers = server.open_containers.write().await;
if let Some(chest) = open_containers.get_mut(&2) {
chest.add_player(entity_id);
} else {
let open_container = OpenContainer::new_empty_container::<Chest>(
entity_id,
Some(location),
get_block("minecraft:chest").cloned(),
);
open_containers.insert(2, open_container);
}
}
player.open_container(server, WindowType::Generic9x3).await;
// TODO: shouldn't Chest and window type be constrained together to avoid errors?
super::standard_open_container::<Chest>(
block,
player,
location,
server,
WindowType::Generic9x3,
)
.await;
// TODO: send entity updates open
}
}
90 changes: 75 additions & 15 deletions pumpkin/src/block/blocks/crafting_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,52 +6,112 @@ use async_trait::async_trait;
use pumpkin_core::math::position::WorldPosition;
use pumpkin_inventory::{CraftingTable, OpenContainer, WindowType};
use pumpkin_macros::pumpkin_block;
use pumpkin_world::{block::block_registry::get_block, item::item_registry::Item};
use pumpkin_world::{block::block_registry::Block, item::item_registry::Item};

#[pumpkin_block("minecraft:crafting_table")]
pub struct CraftingTableBlock;

#[async_trait]
impl PumpkinBlock for CraftingTableBlock {
async fn on_use<'a>(&self, player: &Player, _location: WorldPosition, server: &Server) {
self.open_crafting_screen(player, _location, server).await;
async fn on_use<'a>(
&self,
block: &Block,
player: &Player,
_location: WorldPosition,
server: &Server,
) {
self.open_crafting_screen(block, player, _location, server)
.await;
}

async fn on_use_with_item<'a>(
&self,
block: &Block,
player: &Player,
_location: WorldPosition,
_item: &Item,
server: &Server,
) -> BlockActionResult {
self.open_crafting_screen(player, _location, server).await;
self.open_crafting_screen(block, player, _location, server)
.await;
BlockActionResult::Consume
}

async fn on_broken<'a>(
&self,
block: &Block,
player: &Player,
location: WorldPosition,
server: &Server,
) {
super::standard_on_destroy_with_container(block, player, location, server).await;
}

async fn on_close<'a>(
&self,
_block: &Block,
player: &Player,
_location: WorldPosition,
_server: &Server,
container: &OpenContainer,
) {
let entity_id = player.entity_id();

for player_id in container.all_player_ids() {
if entity_id == player_id {
container.clear_all_slots().await;
}
}

// TODO: should re-add all items to player or drop?
}
}

impl CraftingTableBlock {
pub async fn open_crafting_screen(
&self,
block: &Block,
player: &Player,
location: WorldPosition,
server: &Server,
) {
//TODO: Adjust /craft command to real crafting table
let entity_id = player.entity_id();
player.open_container.store(Some(1));
{
let mut open_containers = server.open_containers.write().await;
if let Some(ender_chest) = open_containers.get_mut(&1) {
let mut open_containers = server.open_containers.write().await;
let mut id_to_use = -1;

for (id, container) in open_containers.iter() {
if let Some(a_block) = container.get_block() {
if a_block.id == block.id && container.all_player_ids().is_empty() {
id_to_use = *id as i64;
}
}
}

if id_to_use == -1 {
let new_id = server.new_container_id();

log::info!("New ct ID: {}", new_id);

let open_container = OpenContainer::new_empty_container::<CraftingTable>(
entity_id,
Some(location),
Some(block.clone()),
);

open_containers.insert(new_id.into(), open_container);

player.open_container.store(Some(new_id.into()));
} else {
log::info!("Using previous ct ID: {}", id_to_use);
if let Some(ender_chest) = open_containers.get_mut(&(id_to_use as u64)) {
ender_chest.add_player(entity_id);
} else {
let open_container = OpenContainer::new_empty_container::<CraftingTable>(
entity_id,
Some(location),
get_block("minecraft:crafting_table").cloned(),
);
open_containers.insert(1, open_container);
player
.open_container
.store(Some(id_to_use.try_into().unwrap()));
}
}
drop(open_containers);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what's the need for manually managing memory as if it was C

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what's the need for manually managing memory as if it was C

I think it's not necassary here but it's more like freeing the guard then managing the memory.
For example if you call an sub function that trys to lock the variable too you create an deadlock.
Which results in freezing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a draft and in progress :D I think I was trying to fix a different issue that I thought was from a deadlock. I will probably refactor this code

player
.open_container(server, WindowType::CraftingTable)
.await;
Expand Down
Loading
Loading