kepler155c / opus-apps

Computercraft Applications for Opus OS
MIT License
31 stars 28 forks source link

Add a Defragment button to Milo #36

Closed Luca0208 closed 4 years ago

Luca0208 commented 4 years ago

This adds a defragment feature to Milo. It checks for fragmented item stacks. E.g. 2 stacks of 32 Cobblestone will be combined into 1 stack of 64 cobblestone, therefore freeing an item slot for Milo.

kepler155c commented 4 years ago

This is the #1 request I've had for milo enhancements - thanks for taking the time. Just a couple issues need to be addressed - see added comments.

xAnavrins commented 4 years ago

I can't seem to see the comments, I think you might have forgotten to add them?

kepler155c commented 4 years ago

I can't seem to see the comments, I think you might have forgotten to add them?

DOH

Luca0208 commented 4 years ago

This should now also handle locked chests correctly, however I'm pretty tired right now, I'm gonna check over the code tomorrow.

kepler155c commented 4 years ago

Is this ready to go ? Anavrins please look over as well.

xAnavrins commented 4 years ago

Firstly, we'll need to update this PR with the new UI changes, since using this PR branch will not work with Opus UI 2.0 changes, it's somehow unable to recognize chest types and you can't add storage nodes, Milo UI 2.0 works fine.

I had to test with an older tree from before the UI changes to make it able to see the chests, once I've done that and connected the chests, I put half a stack of an item in a locked chest, and the other half in an unlocked chest, it was unable to do the defragmentation and errored with storage.lua:377:attempt to index local 'to' (a nil value) This happened with both the locked chest being locked and unlocked.

Luca if you'd like to debug this, I have a setup ready at my base on SC which you have access, or just DM me on Discord.

Luca0208 commented 4 years ago

That commit I just pushed fixed the error, I'm gonna do some more testing with Anavrins setup to make sure it actually works properly

Luca0208 commented 4 years ago

Ok, I did some testing and from what I see it works, if a chest is locked to a specific item it will only push that item into the chest and never pull that item out of the chest.

@xAnavrins Can you elaborate on what happens when using this with the UI changes?

xAnavrins commented 4 years ago

Defrag seem to be working fine! I have no idea why your branch is unable to see networked chests, but it only happens with an up to date Opus version past the UI changes, an up to date Milo works fine though. I will merge the PR now and hope for the best that your changes will merge correctly with the new Milo version, otherwise I'll just make a commit to manually update your branch with the UI changes.

xAnavrins commented 4 years ago

I can confirm that everything merged correctly and everything is working fine! The chests not being able to be seen seem to have been a weird quirk on my setup, because it started working again when I retried.