Tetris inventory (like tarkov) feedback

[near talon]

.

[steel umbra]

There is a lot of code, thus it is hard to do a comprehensive review. However, here a list of thought:

• GenericGrid does not implement IEnumerable.
• GenericGrid should define an Interface for its generic parameter. (GetWidth, GetHeight)
• The whole inventory system should be a POCO. (No reference to Unity at all)
• InventoryItem should not be mutable. (Nor clonable)

[near talon]

I understand, I appreciate it that you did a quick look!

• Thanks, I forgot that was a thing!
• With defining an interface you mean just giving the functions right?
• I need unity for vector3, any suggestion to solve this?
• I can see why you would want that but do you have an idea on how to solve the fact that I want to differentiate 2 items that are the same type but different instances? Also with the direction that it is facing?
  Actually now I think of it, it might be better to have a Item instance and have it reference to the scriptable that contains all const data (like name, sprite and so on)

On the IEnumerable topic, is the foreach still fine?

[steel umbra]

With defining an interface you mean just giving the functions right?
I mean using https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/where-generic-type-constraint

I need unity for vector3, any suggestion to solve this?
This fine to use Vector3. The point is to divide business logic and implementation to reduce the complexity.

Item instance and have it reference to the scriptable that contains all const data (like name, sprite and so on)
Exactly

[near talon]

I am not familiar with the where keyword, I've seen it before but I have not used it myself. How would this add value to the genericgrid class?

[steel umbra]

You would be able to manage if a piece can be placed, move or rotated without depending on knowing the implementation.

[near talon]

Like that, so moving functionality of the gridinventory to the genericgrid

[steel umbra]

Yes

[near talon]

I think I won't do that since you don't expect that from a generic grid class and it complicates stuff when yo uwant to remove an object from the grid that is wider or higher than 1 and is neighbouring a object of the same type

But, I'll keep in mind the existence of that keyword, thanks for the suggestion

[steel umbra]

In fact, I would even remove the whole Generic class idea

Instead, simply having something closer to your needs would be easier to make, manipulate and understand.

Instead of using T, I would simply use an interface.

Performance is hardly an issue here.

[near talon]

yeah I agree, I just figured something like a grid is very common so I wanted to make something very reusable

[steel umbra]

Not having contingous memory should not be a detriment

[steel umbra]

You are adding complexity for no added value