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

Prototype of a way to get the name of inventories #1114

Draft
wants to merge 1 commit into
base: mc-1.19.x
Choose a base branch
from

Conversation

Quezler
Copy link
Contributor

@Quezler Quezler commented Jun 18, 2022

This is just a proof of concept, i do not expect this code to be merged as-is, or even inside this peripheral or this way:

During my most recent enigmatica 6 expert playthough me and my basemate opted for a basewide wired network, this of course meant lots of chests/barrels on the same network, which can get tricky to tell apart.

There are of course solutions to that, like having a disk drive with a floppy that contains a map of addresses, like:

  • ip.storage = 'minecraft:barrel_3' (for a barrel that imports into refined storage)
  • ip.foundry = 'tconstruct:drain_0' (for tinkers construct based fluid storage)

Of course those would still need to be manually maintained in case something gets moved, and all computers that imported it should be rebooted. (though that can be remedied with pulling peripheral change events and whatnot)

Another concept on the same wired network was using a special item in the 1st slot, for the output of several create contraptions we had a purple active logistic provider frame from pneumaticcraft signaling that that inventory should be emptied, and the ip.storage mentioned above had a yellow storage logistic frame in it to signal the destination, as well as a dedicated computer to orchestrate the bunch.

For recipe inputs this was a little more tricky since the "item in slot 1" should not be pulled into the machine, this of course was fixable by just blacklisting the item used for filtering from being pulled into the machine.

But in order for the computer(s) on the network to know which chest belonged to which machine's input we had to rename the slot 1 items in an anvil and then compare the displayName, here is a snippet that accomplished that goal:

_G.inventory_with_logistics_core = function(displayName)
  return peripheral.find('inventory', function(name, inventory)
    local item = inventory.getItemDetail(1)
    return item and item.displayName == displayName
  end)
end

This was stored on the networked floppy, the logistics core from pneumaticcraft was chosen because no recipes used it.

However, it would be easier if the names of chests (& other inventories) renamed in an anvil were accessible, then the input/output/storage chests could simply be named accordingly and there would be no more need for hardcoded _#'s or items inside inventories in order to tell them apart.

Some food for thought:

  • Nameable exists on multiple things (including fluid/energy storage), so inventory methods might not be the best spot.
  • Nameable supports hasCustomName, getDisplayName & getCustomName, maybe players would want access to all 3, or only custom names? perhaps its best made into its own generic peripheral, but those seem to demand capabilities.

This pull request is thus just a proof of concept, see it more as a suggestion/idea than code intended to merge as-is.
2022-06-18_09 10 18

local chest = peripheral.wrap('right')

--print(textutils.serialize( getmetatable(chest) ))

for k, v in pairs(chest) do
    print(k)
end

print('-')

print('the chest on my left is called ', peripheral.wrap('left').displayName())
print('the chest on my right is called ', peripheral.wrap('right').displayName())

(ps, the docblock has not been generated/tested, it received minimal love 💔)

@toad-dev
Copy link
Member

Hey, this looks familiar! I messed around with this feature in CC: Restitched and thought it worked very well in the small, vanilla-ish mod pack I was playing. I was concerned there might be spottiness in anvil renaming support with larger mod packs, but it sounds like you're on a big tech pack and that hasn't been a problem?

@Quezler
Copy link
Contributor Author

Quezler commented Jun 18, 2022

I haven't yet tested it in my current playthough, went ahead and made the pr on 1.19 whilst the modpack is on 1.16 😅

Your way looks a lot cleaner (i have no idea which method arguments can magically appear when requested) 👍

@Lupus590
Copy link
Contributor

made the pr on 1.19 whilst the modpack is on 1.16

1.16 is the main version of CC:T, Squid usually ports everything when releases are made. (Which may not happen anymore or happen less often as he's retired from CC dev now.)

@SquidDev SquidDev added enhancement An extension of a feature or a new feature. area-Minecraft This affects CC's Minecraft-specific content. labels Jul 8, 2022
@Quezler
Copy link
Contributor Author

Quezler commented Nov 24, 2022

I see this has a merge conflict now, is it worth fixing? (aka: does this (or toad's version) stand any chance @SquidDev?)

@Lupus590
Copy link
Contributor

What I've done for my Create Above and Beyond virtual pipe network is make a table called peripheralAlias that contains string keys with the peripheral names as the values.

So something like

local peripheralAlias = {
    treeFarmHopper = "minecraft:hopper_1",
    loggingDrawer = "storagedrawers:standard_drawers_4_4",
}

Under the hood my virtual pipes library is basically doing this peripheral.call(peripheralAlias.treeFarmHopper, "pushItems", peripheralAlias.loggingDrawer, slot).

This might be what you mean by manually maintaining a list, but I've been finding this not too bad for maintenance. I've rearranged my base (changing nearly all of the peripheral ids) and only having to change them in one place has been quite good compared to having string literals everywhere.

@SquidDev
Copy link
Member

SquidDev commented Dec 3, 2022

I see this has a merge conflict now, is it worth fixing? (aka: does this (or toad's version) stand any chance @SquidDev?)

Sorry, meant to reply to this, but entirely forgot!

I've been umming and ahhing about this, and I think it's a feature we want. However, I'm not quite sure how it the implementation should look: while the current implementation does work, it ends up putting the displayName method on every peripheral, which is definitely not what we want! Ideally we'd just target Nameable block entities which are already peripherals, but there's not really a good way to do that.

This is one of those times we kinda hit the limits of the generic peripheral system. Ugghr.

TLDR: I definitely want this, just in a bit of a limbo right now.

@Quezler
Copy link
Contributor Author

Quezler commented Jul 6, 2023

Hey @toad-dev, sorry to resurrect this thread on the forge version and not the restitched fabric version i find myself playing and finally having a chance to play around with it, but its no longer there?

I've tried searching through the git history for a few dozen minutes at this point and i just cannot seem to find the commit/reason why it was removed?

Screenshot 2023-07-06 at 14 16 08

Kinda narrowed it down to these 2 commits, if i press "view file at " on the bottom commit it shows the public static String name as still present, but when i view the file of the top commit i can no longer find it anywhere in that file, and the diff doesn't even mention it being removed:

Screenshot 2023-07-06 at 14 19 01

I would be most grateful if someone could point me to the commit and/or reason restitched pulled that feature. 🙂

@SquidDev
Copy link
Member

SquidDev commented Jul 7, 2023

It looks like this came in from cc-tweaked/cc-restitched@f880396#diff-96744b9a57d72c125614de97f777c3e98a2ba257f86c23ec044c56e48a41939b. This effectively "reset" CC:R to be based on the latest CC:T code.

@Toxic-Cookie
Copy link

Dead thread I know, BUT! I was trying to create a system that could generically transfer items from one inventory to another and the biggest hurdle is trying to find what an inventory is called so that I can transfer items to it. pushItems and pullItems will not accept "top", "bottom", "left", "right", "front" or "back" as valid targets and this inhibits my ability to create this system without a stupid workaround such as manually entering the names into a master table or "testing" each inventory until the target one is found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Minecraft This affects CC's Minecraft-specific content. enhancement An extension of a feature or a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants