Blood-Asp / GT5-Unofficial

Decompiled and modified version of GT5.07.07
160 stars 97 forks source link

[5.09.28] Machines in cleanroom don`t detect controller block. #956

Open Georggi opened 7 years ago

Georggi commented 7 years ago

As I said in discord, according to the code, it goes through tile entities, while searching for controller, but plascrete blocks are not tile entities, so unless you place any gregtech tile entities, like cables, machines, all the way to the top, where filter casings are located, machines won't start working.

MauveCloud commented 7 years ago

I haven't been participating in the discord chats, but why should the machines need to search for the controller? It looks to me like the cleanroom finds the machines: https://github.com/Blood-Asp/GT5-Unofficial/blob/unstable/src/main/java/gregtech/common/tileentities/machines/multi/GT_MetaTileEntity_Cleanroom.java#L129

Georggi commented 7 years ago

Right, that part of code is exactly what I mean by machines searching for cleanroom controller for the first time.

MauveCloud commented 7 years ago

You're not making any sense. What does the code I linked have to do with machines searching for the cleanroom controller? Seems to me your understanding of it might be backwards, and where is there any requirement for a continuous chain of tile entities connecting the machine to the controller block?

Georggi commented 7 years ago

@MauveCloud That whole loop from line 122 makes that, and if you want, try it out yourself, I built 15 15 15 cleanroom, and on start it wasn't working, altough everything was correct, then placed cables from bottom, where my engraver is to the top, where filter casings are and it magically started working.

P.s. If exactly, check this condition, it checks if it is tile entity. https://github.com/Blood-Asp/GT5-Unofficial/blob/unstable/src/main/java/gregtech/common/tileentities/machines/multi/GT_MetaTileEntity_Cleanroom.java#L128

MauveCloud commented 7 years ago

I won't dispute your test results, but that code does not explain them. The loop looks for tile entities anywhere within the cleanroom - it does not have a condition or break statement to stop if there's an air gap between the casings and the machine instead of cables.

Checking whether the machine is in a cleanroom is here: https://github.com/Blood-Asp/GT5-Unofficial/blob/unstable/src/main/java/gregtech/api/metatileentity/implementations/GT_MetaTileEntity_BasicMachine.java#L741

That doesn't handle transferring energy to the machines inside, though, which is probably where the last line of the cleanroom's tooltip comes in: "up to 10 Machine Hull to transfer Items&Energy inside"

Georggi commented 7 years ago

Well, idk then, that part is logical though, cause mCleanroom may be null and so recipe doesn't meet requirement.

MauveCloud commented 7 years ago

I was going to try to set up some screenshots to show what I thought was the correct way to set things up with a machine hull (though I haven't figured out how machine hulls are supposed to allow transferring items inside 😕 ), using a smaller cleanroom (according to the tooltip, it's supposed to allow a variable size), but I'm stuck on "incomplete structure".

Georggi commented 7 years ago

@MauveCloud Place controller at last, machine hulls have 1 slot inside of them, so they are buffers for that

MineAnPlay commented 6 years ago

This issue isn't really Minor... Yes it's easy to fix but people will spam us and everywhere about this issue happening. I don't know coding but it really shouldn't be that hard to code a fix for this.

Blood-Asp commented 6 years ago

I have no idea how it does not work in some cases. I tested some things but could not find a solution. It seems that some sizes or positions inside are problematic but i could not find a pattern there.

So as long nothing points me in the right direction, i can't fix it. I made it "minor" because it somehow still works and i don't want to invest many hours in it while i could fix dozens of other things in the same time.

MineAnPlay commented 6 years ago

OK I understand that you don't have much time to just look at one error for a long time but people might get annoyed (like me :P) when they see an issue that the is labeled Minor. Because not only does it sometimes not even find the machines even when connected with tile entities but also that we will be continuously spammed with this error from all around (mainly from GTNH since I'm there).

Blood-Asp commented 6 years ago

The error should be here. https://github.com/Blood-Asp/GT5-Unofficial/blob/unstable/src/main/java/gregtech/common/tileentities/machines/multi/GT_MetaTileEntity_Cleanroom.java#L135-L142 but i can't find the problem. If it is really that annoying, there are by now so many programmers out there that might take a look at it...

MineAnPlay commented 6 years ago

https://github.com/GTNewHorizons/NewHorizons/issues/1934 He really should do PR but for now there's a link that fixes ore gen and adds debug messages to the cleanroom. (We have also tested the issue in SP but couldn't replicate it. Might be server side.)

richardhendricks commented 6 years ago

I just put a patch file there since I can't fork the repo (I already have the fork from GTNH). Here it is as well. It is based off the 73662e53 commit to unstable branch.

patch.zip

draknyte1 commented 6 years ago

It's not a fix for oregen lol. It's just a way to have oreveins evertywhere which we'd have default to off.

richardhendricks commented 6 years ago

There isn't an on/off, there is a % setting. Based on my numbers for the overworld, ~75% is close to what we get now. The new code would enforce that across all dimensions though - the old code is pretty arbitrary on how much empty space it leaves.

draknyte1 commented 6 years ago

So we'd add a config for said %..

richardhendricks commented 6 years ago

My default in the patch is 100%, but whatever.

Sample debug output for cleanroom (door is open) Cleanroom: Machine detected, adding pointer back to cleanroom Cleanroom: Machine detected, adding pointer back to cleanroom BasicMachine: Voiding output due to efficiency failure. mEfficiency = 6200 Cleanroom: Machine detected, adding pointer back to cleanroom Cleanroom: Machine detected, adding pointer back to cleanroom Cleanroom: Machine detected, adding pointer back to cleanroom Cleanroom: Machine detected, adding pointer back to cleanroom BasicMachine: Voiding output due to efficiency failure. mEfficiency = 6000 Cleanroom: Machine detected, adding pointer back to cleanroom Cleanroom: Machine detected, adding pointer back to cleanroom BasicMachine: Voiding output due to efficiency failure. mEfficiency = 5900 Cleanroom: Machine detected, adding pointer back to cleanroom Cleanroom: Machine detected, adding pointer back to cleanroom

Dream-Master commented 6 years ago

@richardhendricks Did you make a pull to us GTNH fork too?

richardhendricks commented 6 years ago

I didn't, but I can. I'll have to rebase on your latest first. Ugh.