search

AlmasB / FXGL

Java / JavaFX / Kotlin Game Library (Engine)
http://almasb.github.io/FXGL/
MIT License
4.41k stars 552 forks source link

Inventory - allow setting max item quantity #953

Closed AlmasB closed 3 years ago

AlmasB commented 3 years ago

When calling fun incrementQuantity(item: T, amount: Int): Boolean we should check max item quantity limit and split the "stack" into a new item data object

adambocco commented 3 years ago

Hey again,

I'd like to work on this.

Thanks

AlmasB commented 3 years ago

Cool, please provide a short API design and your approach to implement it, so we can discuss. For example, one immediate point is how we are planning to store split stacks. I don't think current approach (i.e. data structure) is sufficient because of duplicate keys.

On Fri, 5 Feb 2021, 4:35 pm Adam Bocco, notifications@github.com wrote:

Hey again,

I'd like to work on this.

Thanks

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/AlmasB/FXGL/issues/953#issuecomment-774142624, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3NT5S5Z7MFP2IQ2ESSSOLS5QM4DANCNFSM4WY5BI6Q .

adambocco commented 3 years ago

@AlmasB Sorry for the delay, I am in school and have been busy. I have given this some thought and I have two ideas.

1: - Add field maxStackQuantity to ItemData class

This would allow users to split stacks in the future.

2: Keep data structures and handle number of stacks in ItemData.

This option would be simpler but would only allow full stacks and one remainder stack (Ex. four stacks of 36 items: 10, 10, 10, 6 ) unless we added a data structure in ItemData to handle heterogenous stacks.

Let me know what you think!

Thanks, Adam Bocco

AlmasB commented 3 years ago

The first option seems most consistent with the current approach. Plus the new API should be compatible with the current one. The items() list is essentially the keys of the new map. Though getItemData will be returning a list I assume.

Feel free to go ahead with the implementation. It would probably be sensible to start with a test, then implementation, then finally update the sample with this new feature.

On Wed, 17 Feb 2021, 9:42 pm Adam Bocco, notifications@github.com wrote:

@AlmasB https://github.com/AlmasB Sorry for the delay, I am in school and have been busy. I have given this some thought and I have two ideas.

1: - Add field maxStackQuantity to ItemData class

  • Change itemsData field in Inventory to hash map ( T -> ObservableList
  • When incrementQuantity is called, it will traverse list and add on first stack that is not at capacity.

This would allow users to split stacks in the future.

2: Keep data structures and handle number of stacks in ItemData.

  • The new number of stacks and the "remainder stack" on add/remove/increment

This option would be simpler but would only allow full stacks and one remainder stack (Ex. four stacks of 36 items: 10, 10, 10, 6 ) unless we added a data structure in ItemData to handle heterogenous stacks.

Let me know what you think!

Thanks, Adam Bocco

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/AlmasB/FXGL/issues/953#issuecomment-780873680, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3NT5QF3KJSZYDFTWA4MZLS7QZ55ANCNFSM4WY5BI6Q .

AlmasB commented 3 years ago

Closed by #973