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

Add ISpecialInventory #257

Closed
FunnyMan3595 opened this issue Nov 17, 2012 · 21 comments
Closed

Add ISpecialInventory #257

FunnyMan3595 opened this issue Nov 17, 2012 · 21 comments

Comments

@FunnyMan3595
Copy link

I'm happy to see the BuildCraft liquid API being added to Forge. I'd really love to see ISpecialInventory make the transition as well.

ISpecialInventory is similar to ISidedInventory in that it provides an interface for inserting and removing items from specific sides of a block. Unlike ISidedInventory, however, it leaves the underlying details of how items are stored up to the implementing TileEntity. This has some major advantages:

  • Items can be filtered, rejecting unacceptable items while accepting appropriate ones. As far as I can tell, this is only possible with ISidedInventory by engaging in hackish (and downright underhanded) subclassing of ItemStack.
  • Items can be sorted, created and destroyed on demand, or simply stored in non-array data structures, without abusing the array-like design of IInventory. This is all possible with ISidedInventory, but it's much cleaner with ISpecialInventory.

Currently, using BuildCraft's ISpecialInventory adds flexibility and cleanliness to your code, but leaves you tied to BuildCraft and not able to interface with other item transport systems--like Redpower's tubes--without adding redundant and ugly code... sometimes exceptionally ugly. Moving it to Forge, where it could be easily shared by all item transport systems, would be a major gain for anyone making blocks that produce, consume, store, or convert items. (Isn't that most of us?)

@CovertJaguar
Copy link
Contributor

+1

Although I've been told it could never work with RP for some reason that never made any sense to me.

@benblank
Copy link
Contributor

benblank commented Jan 7, 2013

I'd like to see this as well. ISidedInventory is very limiting, especially compared to the liquid API.

@immibis
Copy link
Contributor

immibis commented Feb 26, 2013

Why hasn't this been added?

@CovertJaguar
Copy link
Contributor

Because its hated passionately by some very vocal people.

@immibis
Copy link
Contributor

immibis commented Feb 27, 2013

Who would that be? Eloraam?

@Myrathi
Copy link

Myrathi commented Feb 27, 2013

The current, general concept of the inserter getting to choose what it gets to put into my blocks/machines - without some significant hackery on my part - has me in favour of this interface.

I'd like to hear the reasoning of these seemingly vocal people. "I hate it" doesn't hold much water, by itself.

@immibis
Copy link
Contributor

immibis commented Feb 27, 2013

I'd like it if extractItem took a parameter that could filter the extracted items, something like this:

public interface IStackFilter {boolean acceptsStack(ItemStack stack);}
ItemStack[] extractItem(boolean, ForgeDirection, int, IStackFilter);

Still not seeing how you could hate it, though.

@cpw
Copy link
Contributor

cpw commented Feb 27, 2013

I have an idea..

@immibis
Copy link
Contributor

immibis commented Feb 27, 2013

So did I, immibis@ab7f9be

ICustomInventory is the equivalent of ISpecialInventory.
ILinearInventory is the equivalent of ISidedInventory/IInventory. Inventories can be one or both (or neither, if something else is added later).

(Edit: ILinearInventory can do more than IInventory and is less retarded - I'm not duplicating it for no reason, and InventoryAdapters has a method to create ILinearInventory wrappers for IInventory objects)

@cpw
Copy link
Contributor

cpw commented Feb 28, 2013

Immibis, bear in mind isided is in vanilla in 1.5..

@CovertJaguar
Copy link
Contributor

There was talk of creating a pluginable mediator object that could allow inventories to interact with each other, even if they use different interfaces. I think it was @ScottKillen who had the idea.

@ScottKillen
Copy link
Contributor

@CovertJaguar My aged brain does not recall it, but I'll be happy to look into it. 😄

@CovertJaguar
Copy link
Contributor

Ah, no it was xcomp. My bad.

@ScottKillen
Copy link
Contributor

Also it would appear that Overmind has a solution.

@CovertJaguar
Copy link
Contributor

Yes, well there have been disagreements over his solutions in the past.

@FunnyMan3595
Copy link
Author

@cpw, are you going to clue us in on your idea anytime soon, or should we just keep throwing our own ideas at the wall to see if one sticks?

As I see it, the issue is basically a disagreement on what parts of the system should be "smart". Buildcraft provides an (essentially) dumb transport system and assumes that storage blocks can be smart, and so provides ISpecialInventory to allow them to define custom behaviour. RedPower, on the other hand, provides a smart transport system, and assumes that storage blocks are dumb containers, to be rifled through at will. Neither approach is good or bad, simply different architectural decisions.

Looking at it in that light, including ISpecialInventory in its current state would be a Bad Idea, as it provides no suitable interface for RedPower-style transport architecture. As it stands, we have four possible pairings, three of which are solved adequately. ISidedInventory handles Dumb->Dumb and Smart->Dumb transport->storage pairings, while ISpecialInventory handles Dumb->Smart pairings. We need a way to handle Smart->Smart pairings, and might as well absorb ISpecialInventory's Dumb->Smart support in the process, to keep the clutter to a minimum.

So, really, the question to ask is this: What API does @Eloraam need to be able to talk to a smart storage block? I could make some reasonable guesses, like the ability to ask how many it has of a particular item, or how many (more) of that item it can store. But, really, I think the right person to ask the question of is the lady herself, because (for reasons I hope are obvious) nobody else is better qualified to tell us what would work best for her.

@AbrarSyed
Copy link
Member

I am of the opinion that we do whatever we feel best because of 1.5. And then, Elo will probably fix her side when she updates for 1.5.

@cpw
Copy link
Contributor

cpw commented Mar 16, 2013

I think we wait and see what 1.5.1 brings..

@Myrathi
Copy link

Myrathi commented Mar 16, 2013

That statement is both interesting and worrying, cpw. :P

@LexManos
Copy link
Member

LexManos commented Jul 4, 2013

Seems this never got stable enough for everyone to accept. Feel free to try again with actual code and documentation/implementation next time.

@LexManos LexManos closed this as completed Jul 4, 2013
@CovertJaguar
Copy link
Contributor

Meh, its been stable for a year or something, people just don't like it. But its not really needed anymore anyway.

forgedev-teamcity pushed a commit that referenced this issue Mar 7, 2024
The Blogpost is available here: https://neoforged.net/news/20.2registry-rework/

---------

Co-authored-by: Minecraftschurli <minecraftschurli@gmail.com>
Co-authored-by: Dennis C <xfacthd@gmx.de>
Co-authored-by: Technici4n <13494793+Technici4n@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants