-
-
Notifications
You must be signed in to change notification settings - Fork 226
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
Feature: Shopping List #3281
base: beta
Are you sure you want to change the base?
Feature: Shopping List #3281
Conversation
One or more Detekt Failures were detected:
|
src/main/java/at/hannibal2/skyhanni/features/inventory/shoppinglist/ShoppingList.kt
Outdated
Show resolved
Hide resolved
One or more Detekt Failures were detected:
|
One or more Detekt Failures were detected:
|
src/main/java/at/hannibal2/skyhanni/config/commands/Commands.kt
Outdated
Show resolved
Hide resolved
One or more Detekt Failures were detected:
|
This pull request has conflicts with the base branch "beta". Please resolve those so we can test out your changes. |
Conflicts have been resolved! 🎉 |
One or more Detekt Failures were detected:
|
One or more Detekt Failures were detected:
|
One or more Detekt Failures were detected:
|
One or more Detekt Failures were detected:
|
oh shit, now I gotta do detekt |
why doesn't this pass the checks??? |
src/main/java/at/hannibal2/skyhanni/features/inventory/shoppinglist/ShoppingListItem.kt
Outdated
Show resolved
Hide resolved
12 Detekt Failures were detected:
|
12 Detekt Failures were detected:
|
add now, soon or later and maybe or not maybe
8 Detekt Failures were detected:
|
yay! |
src/main/java/at/hannibal2/skyhanni/features/inventory/shoppinglist/ShoppingList.kt
Show resolved
Hide resolved
src/main/java/at/hannibal2/skyhanni/features/inventory/shoppinglist/ShoppingListCategory.kt
Outdated
Show resolved
Hide resolved
@@ -351,6 +351,7 @@ object SackApi { | |||
sackListNames = uniqueSackItems.map { it.itemNameWithoutColor.removeNonAscii().trim().uppercase() }.toSet() | |||
} | |||
|
|||
// TODO: also update sacks when items are added/removed from sack, not only on summary |
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 probably remove TODOs if you aren't intending on doing it, and commented code that you don't intend on uncommenting
|
||
import com.google.gson.annotations.Expose | ||
|
||
class CategoryTemplate { |
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.
I'm not gonna suggest the change on each of them individually, as there are a lot, but do not define explicit constructor()
s for Kotlin classes where it isn't necessary to.
class CategoryTemplate { | |
class CategoryTemplate( | |
@Expose val name: String, | |
@Expose val color: Char = '6', | |
@Expose val hidden: Boolean = false, | |
@Expose val items: List<ItemTemplate> = emptyList(), | |
) { |
|
||
import com.google.gson.annotations.Expose | ||
|
||
class ItemTemplate { |
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.
Remove explicit constructor
import at.hannibal2.skyhanni.utils.PrimitiveRecipe | ||
import com.google.gson.annotations.Expose | ||
|
||
class RecipeTemplate { |
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.
Remove explicit constructor
} | ||
} | ||
|
||
@HandleEvent(onlyOnSkyblock = true, priority = HandleEvent.HIGH) |
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.
Doesn't seem this has a purpose to be high priority.
|
||
clear() | ||
|
||
add("aspect of the end".toInternalName(), 2.0, "Weapons") |
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.
What?
} | ||
|
||
// maybe name it removeCommand ??? | ||
fun remove(name: String, amount: Double? = null, categoryName: String? = null) { |
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.
This function is 3-4 nested if statements - refactor it into some helper functions if necessary, but this ain't it
|
||
fun get(item: NeuInternalName) = allItems[item] | ||
} | ||
|
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.
.toSearchable()
val items = items | ||
|
||
allItems.clear() | ||
for (category in categories + items) { |
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.
Define some vals here instead of nestedly looping, especially using similar names, it gets unreadable quickly.
What
Adds a general shopping list, can be added to by other features etc. also can break down items into their recipes
TODO: add technical details, screenshots
Images
Changelog New Features