SO Inventory with an abstract class as it's container type

[dreamy hinge]

I've got here an inventory SO script, which has an InventoryItem as the type for it's container. I have created three versions of this inventory as SOs in the editor.

The inventoryItem is meant to be an 'inventory slot', holding the item and the quantity. Shown at the bottom of the hastebin link.

But the issue I have is that if I want to create a new InventorySlot, I can't then get the item of that slot because those are held in the abstract classes.

if (!isFound)
{
    inventoryContainer.Add(new InventoryItem(item, quantity));
    InformAboutChange();
}
``` I get an error here.

I understand that you cannot create instances of abstract classes, but id' like it so that the item passed in can be any of these two types and it still work, and then in other scripts if I want to get that item I can equally just get it and it returns the right type.
https://hastebin.com/share/gebuqupuxi.csharp

[latent forge]

youre doing all of this because you want to transfer between scenes right

[dreamy hinge]

nope. It's so I don't have to create 3 different inventory scripts that all do the same things but with different types

[latent forge]

whats wrong with having 1 mono with 3 Lists in them?

[dreamy hinge]

Before I had this absolute mess

[dreamy hinge]

Coming from this, I want just one inventory script, one slot (since a slot will always be an item and a quantity), but for it to work with different item types without needing me to duplicate the inventory scripts and only changing types

[weak frost]

so this item parameter can be both derived types of InventoryItem?

[indigo thorn]

I just barely skimmed over the convo in the channel, but you really dont need to check the type of item. Theres no point in using abstract class if you have to do this.

[weak frost]

can always just do AbstractType abstract = new DerivedType(Item)

and pass by the abstract value

not really sure why the construction of the item is ambiguous

[dreamy hinge]

Basically. These are the different types an item can be, I wanted the item slot to be able to use these interchangably https://hastebin.com/share/mixiderapo.csharp

[dreamy hinge]

Haven't seen an example where the class names are mixed like that, what does that do?

[weak frost]

Sure, you can store it as the base type, but you need to construct the item as the type you expect it to be.

otherwise you don't have that polymorph behaviour

[weak frost]

It upcasts it so you can store it in fields that only accept less derived types.

you can still use the more derived polymorph method calls from it, but it won't know any method implementions of the most derived type it may be

Sooo if you have a Item base type with Use(), you will always use the most derived version even if used as the base type, but you'd have to do type comparison to call the method Attack(), if the type has a more derived method from a type like a weapon

assuming you override Use() that is

[indigo thorn]

What I do is have an Item which stores an ItemSO, it takes whatever values needed like stats, and other effects (which are an SO) That item SO let's me just create new instances

My Item itself doesnt actually use abstract class, which may make things simpler for you. Unique functionality is basically offloaded to the SO which is abstract and can do anything I want it to

[weak frost]

I try to just shove as much functionality into my Use() methods

More like Use(IEntity)

[indigo thorn]

I offload all that to my Effect SO

Although my effect is kinda shitty. It declares each method as virtual (and does nothing), any child class has an enum to state which method it wants to override and declare functionality for.
But yea I understand the confusion the question had, because you actually have to find some way to know what child item you want to make from the item SO. (if I've understood their setup properly)
I think the best solution is not having item be abstract

[weak frost]

honestly not sure of the question but from the looks from it they want to store any derived type in a slot that is of base type, but to do that is you need to construct it as the derived type and then upcast it.

Or make some interface implementation, but that's technically just doing the same thing.

[dreamy hinge]

Does that then lose out on some of the functionality of the derived?

[weak frost]

if the method was implemented in the base, but overridden in the derived, it will use the derived version when called.

what you lose out though is you don't exactly know what derived methods it may have implemented because you don't know the exact type

and to do that is run it through type comparison

[indigo thorn]

You dont lose out on anything that the base class says that exists. If the derived class declares it own methods and fields, then itll be unusable from only having the base class.

But you dont need those in the first place if they weren't on the base class

[dreamy hinge]

in this case I do, which is the issue

[indigo thorn]

If I was on my pc I would show an example I do, but wont be home for a bit

[weak frost]

If this is some inventory you're swapping between other inventories you can do type checking for those type of operations

Like, your inventory only stores the base type, but if you have an equipment inventory that stores derived items of type equipment only, then you need to check and down cast the types

[dreamy hinge]

It's not necessarily swapping between two inventories. Each inventory only holds a certain item type, so at most something will be removed from one, processed then added to another in a different form

[indigo thorn]

I still think itd be better if item itself wasnt abstract, because you will need some other way to define what child class an item is. Unless you define them ALL in code or link some enum to class

[dreamy hinge]

if the item wasn't abstract I have no clue how I'd then make it so a slot can hold different types with shared functionality between them

[weak frost]

if each inventory only stores one type, why not just make an inventory of each type

[dreamy hinge]

cause then I'd have 3 scripts with almost identical code

[weak frost]

Inventory<T> where T : Item

[dreamy hinge]

everything would be the same aside from the item in this inventoryItem slot being a different type

 public class InventoryStructureItem : InventoryItem
{
    public StructureSerialised item;

[indigo thorn]

What I was suggesting above was item is just a class, but it holds data like ItemSO which can be abstract. This way you can just do new Item

Unless you've already fixed this issue about items. I'm not sure what the 3 inventory is about

[dreamy hinge]

Would that be able to replace this at all?

private List<InventoryItem> inventoryContainer;

If I could make it so this accepts any type that extends inventory item how would that work

[weak frost]

T would be the item slotted type you'd change

but otherwise same inventory

[indigo thorn]

The generic they showed is constrained to be Item only, that is what the "where" part is for

[weak frost]

if you needed to implement completely different functionality I think you'd probably just want to derive Inventory to other types but that doesn't mean code duplication

[indigo thorn]

But how are you defining what an item is in the first place? Like where are your actual items?

Are you writing them all in code

[dreamy hinge]

I've got 3 different types of item - Raw, Processed and Structure. A raw item has one asset made in the assets panel, but the inventory for raw items will all have different names and describes that aren't predeteremined. The processed items are all defined as SO assets, as each of those items will have a select pool they can be, and a structure is another SO asset, but each structure that is saved will have it's own details about the different parts that are built together

So for instance

[indigo thorn]

Discord is breaking, some images arent loading

[dreamy hinge]

struggling on my end too

basically, the processed items are SOs like your traditional inventory would be, where you have a list of possible items and each slot references a predefined asset

but the raw and processed ones will be wholely unique, they won't have a pool to choose from, each one will have different values inside

[indigo thorn]

I think the naming here is a bit confusing, what is an InventoryItem?

Because you shouldnt be using new on these objects if they are SO

[dreamy hinge]

Essentially an inventory slot. ```cs
public abstract class InventoryItem
{
public int quantity;

public abstract void ChangeQuantity(int newQuantity);

public abstract InventoryItem GetItem();

}

In this case it holds a quantity, and an inventory consists of these slots.

But since each slot can be one of three types, the actual item this slot holds is in an inherited class.

```cs
public class InventoryPlainItem : InventoryItem
{
    public ItemSO item;

    public InventoryPlainItem(ItemSO item)
    {
        this.item = item;
    }
    public override void ChangeQuantity(int newQuantity)
    {
        quantity = newQuantity;
    }

}```

But a structure item, for instance, isn't an ItemSO, it's its own thing. ```cs
public class InventoryStructureItem : InventoryItem
{
public StructureSerialised item;
public override void ChangeQuantity(int newQuantity)
{

}

}

So all in all, I wanted to just be able to use the inventory code I've written so far, and me to be able to simply do 'add item' or whatever with the type it would be, and then it puts that in the inventory

I thought something like this wouldn't be too uncommon, but i guess I was wrong

Then just for more context, I'd put these on some kind of controller gameobject, and reference the inventories that I've made as separate assets

When I had ItemSO in the inventoryItem class, it all worked fine, but when I tried to make it so that slot can hold any item time, I got stuck as I didn't know how to keep it working smoothly

[weak frost]

If the inventory isnt a mono you can go with the generic idea

[dreamy hinge]

haven't tackled generics yet I don't think

[indigo thorn]

I think the whole InventoryItem thing might be a bit much, because I dont see what youd really ever need it for.

An inventory really can just be a list<item>

[dreamy hinge]

well it's so I can have an item and a quantity associated with it. That's how stuff is presented on the Ui so I figured that would be the neatest way

[weak frost]

Yeah, if there's not real difference in the behaviour

only problem is if it is a mono then you cant really add that constraint

[indigo thorn]

Your item itself can store the quantity too

[weak frost]

well, you can add constraints but you can't serialize that on the inspector

it would need to be a defined constraint

Like DerivedInventory<DerivedItem> : Inventory<DerivedItem>

[dreamy hinge]

hang on, I think I've just made a realisation that a scriptable object won't work for the structures or raw items

[indigo thorn]

It does really sound weird that some items are SO and you said some arent

[dreamy hinge]

cause an SO is created in the asset menu, then items reference that. But a raw item could be anything as it's just described by a collection of strings, and a structure is a series of game objects

so if I have those inventory slots be SOs, then they'd all be pointing to one

dammit

right

well, in that case, I may need that multiple inventory approach instead

I wonder if I could draw up a diagram to help explain a bit better

Tell me if my logic works here, I'm going to have a whole bunch of these, all representing the processed items that can be in the inventory

I was going to use a SO as the raw item, but there can be infinite of them, since they will each just essentially be a string that the user types in

And a structure, the user builds and then stores

So does that mean then that I can't even use SOs for the latter two

My head aches from all this today

been at it for 9 hours now

[indigo thorn]

You can create instances at runtime, but really you'll suffer the same thing as before where you need a way to know what derived class to make

Maybe give it a try for what I said before, keep Item just as one thing. It defines fields for whatever every single item will use. Then it takes data from an ItemSO which is basically used as a data container to fill in data. Since item is no longer abstract you can just new one up and not worry about what child class to use. ItemSO can be abstract if you need it to have effects like a Use() function

Right now I am even lost on what you are doing tbh. All the names and different declarations items such as Raw item, etc, is a bit confusing.

[dreamy hinge]

wish I could find a way to explain it better

only other way would literally just be to talk it through in voice

[indigo thorn]

Maybe try to describe what you want each child class to actually do

[weak frost]

id figure out the 4 inventory idea and work backwards from that

[indigo thorn]

But really these all arent even child classes because not all of them derive from 1 Item

[dreamy hinge]

The child classes of this inventoryItem (inventory slot that is), were going to be ways for me to have different types of items represented by the same slot class, so I can just do a GetItem method for instance, and itll return a structureSerialised or itemSO based on what that slot was made with

[weak frost]

always better to be explicit as possible, but usually it's not managable with multiple types, but in your case you dont have much

[dreamy hinge]

don't suppose any of your are free to go over it in a voice call?

[indigo thorn]

I dont do calls, but also I am at work

[weak frost]

really I am working (not) but if you got a diagram of it ill catch up on it

[indigo thorn]

I think you should start by simplifying it a bit to have a bare bones system. I still dont see the use of the slots thing. Whatever holds the List<item> which something has to eventually do, can just handle the "slots" which are just the index

And try to start by only holding Item, having different classes without one encapsulating class is gonna be a pain

From there you can extend the functionality in child classes of what Item can even do

[dreamy hinge]

I have completely borked it

it was going so well up to this point, was super clean

I've not commited this stuff yet, can I commit it, roll back to where I was before then branch it off to come back to it later?

[indigo thorn]

You can put it on a branch, or stash it, or commit it and revert. Might not want to commit it on the main branch otherwise itll be a pain to undo those changes and keep your current (after todays) changes

Or commit it and branch then work separately I guess.

[dreamy hinge]

I'm on my own one for the minute, but if I can commit, rollback then branch off of that rollback to carry on with the old system I might

[indigo thorn]

Im not sure if itll let you branch without committing but I vaguely remember github desktop letting me bring changes to a new branch

[dreamy hinge]

I can do a commit, that's not really an issue. it's just whether I can branch off of a previous commit

in the meantime I'll bust out my drawing pad to see if I can do a diagram

[indigo thorn]

I would prioritize the ease of creating items and interacting with the inventory before any cleanliness things come into play. If it's a major pain to define what an item can do, it doesnt matter how clean your system is because you likely wont be motivated to making items

[indigo thorn]

I'm not entirely sure. I assume so because you can change what commit you are on

[dreamy hinge]

I had it working perfectly when I just had the processed items, it's adding the different item types into the same slots where things just didn't work

[indigo thorn]

I suffer from something similar with my item effects, it's easy for one item to define an effect when I want it like "when taking damage, do this effect". But then I added an item I want to run every frame (a timer for a heal). I ended up defining every method in the base effect class and logging an error inside it. If it's not overrided by a child class then it shouldnt be called

100% not a clean method and relies on me setting up certain values in inspector properly. But I can create effects so easily

[dreamy hinge]

although with how I set up my ui and inventory interactions, It's going to take even more thought

[indigo thorn]

Worry about UI after. UI will just end up calling existing methods in your code like "swap item 4 and 5"

All you need to be aware of is what the interactions might be. Swapping slots, dropping items, equipping items, using items. All of these should be defined without the UI in mind. Then the UI comes along and decides to call some of these public methods

[dreamy hinge]

in my current project, literally the only thing that happens is clicking an object in the ui to spawn it in

[indigo thorn]

You can use ContextMenu or other premade attributes like NaughtAttributes.Button to let you do this from inspector instead of having to even touch UI for now

In my project as well, one guy made some test UI to interact with the inventory. But it looks bad, isnt clear, doesnt truly work, inventory functionality is changing so then the test UI has to change. It's just a waste

[dreamy hinge]

the UI was working before, I could spawn in those processed items that have set items defined in the assets, it's just introducing the other item types it seems

I'm definitely going to need to have a good think about all this

so I can't really have one item slot whose item can be of any time, and still have it work out

a full 10 hours of work and I've had to go all the way back to square 1 :(

[dreamy hinge]

Wanted to open this back up, I've had a go at using the IS and AS statements to get this thing working, and by the looks of things this achieves what I've set out to do. But is it a worthile way? Looks a bit messy and repetitive, but I'm only going to have these three item types anyways. ```cs
public void AddItem(T item, int quantity)
{
bool isFound = false;
for (int i = 0; i < inventoryContainer.Count; i++)
{
if (item is ItemSO)
{
var CurrentSlot = inventoryContainer[i] as InventoryPlainItem;

        if (CurrentSlot.item == item as ItemSO)
        {
            inventoryContainer[i].ChangeQuantity(quantity + inventoryContainer[i].quantity);
            InformAboutChange();
            isFound = true;
        }
    }
    else if(item is Item)
    {
        var CurrentSlot = inventoryContainer[i] as InventoryUniqueItem;

        if (CurrentSlot.item == item as Item)
        {
            inventoryContainer[i].ChangeQuantity(quantity + inventoryContainer[i].quantity);
            InformAboutChange();
            isFound = true;
        }
    }
    else if (item is StructureSerialised)
    {
        var CurrentSlot = inventoryContainer[i] as InventoryStructureItem;

        if (CurrentSlot.item == item as StructureSerialised)
        {
            inventoryContainer[i].ChangeQuantity(quantity + inventoryContainer[i].quantity);
            InformAboutChange();
            isFound = true;
        }
    }


}
if (!isFound)
{
    if (item is ItemSO)
    {
        InventoryPlainItem newItemSlot = new InventoryPlainItem();
        newItemSlot.item = item as ItemSO;
        inventoryContainer.Add(newItemSlot);
    }
    else if (item is Item)
    {
        InventoryUniqueItem newItemSlot = new InventoryUniqueItem();
        newItemSlot.item = item as Item;
        inventoryContainer.Add(newItemSlot);
    }
    else if (item is StructureSerialised)
    {
        InventoryStructureItem newItemSlot = new InventoryStructureItem();
        newItemSlot.item = item as StructureSerialised;
        inventoryContainer.Add(newItemSlot);
    }
    


    InformAboutChange();
}

}```

[weak frost]

Half awake but if it works then it works honestly, but you could probably make a generic method here (give all these a comparable type perhaps)

seems like you were trying to as well

[dreamy hinge]

Yeah in #💻┃code-beginner I've been having a look at generic methods. Been tough trying to wrap my head around it but so far it seems to be working fairly well

[weak frost]

when you do newItemSlot.item = item as StructureSerialised, is the type info lost?

[dreamy hinge]

not sure

haven't been able to properly test it since so many parts rely on others that I've spent the last few weeks just fixing compile errors. But I'm not seeing any squigglies when I use that

[weak frost]

what type is newItemSlot.item, is it a type they all derive from?

[dreamy hinge]

In this case, no, each of the types are separate. The only derived classes here are the item slot classes, as I've got a base inventory item slot with a quantity value, the rest derive from that and contain their respective unique types

[weak frost]

public void AddItem<T>(T item, int quantity) where T : YourBaseSlotClass //Add the constraint
{
    bool isFound = false;

    for (int i = 0; i < inventoryContainer.Count; i++)
    {
        if (inventoryContainer[i] is InventorySlot<T> currentSlot && currentSlot.item == item)
        {
            currentSlot.ChangeQuantity(quantity + currentSlot.quantity);
            InformAboutChange();
            isFound = true;
            break;
        }
    }

    if (!isFound)
    {
        InventorySlot<T> newItemSlot = new InventorySlot<T>();
        newItemSlot.item = item;
        inventoryContainer.Add(newItemSlot);
    }
}```

Something like that maybe if you want to use T

[dreamy hinge]

So instead of having different slot types for the different items, just one slot where the item is type T?

[weak frost]

You'd have to make the slot generic, but thinking maybe you'd dig a hole if you've not really used them before

[dreamy hinge]

I've already dug myself many holes trying to figure this out

Here's what I have for the time being in the inventory https://hastebin.com/share/ewijekuwey.csharp

[weak frost]

Ah, yeah you've got quite a lot going on but ill take a look later

[dreamy hinge]

Yeah, it's become rather convuluted. This is the result of a refactor of me trying to make it better but my understanding has led me down many pitfalls

[dreamy hinge]

Been talking about about it in code-beginner, it's really not going well. I would have thought this is a problem that would be fairly common for an inventory system, one where many different types of items exist in the same inventory container, but apparently my specific implementation isn't

Might consider another rewrite

if you were to do an inventory system like this, where there are different classes for the items in it, where would you start

Heck I’ve even had a look at the inventory code for games like Minecraft to try and figure something out

[weak frost]

Under the assumption that each inventory can store a single type of item, then I'd go about making an abstract type that all of these items inherit from (let's call it Storable class). With that I'd make an abstract Inventory<T> where T : Storable.

That'll give you enough abstraction to work with, allowing you to further derive with some shared behaviours

[weak frost]

If you are trying to make a shared inventory, sticking to the Storable base type is fine. Type comparisons are w/e, but ideally you do work through polymorphic calls beyond type comparisons.

[dreamy hinge]

Would that then allow all the items in that container to really be of any type?

[weak frost]

If you want to make an inventory that stores any type then they all must derive from the same class (assuming you want to take advantage of polymorphic methods), and that you store these items as that base type and only do type comparisons when you need to do work on them.

Or similar interface implementation

[dreamy hinge]

I’ll give it a shot sometime in the morning. Got another idea to try, but I’m not sure it’ll work yet

I had some difficulty with actually adding the item when using type comparisons, as it wouldn’t let me add the different slot type to the container even though they derived from that class

But I’ll get a more solid explanation tomorrow

[dreamy hinge]

Ended up trying this out ```cs
public class InventorySO<T, I> : ScriptableObject where T : InventoryItem, new() where I : class
{

But then creating concrete classes that I can create assets from, ```cs
public class RawInventorySO : InventorySO<InventoryUniqueItem, RawItemLabels>
{
    
}
``` But the actual inventories don't appear in the inspector anymore, strangely enough. I've got the serialisable tag there, does it just not like being a subclass like this?

Just going to go with the final resort and make three separate inventory scripts entierly. been a solid month now, I'll get it working then explore alternatives later