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

feat(AutoTool): Settings refactoring #5260

Open
wants to merge 2 commits into
base: nextgen
Choose a base branch
from

Conversation

sqlerrorthing
Copy link
Contributor

Made the settings more intuitive

Comment on lines +66 to +70
private val slot by int("Slot", 0, 0..8)

override fun getTool(inventory: PlayerInventory, blockState: BlockState?): IntObjectImmutablePair<ItemStack> {
return IntObjectImmutablePair(slot, inventory.getStack(slot))
}
Copy link
Member

Choose a reason for hiding this comment

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

Why would we want to choose a static slot though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already there, I didn't change the functionality of AutoTool itself

Copy link
Contributor Author

@sqlerrorthing sqlerrorthing Jan 11, 2025

Choose a reason for hiding this comment

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

Look at what's already available

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see.

@1zun4 1zun4 added this to the 0.24.0 milestone Jan 11, 2025
Comment on lines +252 to +273
fun PlayerInventory.findBestToolToMineBlock(
blockState: BlockState?,
ignoreDurability: Boolean = true
): IntObjectImmutablePair<ItemStack>? {
val (hotbarSlot, stack) =
(0..8).map {
it to getStack(it)
}.filter { (_, stack) ->
val durabilityCheck = (stack.damage < (stack.maxDamage - 2) || ignoreDurability)
(stack.isNothing() || (!player.isCreative && durabilityCheck))
}.maxByOrNull { (_, stack) ->
stack.getMiningSpeedMultiplier(blockState)
} ?: return null

val miningSpeedMultiplier = stack.getMiningSpeedMultiplier(blockState)

// The current slot already matches the best
if (miningSpeedMultiplier == player.inventory.mainHandStack.getMiningSpeedMultiplier(blockState)) {
return null
}
return IntObjectImmutablePair(hotbarSlot, stack)
}
Copy link
Member

Choose a reason for hiding this comment

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

No reason to move this into another class when it's not being used multiple times.

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 did it 'cause of the confusion in this code. Logically, I don't think PacketMine should inherit the AutoTool settings.

Copy link
Member

Choose a reason for hiding this comment

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

But it still does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm confused, make your own verdict, if it has to inherit AutoTool settings, then yes, there is no point in a separate [findBestToolToMineBlock] method.

): IntObjectImmutablePair<ItemStack>? {
val (hotbarSlot, stack) =
(0..8).map {
it to getStack(it)
Copy link
Contributor

Choose a reason for hiding this comment

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

IntObjectImmutablePair(it, getStack(it))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesnt make sense

Copy link
Contributor

Choose a reason for hiding this comment

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

int to any? -> Pair<Integer, @Nullable Object> while u has IntObjectImmutablePair as result type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in the end it's not my code (I moved it). If you want to change something, change it and create a pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants