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

Add broadcast command #148

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pumpkin-core/src/math/distance.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use super::vector3::Vector3;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I'm a fan of having something as trivial as "distance" be its own file unless we have a larger set of traits. Is there a reason this isn't in Vector3?

Something like:

    pub fn distance_to(&self, other: &Vector3<T>) -> T {
        (self - other).length()
    }


pub fn distance(p1: &Vector3<f64>, p2: &Vector3<f64>) -> f64 {
let dx = p1.x - p2.x;
let dy = p1.y - p2.y;
let dz = p1.z - p2.z;

dz.mul_add(dz, dx.mul_add(dx, dy * dy)).sqrt()
}
1 change: 1 addition & 0 deletions pumpkin-core/src/math/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod boundingbox;
pub mod distance;
pub mod position;
pub mod vector2;
pub mod vector3;
Expand Down
2 changes: 1 addition & 1 deletion pumpkin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ thiserror = "1.0"

# icon loading
base64 = "0.22.1"
png = "0.17.14"
png = "0.17.14"

# logging
simple_logger = { version = "5.0.0", features = ["threads"] }
Expand Down
123 changes: 123 additions & 0 deletions pumpkin/src/commands/cmd_say.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
use crate::commands::tree::CommandTree;
use crate::commands::tree::RawArgs;
use crate::commands::tree_builder::argument;
use crate::commands::CommandSender;
use crate::entity::player::Player;
use crate::server::Server;
use pumpkin_core::text::{color::NamedColor, TextComponent};

const NAMES: [&str; 1] = ["say"];
const DESCRIPTION: &str = "Sends a message to all players.";

const ARG_CONTENT: &str = "content";

pub fn consume_arg_content(_src: &CommandSender, args: &mut RawArgs) -> Option<String> {
let mut all_args: Vec<String> = args.drain(..).map(|v| v.to_string()).collect();

if all_args.is_empty() {
None
} else {
all_args.reverse();
Some(all_args.join(" "))
}
}

pub fn init_command_tree<'a>() -> CommandTree<'a> {
CommandTree::new(NAMES, DESCRIPTION).with_child(
argument(ARG_CONTENT, consume_arg_content).execute(&|sender, server, args| {
if let Some(content) = args.get("content") {
let content = parse_selectors(content, sender, server);
let message = &format!("[Console]: {content}");
let message = TextComponent::text(message).color_named(NamedColor::Blue);

server.broadcast_message(&message);

if !sender.is_player() {
sender.send_message(message);
}
} else {
sender.send_message(
TextComponent::text("Please provide a message: say [content]")
.color_named(NamedColor::Red),
);
}

Ok(())
}),
)
}

fn parse_selectors(content: &str, sender: &CommandSender, server: &Server) -> String {
let mut final_message = String::new();

let tokens: Vec<&str> = content.split_whitespace().collect();

for token in tokens {
let parsed_token = parse_token(token, sender, server);
final_message.push_str(&parsed_token);
final_message.push(' ');
}

final_message.trim_end().to_string()
}

fn parse_token<'a>(token: &'a str, sender: &'a CommandSender, server: &'a Server) -> String {
let result = match token {
"@p" => {
if let CommandSender::Player(player) = sender {
get_nearest_player_name(player)
.map_or_else(Vec::new, |player_name| vec![player_name])
} else {
return token.to_string();
}
}
"@r" => {
let online_player_names: Vec<String> = server.get_online_player_names();

if online_player_names.is_empty() {
vec![String::from("nobody")]
} else {
vec![
online_player_names[rand::random::<usize>() % online_player_names.len()]
.clone(),
]
}
}
"@s" => match sender {
CommandSender::Player(p) => vec![p.gameprofile.name.clone()],
_ => vec![String::from("console")],
},
"@a" => server.get_online_player_names(),
"@here" => server.get_online_player_names(),
_ => {
return token.to_string();
}
};

format_player_names(&result)
}

// Gets the nearest player name in the same world
fn get_nearest_player_name(player: &Player) -> Option<String> {
let target = player.last_position.load();

player
.living_entity
.entity
.world
.get_nearest_player_name(&target)
}

// Helper function to format player names according to spec
// see https://minecraft.fandom.com/wiki/Commands/say
fn format_player_names(names: &[String]) -> String {
match names.len() {
0 => String::new(),
1 => names[0].clone(),
2 => format!("{} and {}", names[0], names[1]),
_ => {
let (last, rest) = names.split_last().unwrap();
format!("{}, and {}", rest.join(", "), last)
}
}
}
2 changes: 2 additions & 0 deletions pumpkin/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod cmd_help;
mod cmd_kick;
mod cmd_kill;
mod cmd_pumpkin;
mod cmd_say;
mod cmd_stop;
pub mod dispatcher;
mod tree;
Expand Down Expand Up @@ -77,6 +78,7 @@ pub fn default_dispatcher<'a>() -> CommandDispatcher<'a> {
dispatcher.register(cmd_echest::init_command_tree());
dispatcher.register(cmd_kill::init_command_tree());
dispatcher.register(cmd_kick::init_command_tree());
dispatcher.register(cmd_say::init_command_tree());
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't your problem per-se, but When I see large blocks of repetitive code like this my eyes start to glaze over and I stop being able to read.

It sets off alarm bells that the code could be simplified some. It might not be getting to that point right now, but with how large and repetitive this is getting, another abstraction would be nice.

List Option

Something like

fn register_commands(&mut self, commands: impl IntoIterator<C>) 
where C: CommandTree
{
    for command in commands {
        dispatcher.register(command::default());
    }
}

// in default_dispatcher
dispatcher.register_commands(vec![ cmd_pumpkin, cmd_gamemode, ... ]);

Macro Option

macro_rules! init_commands {
    (($command:ident),*) => { $( dispatcher.register($command::init_command_tree()); )*}
}

// In default_dispatcher
init_commands!(cmd_pumpkin, cmd_gamemode, cmd_stop, cmd_help, cmd_echest, cmd_kill);

Say what you will about macros, but as long as they're explained, they make the boilerplate much more legible.

In the current code, dispatcher.register(...::init_command_tree()); is all noise, and what you're actually reading needs to be that tiny cmd_say

The macro option can be implemented as-is, but the list list one requires you to do some more significant refactors. Neither has to be implemented right now, but it's getting there.


dispatcher
}
Expand Down
16 changes: 16 additions & 0 deletions pumpkin/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use connection_cache::{CachedBranding, CachedStatus};
use key_store::KeyStore;
use parking_lot::{Mutex, RwLock};
use pumpkin_config::BASIC_CONFIG;
use pumpkin_core::text::TextComponent;
use pumpkin_core::GameMode;
use pumpkin_entity::EntityId;
use pumpkin_inventory::drag_handler::DragHandler;
Expand Down Expand Up @@ -127,6 +128,21 @@ impl Server {
}
}

/// Sends a message to all players in every world
pub fn broadcast_message(&self, content: &TextComponent) {
self.worlds
.iter()
.for_each(|w| w.broadcast_message(content));
}

/// Get all online player names
pub fn get_online_player_names(&self) -> Vec<String> {
self.worlds
.iter()
.flat_map(|world| world.get_player_names())
.collect::<Vec<_>>()
}

/// Searches every world for a player by name
pub fn get_player_by_name(&self, name: &str) -> Option<Arc<Player>> {
for world in self.worlds.iter() {
Expand Down
33 changes: 32 additions & 1 deletion pumpkin/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ use crate::{
use num_traits::ToPrimitive;
use parking_lot::Mutex;
use pumpkin_config::BasicConfiguration;
use pumpkin_core::math::vector2::Vector2;
use pumpkin_core::{
math::{distance::distance, vector2::Vector2, vector3::Vector3},
text::TextComponent,
};
use pumpkin_entity::{entity_type::EntityType, EntityId};
use pumpkin_protocol::{
client::play::{
Expand Down Expand Up @@ -259,6 +262,34 @@ impl World {
dbg!("DONE CHUNKS", inst.elapsed());
}

/// Sends a message to all players
pub fn broadcast_message(&self, content: &TextComponent) {
self.current_players.lock().values().for_each(|player| {
player.send_system_message(content.clone());
});
}
Comment on lines +266 to +270
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Can we make this generic over an arbitrary ChatMessage trait implemented for strings, textcomponents, etc? Or make it generic over something like Into<TextComponent>?


/// Gets all players
pub fn get_player_names(&self) -> Vec<String> {
self.current_players
.lock()
.values()
.map(|p| p.gameprofile.name.clone())
.collect::<Vec<_>>()
}

pub fn get_nearest_player_name(&self, target: &Vector3<f64>) -> Option<String> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this PR is not intended to give us the functionality for everything, but this should not be get_nearest_player_name, but

let Some(name) = get_nearest_player().map(Player::get_name) else {
    return Err(...)
 }

or something.

for a single get_nearest_... it works, but implementing this functionality for EVERYTHING you can get from a "nearest player" would be terrifying

self.current_players
.lock()
.values()
.min_by(|a, b| {
let dist_a = distance(&a.last_position.load(), target);
let dist_b = distance(&b.last_position.load(), target);
dist_a.partial_cmp(&dist_b).unwrap()
})
.map(|p| p.gameprofile.name.clone())
}

/// Gets a Player by entity id
pub fn get_player_by_entityid(&self, id: EntityId) -> Option<Arc<Player>> {
for player in self.current_players.lock().values() {
Expand Down
Loading