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

Slot System for inventories and board games #860

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Conversation

github-actions[bot]
Copy link
Contributor

Description

Store content in any kind of slots.

Usages

  • Add, Count and Remove items from slots
  • Check how many slots are filled or empty in a space
  • Set a maximum or unlimited amount of items for every slot
  • Add and remove properties of a slot (weapon category, item class, object weight...)
  • Move a slot content to an other position
  • Sum a property (good for weight system)
  • Sort items using a property value (ascending alphabetical only)
  • Flood system : add items automatically and when they reach the maximum of the slot, they go in the next empty slot
  • Export/Import spaces as JSON

Attention: this extension contains mostly 100 functions, it can contains mispelling and approximative English.

How to use the extension

3 systems available (Basic, Named slots and Grid)
They share the same functionnalities but have differences in their usage.

Basic

Use numbers as slot identifiers to retrieve your informations.
Start in setting the space size, then you can use it using numbers to manage your items and properties.

Named slots

Use texts as slot identifiers. Very useful when you want to put an item on a specific part of your character.
Start by adding all the named slots you want in your space. For example :

  • head
  • left hand / right hand
  • ring
    Then you can use the slot system and specify exactly where you want an item to be.

Grid

Use a 2D grid position to retrieve your informations. It uses the basic slot system in a more handy way.
It is a handler for the Basic system.
Instead of using numbers as identifiers, you use a position which comes more handy in many cases.
For example, you have a 3x3 board, it's easier to check if the slot (1,2) has an item, instead of "slot n°7" (row*width + column=2x3 + 1)

Checklist

  • I've followed all of the best practices.
  • I confirm that this extension can be integrated to this GitHub repository, distributed and MIT licensed.
  • I am aware that the extension may be updated by anyone, and do not need my explicit consent to do so.

What tier of review do you aim for your extension?

Reviewed

Example file

slot_system.zip

Extension file

SlotSystem_v2.zip

@github-actions github-actions bot requested a review from a team as a code owner April 11, 2023 16:19
@github-actions github-actions bot added the ✨ New extension A new extension label Apr 11, 2023
@github-actions github-actions bot added this to Needs review in Extensions review Apr 11, 2023
@D8H
Copy link
Contributor

D8H commented Apr 13, 2023

Thank you for submitting an extension.

  • What are the benefits from having a 1d and 2d access to the same inventory?
  • What happens if the number of slots is changed after setting the grid dimensions?
  • Do you see any issue if the extension were to be split in 3?

@D8H D8H added the 🔍 Reviewed extension An extension that is to be reviewed in great detail before merging. label Apr 13, 2023
@infokub
Copy link
Contributor

infokub commented Apr 13, 2023

  1. As said, 2D access is a handler, because depending on the case, it's more comfy to "think" the access with X/Y positions instead of a slot number. If i cut that, nothing will change for the extension, the 2D access only hiding the maths for a 1D access. And ppl will have to do the calculation themselves.
  2. Nothing, literrally. I wanted to let people being able to freely expand their system, It will have an impact if your new size is smaller than before, because if you added items on slots that are now over the max size, they won't be deleted. And they will also be kept in the JSON export. I note this as an issue.
  3. It means you will have 3 dataset that can't communicate between them like it does actually (you can move items from a set to an other). Splitting means considering they don't exist for each other, It means that all the methods that already exists for this purpose will have to be done by the user.
    It also means that the Grid access will require a complete rewrite because for now, it only use the Slot access. I will have to copy/paste the exact same code for the Slot but with a math line on top.

@D8H
Copy link
Contributor

D8H commented Apr 14, 2023

  1. As said, 2D access is a handler, because depending on the case, it's more comfy to "think" the access with X/Y positions instead of a slot number. If i cut that, nothing will change for the extension, the 2D access only hiding the maths for a 1D access. And ppl will have to do the calculation themselves.
    [...] the Grid access will require a complete rewrite because for now, it only use the Slot access. I will have to copy/paste the exact same code for the Slot but with a math line on top.

I guess users can do a 1d grid using a 2d grid of 1 row or 1 column.

  1. It means you will have 3 dataset that can't communicate between them like it does actually (you can move items from a set to an other). Splitting means considering they don't exist for each other, It means that all the methods that already exists for this purpose will have to be done by the user.

Moving items from one inventory type to another can be done with ony 2 action, so I think it's fine.

From what I understand a "space" contains:

  • 1 grid inventory
  • 1 text inventory

Why (from users point of view) are they group together under the same identifier?

I was expecting to find a "grid inventory name" parameter for grid and a "text inventory name" parameter for the text inventory.

@infokub
Copy link
Contributor

infokub commented Apr 14, 2023

I guess users can do a 1d grid using a 2d grid of 1 row or 1 column.

I don't get how this is an better evolution of what is already made.

Moving items from one inventory type to another can be done with ony 2 action, so I think it's fine.

It checks if the space and the slot exists, it copies the data to a temp variable, erase the destination and empty the source.
And you also need to copy all the custom properties you added to the slot, this is not provided yet.

From what I understand a "space" contains:

* 1 grid inventory

* 1 text inventory

Why (from users point of view) are they group together under the same identifier?

I was expecting to find a "grid inventory name" parameter for grid and a "text inventory name" parameter for the text inventory.

There are no grid, it's all based on an "array". The grid is the handler for making it easier for some cases to retrieve data.

It's under the same space bc there is no point to have two different ones. You can use it like that, or create two spaces, both are fine bc everything is made to manipulate data easily even on multiple spaces.

But for example, you could have one slot system with a space "inventory" which contains a grid of items + 3 "favorite items" slots (like in many survival games), made with named slots and put items from a slot to another with the Move actions easily.

I know you want to split it, but i still don't see any advantage of breaking a tool in 3 that will make the exact same thing, with a different data access. I mean for variables, you don't have a "boolean" part, "text" part and "number" part, they are all "variables" and they are also not split between "Global" and "Scene" for the same reason : both do the exact same thing, but the data is somewhere else.

I understand your concerns about users, but i don't think they will be so lost. The tool will require some documentation ofc, but it's not that hard to use. The concept is still to store stuff, it's not very deep.

@D8H
Copy link
Contributor

D8H commented Apr 15, 2023

Moving items from one inventory type to another can be done with only 2 action, so I think it's fine.

It checks if the space and the slot exists, it copies the data to a temp variable, erase the destination and empty the source. And you also need to copy all the custom properties you added to the slot, this is not provided yet.

Indeed, I hadn't though about the properties. It would not be user-friendly to make the data travel between 2 extensions.

There are no grid, it's all based on an "array". The grid is the handler for making it easier for some cases to retrieve data.

There is a grid if users can picture it. What is important for users to easily understand an extension is to use terms they are used to in games or in life in general. "Grid" is a good term because anyone know what is it whereas "Space" in this case has only the meaning the extension creator gave it. No one can guess that a "space" is an inventory with 1d, 2d and name indexes.

What do you think about "grid inventory" and "labeled inventory" ?

I would suggest to:

  • Make the 1d access private
  • Use a different id for grid inventories and labeled inventories
    • the identifier parameter names must be different
    • the extension variable should have 2 child-variable as root for the indexes

Tell me what you think (I don't know the extension well, I hope I didn't miss something).

@infokub
Copy link
Contributor

infokub commented Apr 15, 2023

Indeed, I hadn't though about the properties. It would not be user-friendly to make the data travel between 2 extensions.

Im glad we agree about that.

There is a grid if users can picture it. What is important for users to easily understand an extension is to use terms they are used to in games or in life in general. "Grid" is a good term because anyone know what is it whereas "Space" in this case has only the meaning the extension creator gave it. No one can guess that a "space" is an inventory with 1d, 2d and name indexes.

What do you think about "grid inventory" and "labeled inventory" ?

I get your point, good idea for "labeled" instead of "named", way better. But there is no inventory, it's a slot system, the inventory is one of the usecases but a board game, that is an other usecase, is not an inventory.

So if you have a better idea than space (that i'm not happy with) that can include all the cases and not let think that this extension is dedicated to inventories, i'll follow you bc for now, i don't have any better idea.

I would suggest to:

* Make the 1d access private

* Use a different id for grid inventories and labeled inventories
  
  * the identifier parameter names must be different
  * the extension variable should have 2 child-variable as root for the indexes

I don't understand what do you want here ?

Ok for 1D private, i understand your concerns.

But grid system and labeled system are already disconnected:

  • i'll find a way to force having two different ids (but i don't understand why, what will happen if people choose the same name for both ? they are already stored in two different namespace so they won't erase each other)
  • the identifier is already different bc one is a string, the other is a number, and they are already identified with a different tag (Slot for number, NamedSlot for string)
  • Each space is already split with a numerical storage and a labeled storage, the numerical one is directly under the root namepspace, for example : __SlotSystem.character[0] ..., the labeled one is under _NamedSlots, for example: __SlotSystem._NamedSlots.character.head so i don't understand what should i do here ?

Thanks for your time, i'll already work on what we agree and wait your answer for the other points.

@D8H
Copy link
Contributor

D8H commented Jul 6, 2023

Are you working on an new version?
I think this extension can already be useful as it is. If you want we could merge a first version.

@D8H D8H merged commit ddb592a into main Jul 20, 2023
Extensions review automation moved this from Needs review to Added to GDevelop Jul 20, 2023
@D8H D8H deleted the extension/infokub/859 branch July 20, 2023 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ New extension A new extension 🔍 Reviewed extension An extension that is to be reviewed in great detail before merging.
Projects
Extensions review
  
Added to GDevelop
Development

Successfully merging this pull request may close these issues.

None yet

2 participants