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(nextgen)(in progress): added ability to limit amount of different items #3784

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

Conversation

be4dev
Copy link
Contributor

@be4dev be4dev commented Aug 24, 2024

current issues:

  • filters aren't saved to config file
  • clickgui should be reentared to see new added filter

@be4dev be4dev marked this pull request as draft August 24, 2024 20:20
Copy link
Contributor

@superblaubeere27 superblaubeere27 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this! Nobody was brave enough to tackle this in a long time haha. This feature will be very useful when it is there.

I will help by creating a powerful interface for CleanupPlanGenerator which would support filter groups, if you want to use them :)

Comment on lines +63 to +74
availableItems.forEach { slot ->
val limit = template.itemLimitPerItem[slot.itemStack.item] ?: Int.MAX_VALUE
var itemCount = packer.usefulItems.count { it.itemStack.item == slot.itemStack.item }
packer.usefulItems.removeIf {
if (itemCount > limit) {
logger.info("removed item from useful items list")
itemCount--
return@removeIf true
}
false
}
}
Copy link
Contributor

@superblaubeere27 superblaubeere27 Aug 25, 2024

Choose a reason for hiding this comment

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

This is pretty hacky and doesn't really integrate with the rest of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is pretty hacky and doesn't really integrate with the rest of the code.

I agree, I will refactor this later

Comment on lines +72 to +92
@Suppress("UnusedPrivateProperty")
private val addNewFilter by boolean("AddNewFilter", false).onChange {
val itemType: Value<MutableList<Item>> = items("ItemsToLimit", mutableListOf()).onChanged {
recount()
}
val itemLimit: Value<Int> = int("MaxItemSlots", 0, 0..40).onChanged {
recount()
}
presentSettings.add(Pair(itemType, itemLimit))
false
}

@Suppress("UnusedPrivateProperty")
private val deleteFilter by boolean("DeleteFilter", false).onChange {
listOf("ItemsToLimit", "MaxItemSlots").forEach { name ->
val index = inner.indexOfFirst { it.name == name }
inner.removeAt(index)
}
recount()
false
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems hacky.

@SenkJu is this ok?

Comment on lines +55 to +70
private fun recount() {
val limits = mutableMapOf<Item, Int>()
presentSettings.forEach { (itemsValue, countValue) ->
val count = countValue.get()
itemsValue.get().forEach { item ->
val limitState = limits[item]
// we just follow the lowest filter
limits[item] = if (limitState == null) {
count
} else {
min(count, limitState)
}
}
}
itemLimits = limits
}
Copy link
Contributor

@superblaubeere27 superblaubeere27 Aug 25, 2024

Choose a reason for hiding this comment

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

The user configures i.e.:

- Firery Items:
    - items:
        - minecraft:flint_and_steel
        - minecraft:lava_bucket
    - limit: 2

As a user I'd think that the group Firery Items acts as a filter group and accepts 2 of any of the given items. (i.e. Lava, Lighter) or (Lighter, Lighter). But the client would accept 2 of each. (i.e. Lava, Lava, Lighter, Lighter). Is that the desired behaviour?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The user configures i.e.:

- Firery Items:
    - items:
        - minecraft:flint_and_steel
        - minecraft:lava_bucket
    - limit: 2

As a user I'd think that the group Firery Items acts as a filter group and accepts 2 of any of the given items. (i.e. Lava, Lighter) or (Lighter, Lighter). But the client would accept 2 of each. (i.e. Lava, Lava, Lighter, Lighter). Is that the desired behaviour?

I think it should be like this, may be it would be good to add ability to add new limits per category (and add various categories like Firery items (we can also possibly add ability to add new categories, but that seems kinda unnecessary))

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.

Making inv cleaner be less messier
3 participants