(#364) Item objective#571
Conversation
ImplementsLegend
left a comment
There was a problem hiding this comment.
Seems fine.
I'll note thet there's nothing preventing you from bringing the items into the rift (atleast for the toss goal).
Also I was looking a bit into the Instant loot and I think it wouldn't be a bad idea to make an objective based on that.
| List<ItemStack> output = new ArrayList<>(); | ||
| for (int i = 0; i < lootRolls.getInt(context); i++) { | ||
| table.getRandomItems(context, output::add); | ||
| } | ||
| Object2IntMap<Item> itemCounts = new Object2IntArrayMap<>(output.size()); | ||
| for (ItemStack stack : output) { | ||
| itemCounts.merge(stack.getItem(), stack.getCount(), Integer::sum); | ||
| } |
There was a problem hiding this comment.
Does this need two separate loops?
| List<ItemStack> output = new ArrayList<>(); | |
| for (int i = 0; i < lootRolls.getInt(context); i++) { | |
| table.getRandomItems(context, output::add); | |
| } | |
| Object2IntMap<Item> itemCounts = new Object2IntArrayMap<>(output.size()); | |
| for (ItemStack stack : output) { | |
| itemCounts.merge(stack.getItem(), stack.getCount(), Integer::sum); | |
| } | |
| Object2IntMap<Item> itemCounts = new Object2IntArrayMap<>(); | |
| for (int i = 0; i < lootRolls.getInt(context); i++) { | |
| table.getRandomItems(context, stack->itemCounts.merge(stack.getItem(), stack.getCount(), Integer::sum); | |
| } |
|
|
||
| @SubscribeEvent | ||
| public static void onContainerClose(PlayerContainerEvent.Close event) { | ||
| GoalManager.getGoalStates(event.getEntity(), CollectItemGoal.class).forEach(goalState -> { |
There was a problem hiding this comment.
Iterating through the entire inventory per goal seems inefficient, although I'm not sure what would be better.
There was a problem hiding this comment.
I could change the logic to get the ingredients from all the CollectItem goals, loop over the inventory once checking each item against each ingredient and collecting their counts, and then progressing each goal with a second loop afterwards. It effectively is just swapping inner and outer loops. Though the recursive inventory loop is presumably a bit chunkier in terms of iteration cost so might be some value there. Might be premature optimisation though.
There was a problem hiding this comment.
In terms of generally iterating the entire inventory... Yeah, it is mostly a catch all. I didn't find a good hook for container interaction. Although looking now potentially could add a slot listener to every menu.
| /** | ||
| * Visit for traversing containers | ||
| */ | ||
| public interface ItemVisitor { |
There was a problem hiding this comment.
Why not use Consumer<ItemAccessor>?
Yeah. My understanding is the plan would be to have objective-specific loot in the future, in which case we'ld presumably remove it from the player when they leave the rift. Which would solve this and other issues with the toss goal (e.g. tossing your gear on accident when it happens to be the goal and it being wiped from existence). |
feat: Collections helper class refactor: combined goal event handlers with relevant event type
Honestly, insta-loot with custom items would probably make a better item collection objective than the toss goal. Also we could drop Rewards and just use instant pickup items instead. |
refactor: combined two loops
671f5bc to
6a7706e
Compare
fix: add rewards to collect_items objective
Background
This PR adds an item collection based objective to the rift.
Testing Instructions
The objective can added to a rift key by including one or more buckets in the recipe. It requires throwing away items of specific types, which are deleted when thrown away.
Implementation
Notes
Closes #364