Create icon indicating copy to clipboard operation
Create copied to clipboard

Computer Craft integration for Update 6 Logistics Blocks

Open BirbIrl opened this issue 8 months ago • 24 comments

This is the backported version of @AsterAether, @Karotte128, @McArctic and I's efforts at bringing the logistics blocks to full compatibility with computercraft! The original PR can be found here: https://github.com/Creators-of-Create/Create/pull/7669 Note that 1.20.1 is technically the "base" branch for us, and our 1.21.1 branch merges these commits in with extra patches.

Oh also there's like 10 lines that got edited out cuz of linting (probably cause of me, i refuse to use an IDE and rock neovim just to feel something), whoops! I'm afraid of adding commits to undo that as to not confuse git with what should be merged and how between the branches. Hope that's okay!

oh and Aster, Karotte and Arcitc all have equal say here. if you have any questions and i'm unavailable when they are - take their word and assume i agree with them! :D

BirbIrl avatar Mar 10 '25 12:03 BirbIrl

Question: I may have missed it in the linked issue, but would this add the ability for computers to send item requests (similar to the existing stock-keeper) via a function (ie something like stockRequest(itemname, quantity, destination, source(optional, could just grab from any available storage)) and/or requestCraft(itemname, quantity, nameofporttosendmaterials)? Again, if this is already in #7669 or the PR, I apologise, thank you for your time.

edit - Nevermind, just checked the wiki linked in the issue, thanks for making this! This'll defo come in handy

Natelolzzz avatar Mar 12 '25 11:03 Natelolzzz

Question: I don't even know if I'm allowed to ask this hear, So please excuse if I'm not. There isn't a way to get the address of a package at the moment right? If it's not will that ever be added? Thanks in advance.

lucyamonster avatar Mar 12 '25 17:03 lucyamonster

Question: I don't even know if I'm allowed to ask this hear, So please excuse if I'm not. There isn't a way to get the address of a package at the moment right? If it's not will that ever be added? Thanks in advance.

all good to ask here! Sadly - not in a straightforward way. Packages in packagers instantly get sucked in and turned into items. When the Event PR comes in from ElementW (https://github.com/Creators-of-Create/Create/pull/7453) we might be able to add an event to packagers that will return the package contents and it's address whenever it accepts a package :D

For now, if that doesn't get pulled in or our extra patch doesn't come through in time, you won't be able to tell what the package is addressed to unless you set a frogport to a specific address, then it'll call for those to be brought over and swatted out. That's one way to keep track of addresses in the current state of the PR.

The address of the package is hidden in NBT, so unfortunately there's no way to get the address with .getItemDetail() either, and we shouldn't inject into an existing Computercraft function.

There was an idea to make the repackager be able to analyse packages in the inventory it's connected to, and assemble them via code, but it turned out to be a project wayyy over our heads, considering the amount of things we've already added. We're a bit out of steam. Anyone is free to try and contribute to our fork (which will get synced with this PR), though! We have a group on discord regarding this PR if anyone's interested in contributing. Anyone is free to join, just add me @birbirl on discord :>

BirbIrl avatar Mar 12 '25 17:03 BirbIrl

the original PR should be closed, this one will be merged up if accepted

TropheusJ avatar Mar 12 '25 21:03 TropheusJ

the original PR should be closed, this one will be merged up if accepted

On it! Do note though that the other PR has the additional patches already included for your convenience to merge up to 1.21.1 cleanly. the way peripherals are registered has changed between versions, as well as some rendering stuff. So feel free to look at the 1.21.1 branch of our fork when merging up. Thanks!

BirbIrl avatar Mar 12 '25 22:03 BirbIrl

Small set of tweaks came in: 1: the Repackager can now get the package's address with .getPackageAddress (Thank you, lucyamonster!) 2: the checkPacakge() function has been renamed to .getPackageItems, the repackager now also has access to that function. 3: the packager and repackager now remember their address when unloaded, but still forget it if they call a package with a computer detached. (Thank you, AsterAether!)

Gonna update the wiki pr with the new information.

also we're no longer maintaining the 1.21.1 branch on our end. :D

BirbIrl avatar Mar 13 '25 18:03 BirbIrl

Would it be possible to add a filtered makePackage() function to the packager and/or to have multiple filters for the stock ticker's requestFiltered() function?

ThePuzzlemaker avatar Mar 14 '25 07:03 ThePuzzlemaker

Would it be possible to add a filtered makePackage() function to the packager and/or to have multiple filters for the stock ticker's requestFiltered() function?

for packager's filtered makePackage() - that is not a base create feature. the packager grabs the first few items from an inventory and packages them. The stock ticker supports sorting items by tags in the ui, hence why it excluisvely gets the requestFiltered() function. The point of the CC additions is to automate manual tasks (like changing signs for addresses, powering it with redstone, changing the redstone requester's filter, changing shop prices), not add brand new functionality to blocks, which all serve specific purposes. The previous CC integration added the ability to automate changing the speed of the rotational speed controller - but it doesn't let you set decimal speeds, it doesn't let you set 0rpm either. all of which it could do, but the first one is against Create as a whole, and the other is the Clutch's task. Hope that makes sense. Technically we upgraded the shop's functionality because you can change wares without placing/breaking the cloth, but i think it's fine. The only block we're looking at "expanding" functionality is the re-packager, but we feel like the PR is in a healthy spot without that.

Finally, as for multiple filters in the requestFiltered() function, just call the function multiple times with different filters. If we were to add multiple filters on the java end, it'd just be us looping through every arg - so instead of that, we let people run the function multiple times in lua :D.

BirbIrl avatar Mar 14 '25 12:03 BirbIrl

Hello guys. When this PR will be finished, what would happen with it? Will it come into newest major version or minor? Or it will be as separate extension mod? I don't really know how your's development pipeline happens

Redplcs avatar Apr 26 '25 18:04 Redplcs

Hello guys. When this PR will be finished, what would happen with it? Will it come into newest major version or minor? Or it will be as separate extension mod? I don't really know how your's development pipeline happens

Depends, its up to the create devs if they want to merge it into the mod or not. I cannot speak to what version it will come into either if it gets added at all

McArctic avatar Apr 26 '25 18:04 McArctic

Hello guys. When this PR will be finished, what would happen with it? Will it come into newest major version or minor? Or it will be as separate extension mod? I don't really know how your's development pipeline happens

Currently feature work is on the back burner while bugs are fixed, once feature work is resumed any and all feature PRs will be reviewed.

IThundxr avatar Apr 26 '25 18:04 IThundxr

Depends, its up to the create devs if they want to merge it into the mod or not

So what if devs will discard PR. Will it become as separate mod?

Redplcs avatar Apr 26 '25 18:04 Redplcs

Depends, its up to the create devs if they want to merge it into the mod or not

So what if devs will discard PR. Will it become as separate mod?

No clue lol, I only did a lil bit for this pr and setup some ground work stuff with like one block lol. So I guess it depends what the rest of the group wants to do. Altho I have a good feeling it will get merged eventually:)

McArctic avatar Apr 26 '25 18:04 McArctic

Depends, its up to the create devs if they want to merge it into the mod or not

So what if devs will discard PR. Will it become as separate mod?

If this PR is not merged we can look into making this a addon. But I hope this will be merged. I do not want to waste time creating the addon if this ends up getting merged.

Karotte128 avatar Apr 26 '25 20:04 Karotte128

Altho I have a good feeling it will get merged eventually:)

I don't think this is a good idea to patch few lines of code just for better integrations with other mods. Having patches for individual mods in one day will become a mess that you must to support. That will increase the complexity and maintenance will be must harder. If the integration more relies on reflection then it's much worth, because in term of modding when one mod getting methods of other mods through reflection someday it will break the backward compatibility, because any refactoring will break everything.

I think addon will be more realistic here. The developers doesn't need to support the patch. I assume they think the same

Redplcs avatar Apr 26 '25 21:04 Redplcs

Altho I have a good feeling it will get merged eventually:)

I don't think this is a good idea to patch few lines of code just for better integrations with other mods. Having patches for individual mods in one day will become a mess that you must to support. That will increase the complexity and maintenance will be must harder. If the integration more relies on reflection then it's much worth, because in term of modding when one mod getting methods of other mods through reflection someday it will break the backward compatibility, because any refactoring will break everything.

I think addon will be more realistic here. The developers doesn't need to support the patch. I assume they think the same

All the code used is part of CC: Tweaked's API, and create already has CC: Tweaked compat, a addon is not the realistic approach here.

IThundxr avatar Apr 26 '25 21:04 IThundxr

All the code used is part of CC: Tweaked's API, and create already has CC: Tweaked compat, a addon is not the realistic approach here.

Wait what? Why the hell Create depends on CC: Tweaked? What's the purpose of it??

Redplcs avatar Apr 26 '25 21:04 Redplcs

All the code used is part of CC: Tweaked's API, and create already has CC: Tweaked compat, a addon is not the realistic approach here.

Wait what? Why the hell Create depends on CC: Tweaked? What's the purpose of it??

It doesn't it just uses it's API when it's present. Aka it's "compatible"

Vap0r1ze avatar Apr 26 '25 21:04 Vap0r1ze

It doesn't it just uses it's API when it's present. Aka it's "compatible"

Through reflection? I still don't see any reasons for relying on other mod, that's not a part of the core, even if it's presented on fly. Anyway you know way better than me, so good luck with your project

Redplcs avatar Apr 26 '25 22:04 Redplcs

So what if devs will discard PR. Will it become as separate mod?

All of our work is under MIT! i don't think any of us mind if our code gets used in any addon willing to add the compatibility as long as we get credit! ComputerCraft Create Bridge/Create Tweaks and Additions both already add some things we don't have in the base mod regarding computers, so the pr could be adapted to both those mods and sent their way. I personally won't be making any other pr's or making my own addon - but anyone is free to! I'd advise waiting for the verdict from the create team - and just compiling our fork if you wanna mess around with the features.

I still don't see any reasons for relying on other mod

tldr: It allows people to make cool things, while still staying with create's base design principles. Also not using the CC api would really just be reinventing the wheel, and after dealing with minor lua vs java types and type safety i can say that squid did an amazing job on the api and i wish upon no one else the pain of making that infrastructure ever again.

Funnily enough using the computercraft api has actually led to issues already with a crash on boot, but it only happened if you have both computercraft: tweaked and create installed. That got patched in like 2 days on CC: Tweaked's end (thanks Squid!). It's worth it, even if it leads to some extra work sometimes.

And yeah we're a patient bunch, take all the time you guys need to squash bugs :D

BirbIrl avatar Apr 27 '25 01:04 BirbIrl

Hi, a bit of test feedback for you here: we've been running your PR (updated for 6.0.4, see https://github.com/aearil/Create/tree/cctweaked-logistics) for nearly a month now without any issues.

In particular my GF built a 1500 LoC logistic system heavily relying on the redstone requester's setAddress(), configure(), and request() APIs, as well as the stock ticker's list(). That feels like a pretty good stress test (that computer basically replaces and improves upon a full wall of gauges).

The only caveat here is that I didn't take the time to get crafting info embedded into packages working with the new PackageOrderWithCrafts API.

In any case, let me now if you want me to bring my changes to this PR.

aearil avatar Apr 28 '25 19:04 aearil

In any case, let me now if you want me to bring my changes to this PR.

Anyone is free to contribute! Development on this is "done", as in everything we felt like adding - we added, but we have a group chat on discord you can join by adding me (@birbirl on discord) where we can talk about if adding something is a good idea or not before spending a day implementing something only to realize it wasn't a good idea to begin with (also we can add you to our fork as a contributor).

Also feel free to PR your updated version on our fork so that the create devs have an easier time, since you seem to have also added a patch for the redstone requester (was it not working on 6.0.4 without it?)

and yeah the PackageOrderWithCrafts api makes as much sense in my tiny bird brain as lovecraftian horrors. If you feel like working with it, go ahead!

Also since your partner had a lot of experience with the pr, mind asking her how was the documentation? anything missing, unclear, maybe in need of examples?

Thanks :D

BirbIrl avatar Apr 29 '25 14:04 BirbIrl

Hey @IThundxr, hope you don't mind the ping after the couple of months of this PR - but we reached a game-design choice we can't quite decide on.

So, originally this PR allowed people to make crafting requests with the redstone requester by adding blank spaces, but some time between 6.0.1 and 6.0.4 this has become impossible via a normal request, so now as a "bandaid" fix we let a computer set a requester's configuration to a crafting request with configureCraft, and normal requests with configure. this means, if you interact with the ui - it gets converted to a normal request. I.e certified jank teritorry. (As well as, this now gives the redstone requester a function it previously never had, which we try to avoid in this pr.) We believe this is... barely passable. You can check the documentation on how it works here.

So, right now the redstone requester can .setAddress then configure or configureCraft and finally .request what was assigned in the configure/configureCraft's 9 slots While the stock ticker just has access to requestFiltered, which takes an address and filter, then goes through the filter and requests by enchants, durability and other traits CC can read. Stock ticker also has the list and listDetailed functions which add a way to look into the items in the storage as a lua table.

But we wanna make it as good as it can be, like the rest of this pr, so we have a couple options:

  • We can turn the setAddress+configureCraft+request combo from the redstone requester and put it on the stock ticker, as something like requestCraft. This means the stock ticker would requestCraft and requestFiltered, while the redstone requester is configure-d for normal package requests
  • We can do the above, and also give it the request to work like the redstone requester's setAddress+configure+request combo, "because it makes sense", but then there's no reason to use the redstone requester, because the stock ticker will have all of it's functionality basekit (except i guess the ability to be mutable, which is cool for the redstone requester). This has been implemented by Glass on Aearil's fork, here
  • We can have it how it is now (redstone requester gets to configure and configureCraft), and possibly lock out the stock ticker's ui while it's connected to a computer, like the sequenced gearshift, so players can't break the request recipie. This is still a design break where the requester can make crafting recipes, which recent patches removed
  • We could possibly make a second pr that normalizes and adds a switch to the redstone requester to toggle between making crafting and normal requests, but then that slowly gnaws at stock gauge's functionalities

Sorry for blasting like days worth of discussion between us upon ye, but getting a thumbs up on any of the ideas (or leaving as is) would help us a ton :3

ps. we're working with powerpig now on combining their pr with ours :D

BirbIrl avatar May 02 '25 21:05 BirbIrl

Hey @IThundxr, hope you don't mind the ping after the couple of months of this PR - but we reached a game-design choice we can't quite decide on.

So, originally this PR allowed people to make crafting requests with the redstone requester by adding blank spaces, but some time between 6.0.1 and 6.0.4 this has become impossible via a normal request, so now as a "bandaid" fix we let a computer set a requester's configuration to a crafting request with configureCraft, and normal requests with configure. this means, if you interact with the ui - it gets converted to a normal request. I.e certified jank teritorry. (As well as, this now gives the redstone requester a function it previously never had, which we try to avoid in this pr.) We believe this is... barely passable. You can check the documentation on how it works here.

So, right now the redstone requester can .setAddress then configure or configureCraft and finally .request what was assigned in the configure/configureCraft's 9 slots While the stock ticker just has access to requestFiltered, which takes an address and filter, then goes through the filter and requests by enchants, durability and other traits CC can read. Stock ticker also has the list and listDetailed functions which add a way to look into the items in the storage as a lua table.

But we wanna make it as good as it can be, like the rest of this pr, so we have a couple options:

  • We can turn the setAddress+configureCraft+request combo from the redstone requester and put it on the stock ticker, as something like requestCraft. This means the stock ticker would requestCraft and requestFiltered, while the redstone requester is configure-d for normal package requests
  • We can do the above, and also give it the request to work like the redstone requester's setAddress+configure+request combo, "because it makes sense", but then there's no reason to use the redstone requester, because the stock ticker will have all of it's functionality basekit (except i guess the ability to be mutable, which is cool for the redstone requester). This has been implemented by Glass on Aearil's fork, here
  • We can have it how it is now (redstone requester gets to configure and configureCraft), and possibly lock out the stock ticker's ui while it's connected to a computer, like the sequenced gearshift, so players can't break the request recipie. This is still a design break where the requester can make crafting recipes, which recent patches removed
  • We could possibly make a second pr that normalizes and adds a switch to the redstone requester to toggle between making crafting and normal requests, but then that slowly gnaws at stock gauge's functionalities

Sorry for blasting like days worth of discussion between us upon ye, but getting a thumbs up on any of the ideas (or leaving as is) would help us a ton :3

ps. we're working with powerpig now on combining their pr with ours :D

I think leaving it as is for now is fine, if there's any changes that need to be made later we can go ahead and do those based on what the rest of the team thinks about the different methods

IThundxr avatar May 02 '25 21:05 IThundxr

Hey! Mind if I add a bit of feedback for this issue as a player, and not a developer?

I would love this integration for the purpose of creating a pseudo ME network using just Create and CC. The main features I'd like to replicate are autocrafting, crafting queue and wireless access.

Wireless access is probably impossible with just Create and CC. I'd need to add an Ender Chest+ mod. Although, I could load a turtle full of packages and send it to my location... But I digress, as I was saying, this feature has nothing to do with this PR.

Autocrafting is another feature that I'd like to implement using CC. I know that stock gauges already implement autocrafting, but I was thinking about something more on-demand. Instead of needing to have some stock to craft things, I'd like to request a dispenser and have my system craft some sticks, wait for the sticks, craft a bow, wait for the bow, and then craft a dispenser. For this I think I'd need a way to interface with the network's promises. I don't know if this PR's .requestCraft method creates a Promise in the network, since I don't quite know how the Promises work code-wise, but if so I'd love to be able to hold a reference to the promise in my CC computer and to be able to query for the promise status.

Now, what I mean for "crafting queue" is this menu AE2 has in the terminals that let you know the current state of your crafting orders. I'd love to have a big screen with the current status of my factory: This autocraft has been ongoing for X seconds and it's waiting for sticks, this stock ticker has requested 64 cobblestone to be smelted, etc. For this I think I'd need to be able to access Promises. Maybe to be able to query for long a Promise has existed, and data regarding the stock gauge that created the promise.

As a bonus, if crafting orders create Promises, and that promise had the crafting recipe, maybe the system could automatically learn crafting patters like Isy's Inventory Manager does for Space Engineers.

I don't know if what I'm asking for is technically feasible, so if I'm talking crazy here forget me!

rmortes avatar May 07 '25 18:05 rmortes

Hey! Mind if I add a bit of feedback for this issue as a player, and not a developer? I would love this integration for the purpose of creating a pseudo ME network using just Create and CC. The main features I'd like to replicate are autocrafting, crafting queue and wireless access.

tldr: you can do it right now in lua, with the functions provided, so no need

the long version: i hear you! All of this is actually entirely possible within lua code right now! As per the rules of Computercraft - you've got to code it yourself (or copy someone else's code :P.). The point of peripheral apis is to allow computers to access basic functions of a block, to then, using the api, make your own complex systems. Using the redstone requester you can already make packages for the autocrafter, and you can route it however you want with the existing create address system (also modifiable with computers).

Would implementing it as basekit with the PR make it more convenient for everyone? yeah, but there's a reason why computercraft turtles don't have a "craft" function, that just lets you pick 9 slots in whatever order and crafts them - instead we have dedicated 9 slots inside the inventory that, unless carefully worked with, will clog up after a couple crafts, as whatever you craft is dumped into the same inventory. There's a reason why there is no "getPos" function. You first have to place 3 turtles with modems and make your own trigonometric gps system. CC:Tweaked only provides a helper program for it, but it's entirely written in lua, you can code your own!

I think we should take notes from the way mekanism implements the computercraft integration. it lets you manage your upgrades, machine settings, but doesn't suddenly let you make the machines do more than they were designed. They simulate things the player would do with the machines, and let you automate that.

At most, i'd allow computers to manage a wall of stock gauges - but coding that is beyond me and everyone else involved. Plus you'd still have to have turtles placing the stock gauges, et cetra. no idea how that'd work. honestly. The stock gauges source code scares me.

So, we're providing all the building blocks needed to make whatever you want. It will take time and effort - and that's the fun part! Why make an entire hard-coded-java autocrafting-queue stack, that half the people don't like, and would want to work slightly differently ("ermm i encode my addresses like "destination.reicpieid", not "destination!recipeid" because it conflicts with the rest of my address system, please add a string parameter in .makeQueuedRequestOrSomething() to change the separator") . Plus some people would want to sort packages via forgports and chians, some would wanna sort them via turtles - et cetra. I think giving the player versatile tools to make specialized systems is the create way of doing things. Anything else would be room for an addon.

Oh and i'm speaking for myself here, not as the whole team working on the PR

BirbIrl avatar May 08 '25 12:05 BirbIrl

Finally, as for multiple filters in the requestFiltered() function, just call the function multiple times with different filters. If we were to add multiple filters on the java end, it'd just be us looping through every arg - so instead of that, we let people run the function multiple times in lua :D.

Just stumbled on this PR recently, so I'm sorry if this reply is a little old xD. I already cloned this and have been messing around a bit but I can't wait to properly see what's possible with this, but I'd like to mention I disagree with this idea of calling requestFiltered multiple times.

The first thing I noticed was the documentation was a bit out of date, which is fine since it was relatively easy to figure out the new format. However, the issue I have is with calling the requestFiltered() function multiple times, since it results in each item being sent as a separate package. I can see how it would be useful if it stacked in some kind of cache, similar to how the DisplayLink works with write() and update(), but I really want to see a form of multiple request system, in any implementation.

I went ahead and took a stab at it, even if it's just for my personal use, and rewrote it into the original request() function that takes an array of items to process them into one request. From my naive understanding, it should be about the same speed as calling requestFiltered() multiple times, except it has the advantage of packing all the items into one package and will not send the package if any of the requests fail.

If anyone is interested in seeing my implementation, I have the fork on my github. Although it's forked from BirbIrl's fork so I don't think it can merge but it doesn't seem like this is the approach Birb is going for anyway. (The forking thing is just because I'm not experienced enough with git to PR a PR..)

Edit: I did find out how to merge my fork into Birb's fork in the Github UI, it's just something I've never done before I guess. So if anyone is curious (or if Birb/others want to merge it with this) it's there if you want it

Electric131 avatar May 17 '25 04:05 Electric131

mr. create mainter a 9th contributor has hit the 7k lines changed "minor mod integration" pr

Noted, it's 5am for me and i'm about to head to bed, but i see this, @Electric131, and sent a screenshot to the group chat and @MaxOrtGit (who's most recently been working on the filter logic, and just generally been doing god's work) is already looking through it. At first glance, i agree, getting 1 request for multiple filters sounds like a good idea, but i'm looking at the code and my brain is blanking. I feel like there's probably a cleaner way to do this. I trust @MaxOrtGit in figuring out the approach (Thank you, both!).

Personally i've been completely out of gas on this, plus i'm a little terrified that we're at +7k -2k lines of code in this pr (going from like, +1000, -2) so there must've been sooome linting/rebase incident, but if anyone wants to do the janitor work/review the existing pr's it'd be much appreciated :3

Either way i'm going back to crawling at a snail's pace with this pr as my finals approach for the next month or so :(

Oh and to the create maintainers: If you're done fixing all the bugs and are open to feature pr's again, let me know. I could just dedicate a day to locking in and implementing, cleaning everything and updating the wiki so that reviewing and merging this is as nice as possible. It'd probably be enough motivation for my brain. Knowing it'd be the last cleaning up i'll have to do before passing alllll the responsibility down to main branch nyeheheh >:)

BirbIrl avatar May 18 '25 03:05 BirbIrl

Hi, thanks a lot for the contributions.

I tried merging this to a local branch and the diff looks ok (+1311-8), must be github acting up

Happy to merge this as soon as you consider it ready.

I will warn you about messing with PackageOrderWithCrafts information, though. The re-packager does not validate it on its end and will dupe or void items if the recipe and order data does not match up. This has been a common source of issues even for base create.

simibubi avatar May 19 '25 13:05 simibubi

The .factorypath file looks like it should not be pushed to git. @BirbIrl Did you forget to add it to gitignore?

your honor i plead an oopsie daisy

BirbIrl avatar May 23 '25 11:05 BirbIrl