-
-
Notifications
You must be signed in to change notification settings - Fork 186
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
Fix tool rules: Ensure get_speed and is_correct_for_drops check for tags #566
Changes from 5 commits
78bfb49
3d34c03
e5d7d43
30517cc
39d6b13
1895fdb
f187dfb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
use pumpkin_data::item::Item; | ||
use pumpkin_data::tag::{get_tag_values, RegistryKey}; | ||
|
||
mod categories; | ||
#[derive(serde::Deserialize, Debug, Clone, PartialEq, Eq)] | ||
|
@@ -28,26 +29,71 @@ impl ItemStack { | |
Self { item_count, item } | ||
} | ||
|
||
/// Determines the mining speed for a block based on tool rules. | ||
/// Direct matches return immediately, tagged blocks are checked separately. | ||
/// If no match is found, returns the tool's default mining speed or `1.0`. | ||
pub fn get_speed(&self, block: &str) -> f32 { | ||
if let Some(tool) = self.item.components.tool { | ||
for rule in tool.rules { | ||
if rule.speed.is_none() || !rule.blocks.contains(&block) { | ||
continue; | ||
// No tool? Use default speed | ||
let Some(tool) = &self.item.components.tool else { | ||
return 1.0; | ||
}; | ||
|
||
for rule in tool.rules { | ||
// Skip if speed is not set | ||
let Some(speed) = rule.speed else { | ||
continue; | ||
}; | ||
|
||
for entry in rule.blocks { | ||
if entry.eq(&block) { | ||
return speed; | ||
} | ||
|
||
if entry.starts_with('#') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be done better as:
|
||
// Check if block is in the tag group | ||
if let Some(blocks) = | ||
get_tag_values(RegistryKey::Block, entry.strip_prefix('#').unwrap()) | ||
{ | ||
if blocks.iter().flatten().any(|s| s == block) { | ||
return speed; | ||
} | ||
} | ||
Comment on lines
+41
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a method associated with |
||
} | ||
return rule.speed.unwrap(); | ||
} | ||
return tool.default_mining_speed.unwrap_or(1.0); | ||
} | ||
1.0 | ||
// Return default mining speed if no match is found | ||
tool.default_mining_speed.unwrap_or(1.0) | ||
} | ||
|
||
/// Determines if a tool is valid for block drops based on tool rules. | ||
/// Direct matches return immediately, while tagged blocks are checked separately. | ||
pub fn is_correct_for_drops(&self, block: &str) -> bool { | ||
if let Some(tool) = self.item.components.tool { | ||
for rule in tool.rules { | ||
if rule.correct_for_drops.is_none() || !rule.blocks.contains(&block) { | ||
continue; | ||
// Return false if no tool component exists | ||
let Some(tool) = &self.item.components.tool else { | ||
return false; | ||
}; | ||
|
||
for rule in tool.rules { | ||
// Skip rules without a drop condition | ||
let Some(correct_for_drops) = rule.correct_for_drops else { | ||
continue; | ||
}; | ||
|
||
for entry in rule.blocks { | ||
if entry.eq(&block) { | ||
return correct_for_drops; | ||
} | ||
|
||
if entry.starts_with('#') { | ||
// Check if block exists within the tag group | ||
if let Some(blocks) = | ||
get_tag_values(RegistryKey::Block, entry.strip_prefix('#').unwrap()) | ||
{ | ||
if blocks.iter().flatten().any(|s| s == block) { | ||
return correct_for_drops; | ||
} | ||
} | ||
Comment on lines
+71
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can all be de-duplicated by using the above suggestion |
||
} | ||
return rule.correct_for_drops.unwrap(); | ||
} | ||
} | ||
false | ||
|
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.
You should pull out the default speed to a constant value to avoid magic numbers