GTNewHorizons / GT-New-Horizons-Modpack

New Modpack with Gregtech, Thaumcraft and Witchery
https://www.gtnewhorizons.com/
Other
986 stars 299 forks source link

Multifarm layer stacking (one multifarm, many layers) should be fixed #5975

Closed Prometheus0000 closed 4 years ago

Prometheus0000 commented 4 years ago

Which modpack version are you using?

2.0.8.8 # If you didn't know, you can 'stack' multifarms in orchard mode, since they are designed to be able to get produce from large trees and such. However, stacking a lot can apparently cause lag, and is clearly not intended. I believe this should be fixed to prevent this kind of abuse. #

What do you suggest instead/what changes do you propose?

Prevent stacking, however that would work code-wise.

Dream-Master commented 4 years ago

you spoke about forestry farm ? @Prometheus0000

Prometheus0000 commented 4 years ago

Yes

leagris commented 4 years ago

Multifarm stacking is intentional and expected design where you put peat farm at lower layer to fuel the farms and renew soil of arboretum, provide source of ash for fertilizer multiplier from the peat engines burning peat.

Orchard farms shall not be stacked. Instead you shall add more gearboxes to a single orchard, so it processes faster. The orchard has a very high and wide processing area.

Now to prevent multiple multifarms to fight intersection of their processing area maybe logic could be added to chop limit the processing height when another multifarm tile is intersecting the processing volume, so there is no more intersection of processing volumes.

Prometheus0000 commented 4 years ago

I don't know where you got your information on peat farms? There's no indication you should do that at the forestry wiki? https://forestryforminecraft.info/farms:structure Or anywhere else I looked.

You might want to check out the discussion on the discord server

Bluebine commented 4 years ago

Honestly we should just do whatever causes the least lag. Multifarms aren't expensive, so stacking isn't really an exploit that needs to be fixed. But I've seen people reporting that huge multi-layer farms can cause lag, so IMHO we should just do whatever will reduce TPS the least.

ghost commented 4 years ago

A suggestion I brought up in discord if someone would like to code it: Crops really only need to be checked for harvesting every 12.8 seconds, so the orchard is currently doing quite a bit of redundant work. Perhaps it could be significantly improved if crops were disabled from the orchard mode, and a new circuit mode was added that only checks the crops as much as they need. Sampsa suggested having it update an array of crop locations every minute or so, and check them all every ~10s, which would likely be a good performance boost. To not trivialize the gearboxes with this change, some restriction could be added such as max 750 crops per gearbox or so to be done in a single pass.

quedral commented 4 years ago

Peat farms is a thing. It was there since mc 1.4.7 but today its not as popular as before. you can check there in farm configuration section of that wiki and you'll see "peat bog" configuration. wiki does not contain peat bog layered setup.

that being said the orchard mode should not act as peat bog farm as leagris mentioned.

Prometheus0000 commented 4 years ago

I know peat farms are a thing, there's even a mode for it. I meant the stacking of peat farms. Though I thought that stacking only worked in orchard mode? Also, I think it shouldn't be allowed because it violates the spirit of farming (how do the plants undergo photosynthesis with no light?), and it looks stupid. But it is a game after all. If someone does go through the code, it would be nice if it was made faster when harvesting crops, or if adding gearboxes had a more noticeable effect to the speed of harvesting.

Anarack commented 4 years ago

This is a different issue, but should we think about buffing peat? I honestly can't remember the last time I saw someone making a peat farm. though I didn't know you could stack it like that.

leagris commented 4 years ago

I spent a good amount of time playing with all aspects and flavours of Forestry and a good amount of time testing and chatting with @Mezz about the features he added to Forestry 4.0 to 4.2. One of the major feature we shall not underestimate here is the ability to add multiple gearboxes to a multi-farm to speed-up processing speed. Now the orchard circuit configuration purpose is to harvest fruits and dropped items in a large and high volume. So instead of stacking farms, just stack soil and cropsticks to a multifarm. The farm does not care if it is touching the lower farm ground layer. It just check at height like up to 128 blocks up, to be able to harvest fruilts from trees with the gigantic height trait. If you stack farms, you use an overkill scanning rate for the slow speed of IC2 crops maturation. But if you stack 4dirtsoil+crop+4air multiple times for the same farm, up to max harvest height. Then the processing speed of the multifarm will not be overkill. You will probably need to add a couple more gearboxes to speed it up. But for sure, you will not create excess server ticking time as you may encounter if you stack multiple farms who all intersect their processing area vertically and disputes processing the blocks at intersected regions.

Prometheus0000 commented 4 years ago

Didn't sampsa say his server went from 75 to 40 milliseconds lag just turning off 2 group's farms though? He didn't say how many there were or how stacked though. That's not insignificant on a server.

I think an ic2 crop mode for the circuits that has greatly reduced vertical range might be an idea worth considering. Like stone brick (or whatever you use)+6 vertical height searched. You should still be able to get full stats and checkerboard for weed crops like that.

You mentioned multiple farms fighting each other twice, but this issue isn't about that though?

leagris commented 4 years ago

You mentioned multiple farms fighting each other twice, but this issue isn't about that though?

If you stack multiple farms with the orchard circuit, the lower farm check volume will also include the volume of the farm above and so on… No wonder it can create lag.

My point is, instead of stacking multiple orchard farms each with its controllers hatches, gearboxes:

Use a single orchard farm and use its whole processing height, to stack multiple layers of dirt, crops, air, lightsources, water... as relevant to the crop type. Even if 9 total block height are dedicated to each layer, you can still manage 14 layers of crops with a single farm.

The ticking block doing work and taking CPU is the farm gearbox. If a single gearbox is handling 14 layers of crops with the wider farm setup. It will not take more CPU load that it would if it had a single layer. (It may not be fast enough for this gigantic setup, so you may want to add more gearboxes to the farm though).

Prometheus0000 commented 4 years ago

I think we may be talking about different things? When I say stacked farms, I'm talking about the exact thing you suggest doing in your third paragraph, not separate multifarms on top of each other. And the thing you suggest doing is the thing I say needs fixing. People should use different farms, spread horizontally. And as I already said, this does cause lag, as stated by sampsa.

Methes commented 4 years ago

Might be easier to do your own testing to back up your cause. Try making as high farm as possible filled with crops and then the same number of crops using one layered farms. Tell us the server ms difference.

EDIT: Also note that the alleged lag source in Sampsa's case might not be from the farms themselves but from processing the vast amount of products comming from them.

Prometheus0000 commented 4 years ago

To me the main point isn't whether it causes lag, it's about the fact that it's clearly unintended. Plus I don't have my own private server to test, it sounds like a pain to setup just to test (just placing cropsticks for one farm takes a while, much less building the whole thing, or doing so dozens of times), and I trust sampsa's result.

mitchej123 commented 4 years ago

It sounds like leagris thinks it is intentional and has chatted with the mod author about it in the past..

Prometheus0000 commented 4 years ago

But they also seem to have been thinking this was about multiple complete multifarms on top of each other fighting each other over the crops, when it isn't about that even a little bit.

mitchej123 commented 4 years ago

Your title implies it was ;).

I'd like to see some profiling of one massive farm with multiple gear boxes vs multiple farms that have the same area, rather than anecdotal evidence.

Having read the code, I don't think it is unintentional, so that point is invalid as far as I am concerned

Prometheus0000 commented 4 years ago

Isn't it just like this to allow for high trees? Though maybe it wouldn't allow following the dirt if it wasn't intentional. If I'm in the minority (if we do a vote) maybe just focus on making it more efficient like sthegreat's suggestion then?

mitchej123 commented 4 years ago

It follows dirt and water pillars. It's for more than trees

Prometheus0000 commented 4 years ago

If it's intentional then I wonder why only GTNH seems to know about it then though? Nobody else using crops I guess?

mitchej123 commented 4 years ago

It's not well documented, and few other modpacks have resource demands as high as GTNH, so there's less incentive to find it. Plus most people just default to spamming more of the same machine.

Anarack commented 4 years ago

That, and well nothing even remotely suggests that the range is close to how big it is.

leagris commented 4 years ago

Range is that big because:

The processing volume size was designed to work with the Arboretum witch has to cut logs very high, and collect fallen saplings and apples at a wide range.

Now, in regard to the Orchard mode, I agree that this width and height are overkill. So the Orchard circuit could have its own volume size (would probably require significant code change to make to processing volume a property of the farm circuit, rather than the farm multiblock size).

Another consideration is not disrupting existing setups. If the orchard work volume is reduced, every farm making use of the old volume will stop functioning.

What could be done smartly without impacting existing setups is: Checking what is the most significant source of lag. I have a suspicion that the farm is looking of item entities within the whole volume, and since the call to check for entities takes a cuboid box. So the farm has to perform multiple calls for the check of its diamond shaped box. I also suspect that the check for item entities could be squeezed to the lowest 1 or 2 layers. Also the check for item entities in an orchard could be removed since no orchard drops item entities AFAIK.

And finally, instead of throwing the blame at Forestry Multifarm for lagging with oversized layered orchard farms harvesting IC2 crops. Maybe question the overly inefficient working of the IC2 crops, horribly low yield that incentivize players into spamming crops to have enough production rate.

Maybe instead of having to spam layered IC2 crops, there was a smaller Gregtech machine with hydroponics/airponics, technology that would:

Prometheus0000 commented 4 years ago

I have no idea if the IC2 crop code is inefficient, I'm no coder or anything. Don't they drop a decent amount if you level them up though? I agree about the alternate GT machine thing though. That was mentioned in discord, and something similar on another issue previously #4164. Though I think the 'digital crop farm' idea sounds like a much better implementation for reducing lag.

Ngar1 commented 4 years ago

Aware, that also orchard farms needs higher level, because of tree products.

Prometheus0000 commented 4 years ago

I'm closing this since it seems that stacking is intentional (though I still don't like it). Adjusting the MF to be more efficient checking crops is still a good idea regardless if anyone wants to try that, but if anyone wants to talk about a new machine they should post in #4164 instead.

bookerthegeek commented 3 years ago

If it's intentional then I wonder why only GTNH seems to know about it then though? Nobody else using crops I guess?

I don't know about other people but using one multiblock farm in orchard mod and multiple farm layers is a thing on the regrowth server I play on. Myself an a few other people build our farm at the bedrock layer and it goes Dirt -> AgriCraft Crops -> x3 Air -> Repeat.

With multiple max sized farms (Mine is 8 layers, buddies is 5 i think) they are still less laggy then a tinkers smeltery pouring into 10 casting basins at the same time.

Prometheus0000 commented 3 years ago

It's been changed by bart though, so don't expect it to work here

bookerthegeek commented 3 years ago

... Well that is just unsporting.