Skip to content
This repository has been archived by the owner on Aug 9, 2023. It is now read-only.

API for backpacks #33

Closed
grapereader opened this issue Oct 7, 2021 · 7 comments
Closed

API for backpacks #33

grapereader opened this issue Oct 7, 2021 · 7 comments

Comments

@grapereader
Copy link

The current backpack support is hard-coded and the recipes depend on minetest-game/farming for string. This seems out-of-place in what should be a generic inventory implementation.

Proposed solution:

  1. Define no backpacks by default
  2. Create a new API call i3.register_backpack(itemid, backpack_defn) which takes an existing registered item and adds the backpack behaviour to it. backpack_defn is a table with whatever properties make sense for a backpack (I imagine slot width and height, at a minimum).

If desired, the existing backpacks could be extracted to a companion mod i3-backpacks which uses the new call. This would have the added benefit of serving as an example implementation.

@grapereader grapereader changed the title Control backpack support from API API for backpacks Oct 7, 2021
@kilbith kilbith added enhancement New feature or request feature request and removed enhancement New feature or request labels Oct 7, 2021
@wsor4035
Copy link
Contributor

wsor4035 commented Oct 12, 2021

to be honest, this seems like a over complicated solution to just overriding the craft for bags
re:

  1. this is bad for non technical players or players who just want ease of use to install the mod and have it just work
  2. this will break any mod that already supports the existing bag design.

additionally as the bag is a craftitem you can just override pretty much every aspect of it (texture, groups, etc)

@wsor4035
Copy link
Contributor

solution for allowing completely custom backpacks to work while trying to remain as agnostic as possible would be changing https://github.com/minetest-mods/i3/blob/main/init.lua#L2956 to use minetest.get_item_group custom backpacks would need to simply add i3_backpack = #1-3 or whatever you want the group name to be

@grapereader
Copy link
Author

grapereader commented Oct 12, 2021

Well.. I don't agree with having fixed bag sizes either 😊

What if I want to create a mega-backpack? Or a super tiny single-inventory-row satchel?

Also: What if I want to limit the sorts of items that can be stored in it? Like a belt just for potions? Or a bag for food?

If this metadata can be communicated with item groups or some other field on the item itself, by all means. But there is more that can be exposed here beyond changing the recipe.

Fine with keeping the existing items as long as there is a way to remove them.

@wsor4035
Copy link
Contributor

  1. i mean, most other bag solution are all fixed sizes
  2. i3 inventory is expanded depending on the bag, so the expansion+(3 rows) would need to be divisible by 3
  3. i3 bags just add more slots to the players main inventory, so you would be forced into limiting there whole inventory(which seems like a terrible idea) or showing two different lists in the formspec(one for main, one for the bag), but then the player will wonder why in the world these random slots at the bottom are limited but not the rest. such a idea for limiting the type of items in a bag would be better suited for unified inventory where the bags are not inventory expansion slots
  4. its not metadata, its the groups table in a items def
  5. again, minetest api methods already exists for this minetest.unregister_item(name)

@kilbith
Copy link
Collaborator

kilbith commented Oct 19, 2021

e5f4464

@kilbith kilbith closed this as completed Oct 19, 2021
@wsor4035
Copy link
Contributor

@kilbith Perhaps this should stay open as a part two allowing mods to register additional group values as proposed here https://irc.minetest.net/minetest/2021-10-14#i_5886620 has not yet been implemented. Alternatively a new issue for this could be opened if you prefer?

@kilbith
Copy link
Collaborator

kilbith commented Oct 24, 2021

Current API fine as it is.

@kilbith kilbith closed this as completed Oct 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants