Civcraft / RealisticBiomes

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/RealisticBiomes
7 stars 14 forks source link

Dark Oak doesn't always start growing #68

Open Langly- opened 8 years ago

Langly- commented 8 years ago

Dark oak can sit and never grow until refreshed with a stick, it's timer will show it never started. It seems if the northwest sapling isn't the last planted it will never grow unless hand updated. The Northwest sapling seems to be the control sapling. The other 2x2 trees can still grow in 1x1 form, dark oak can not. I think it might not be starting because a single sapling on it's own is considered ungrowable. Once the 2x2 is formed it becomes growable, but if the northwest sapling wasn't the last it doesn't update from not being growable, as long as it is the last sapling it always works, sometimes it works if just the north or west side is done last, but not that corner. More testing is needed to peg it exactly, but it's being quite odd.

WildWeazel commented 8 years ago

I'm not sure if 2x2 trees were ever persisted correctly. It's probably checking, when each sapling is planted/updated, whether that one is a valid NW corner before storing it.

WildWeazel commented 8 years ago

Yeah, this seems to be the culprit: https://github.com/Civcraft/RealisticBiomes/blob/master/src/com/untamedears/realisticbiomes/utils/Trees.java#L120 It should check the 3 other blocks for each of 4 possible arrangements.

E && SE && S OR W && SW && S OR N && NE && E OR W && NW && W

and then add the NW-most of the four, if they match.

ProgrammerDan commented 8 years ago

safer test would be:

Scan diagonals NE,NW,SW,SE for sapling of same type. If found, check for "inbetween" along the diagonal path:

and done.

benjajaja commented 8 years ago

@Langly- is right, the northwestern sapling is the "root" for giant trees as in vanilla: http://minecraft.gamepedia.com/Tree

So the logic of RB is built on that assumption, which means that Trees.canGrowLArge should not be changed. The problem is that a NW sapling get added to the list (and db) with the growth rate at the time of placement - so if the rest of the 2x2 array isn't present, it gets added with the growth of a single sapling. When the rest of the sapplings are added, the existing NW will not be updated. The scenario described supports this.

The minimal approach would be to check, in BlockPlaceEvent, if a sapling being placed completes a 2x2 array, then update the NWmost sapling of that array.

benjajaja commented 8 years ago

BTW, reloading a chunk should also have the same effect as clicking the NW sapling with a stick, currently.

ProgrammerDan commented 8 years ago

Thanks for the expert knowledge dump. Worth noting that the algorithm I framed would be also be optimal for any check injected in BlockPlaceEvent. It has worst-case 7 tests, best case 3. (edit: corrected best case time)

Langly- commented 8 years ago

"BTW, reloading a chunk should also have the same effect as clicking the NW sapling with a stick, currently."

That has been working IF the clock was already running, it updates the clock, but it doesn't seem to actually start it if it thought it couldn't run.

I found many Dan had planted the night before, and even when I loaded them the timer still wasn't running on them.

benjajaja commented 8 years ago

Yes, now that you say it, the sapling will probably never get added to the list (and db) if the growth rate was zero. It might get re-checked for 2x2 growth times, but only if there was any chance for the sapling to grow as a simple tree, which is not the case for dark oak.

benjajaja commented 8 years ago

@ProgrammerDan your algo would be great for testing if a newly planted sappling completes a 2x2 array, that is true.